pkarashchenko commented on a change in pull request #5014: URL: https://github.com/apache/incubator-nuttx/pull/5014#discussion_r771995596
########## File path: fs/vfs/fs_timerfd.c ########## @@ -0,0 +1,840 @@ +/**************************************************************************** + * fs/vfs/fs_timerfd.c + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> +#include <stdio.h> +#include <poll.h> +#include <assert.h> +#include <errno.h> +#include <fcntl.h> + +#include <debug.h> + +#include <nuttx/wdog.h> +#include <nuttx/irq.h> +#include <nuttx/wqueue.h> + +#include <sys/ioctl.h> +#include <sys/timerfd.h> + +#include "clock/clock.h" +#include "inode/inode.h" + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#ifndef CONFIG_TIMER_FD_VFS_PATH +#define CONFIG_TIMER_FD_VFS_PATH "/dev" +#endif + +#ifndef CONFIG_TIMER_FD_NPOLLWAITERS +/* Maximum number of threads than can be waiting for POLL events */ +#define CONFIG_TIMER_FD_NPOLLWAITERS 2 +#endif + +#define TIMERFDWORK LPWORK + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +typedef struct timerfd_waiter_sem_s +{ + sem_t sem; + struct timerfd_waiter_sem_s *next; +} timerfd_waiter_sem_t; + +/* This structure describes the internal state of the driver */ + +struct timerfd_priv_s +{ + sem_t exclsem; /* Enforces device exclusive access */ + timerfd_waiter_sem_t *rdsems; /* List of blocking readers */ + int clock; /* Clock to use as the timing base */ + int delay; /* If non-zero, used to reset repetitive + * timers */ + struct wdog_s wdog; /* The watchdog that provides the timing */ + struct work_s work; /* For deferred timeout operations */ Review comment: `wdog` + `work_queue` is widely used in NuttX code when deferred processing that involve synchronization primitives is needed. I explored `timer_settime` case before implementing `timerfd` exactly in this way as it is now. I really do not think that calling `nxsem_wait_uninterruptible(&dev->exclsem);` from `wdog` expiration callback is a good idea and obtaining `dev->exclsem` is mandatory to ensure integrity of `struct timerfd_priv_s` members (except `counter` of course that is guarded by the critical section). So I see `wdog` + `work_queue` as an only possible solution to implement `timerfd` in a reliable way from both timing and date integrity perspective. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
