You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/12/17 18:20:21 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5014: fs/vfs: Add file descriptor based timers support
xiaoxiang781216 commented on a change in pull request #5014:
URL: https://github.com/apache/incubator-nuttx/pull/5014#discussion_r771588692
##########
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"
Review comment:
/var/timer?
##########
File path: include/sys/timerfd.h
##########
@@ -0,0 +1,80 @@
+/****************************************************************************
+ * include/sys/timerfd.h
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_SYS_TIMERFD_H
+#define __INCLUDE_SYS_TIMERFD_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdint.h>
+#include <fcntl.h>
+#include <time.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define TFD_NONBLOCK O_NONBLOCK
+#define TFD_CLOEXEC O_CLOEXEC
+
+#define TFD_TIMER_ABSTIME TIMER_ABSTIME
+
+/* Get device minor number */
+
+#define TFD_FIOC_MINOR FIOC_MINOR
+
+/****************************************************************************
+ * Public Type Declarations
+ ****************************************************************************/
+
+/* Type for timer counter */
+
+typedef uint32_t timerfd_t;
Review comment:
timerfd_t isn't used?
##########
File path: include/sys/timerfd.h
##########
@@ -0,0 +1,80 @@
+/****************************************************************************
+ * include/sys/timerfd.h
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_SYS_TIMERFD_H
+#define __INCLUDE_SYS_TIMERFD_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdint.h>
+#include <fcntl.h>
+#include <time.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define TFD_NONBLOCK O_NONBLOCK
+#define TFD_CLOEXEC O_CLOEXEC
+
+#define TFD_TIMER_ABSTIME TIMER_ABSTIME
+
+/* Get device minor number */
+
+#define TFD_FIOC_MINOR FIOC_MINOR
+
+/****************************************************************************
+ * Public Type Declarations
+ ****************************************************************************/
+
+/* Type for timer counter */
+
+typedef uint32_t timerfd_t;
+
+/****************************************************************************
+ * Public Function Prototypes
+ ****************************************************************************/
+
+#ifdef __cplusplus
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+int timerfd_create(int clockid, int flags);
+
+int timerfd_settime(int fd, int flags,
+ FAR const struct itimerspec *new_value,
+ FAR struct itimerspec *old_value);
+
+int timerfd_gettime(int fd, FAR struct itimerspec *curr_value);
+
+int timerfd_get_minor(int fd);
Review comment:
Not defined on Linux
##########
File path: include/sys/syscall_lookup.h
##########
@@ -218,6 +218,11 @@ SYSCALL_LOOKUP(pwrite, 4)
#ifdef CONFIG_EVENT_FD
SYSCALL_LOOKUP(eventfd, 2)
#endif
+#ifdef CONFIG_TIMER_FD
+ SYSCALL_LOOKUP(timerfd_create, 2)
+ SYSCALL_LOOKUP(timerfd_settime, 4)
+ SYSCALL_LOOKUP(timerfd_gettime, 2)
Review comment:
it's better to implement timerfd_settime/timerfd_gettime through ioctl to reduce the syscall number.
##########
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:
why not remove wdog and call work_queue with delay instead?
##########
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 */
+ timerfd_t counter; /* timerfd counter */
+ size_t minor; /* timerfd minor number */
+ uint8_t crefs; /* References counts on timerfd (max: 255) */
+
+ /* The following is a list if poll structures of threads waiting for
+ * driver events.
+ */
+
+#ifdef CONFIG_TIMER_FD_POLL
+ FAR struct pollfd *fds[CONFIG_TIMER_FD_NPOLLWAITERS];
+#endif
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static int timerfd_open(FAR struct file *filep);
+static int timerfd_close(FAR struct file *filep);
+
+static ssize_t timerfd_read(FAR struct file *filep, FAR char *buffer,
+ size_t len);
+static int timerfd_ioctl(FAR struct file *filep, int cmd,
+ unsigned long arg);
+#ifdef CONFIG_TIMER_FD_POLL
+static int timerfd_poll(FAR struct file *filep, FAR struct pollfd *fds,
+ bool setup);
+
+static void timerfd_pollnotify(FAR struct timerfd_priv_s *dev,
+ pollevent_t eventset);
+#endif
+
+static int timerfd_blocking_io(FAR struct timerfd_priv_s *dev,
+ timerfd_waiter_sem_t *sem,
+ FAR timerfd_waiter_sem_t **slist);
+
+static size_t timerfd_get_unique_minor(void);
+static void timerfd_release_minor(size_t minor);
+
+static FAR struct timerfd_priv_s *timerfd_allocdev(void);
+static void timerfd_destroy(FAR struct timerfd_priv_s *dev);
+
+static void timerfd_timeout_work(FAR void *arg);
+static void timerfd_timeout(wdparm_t idev);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct file_operations g_timerfd_fops =
+{
+ timerfd_open, /* open */
+ timerfd_close, /* close */
+ timerfd_read, /* read */
+ NULL, /* write */
+ NULL, /* seek */
+ timerfd_ioctl /* ioctl */
+#ifdef CONFIG_TIMER_FD_POLL
+ , timerfd_poll /* poll */
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static FAR struct timerfd_priv_s *timerfd_allocdev(void)
+{
+ FAR struct timerfd_priv_s *dev;
+
+ dev = (FAR struct timerfd_priv_s *)
+ kmm_zalloc(sizeof(struct timerfd_priv_s));
+ if (dev)
+ {
+ /* Initialize the private structure */
+
+ nxsem_init(&dev->exclsem, 0, 0);
+ }
+
+ return dev;
+}
+
+static void timerfd_destroy(FAR struct timerfd_priv_s *dev)
+{
+ wd_cancel(&dev->wdog);
+ work_cancel(TIMERFDWORK, &dev->work);
+ nxsem_destroy(&dev->exclsem);
+ kmm_free(dev);
+}
+
+#ifdef CONFIG_TIMER_FD_POLL
+static void timerfd_pollnotify(FAR struct timerfd_priv_s *dev,
+ pollevent_t eventset)
+{
+ FAR struct pollfd *fds;
+ int i;
+
+ for (i = 0; i < CONFIG_TIMER_FD_NPOLLWAITERS; i++)
+ {
+ fds = dev->fds[i];
+ if (fds)
+ {
+ fds->revents |= eventset & fds->events;
+
+ if (fds->revents != 0)
+ {
+ nxsem_post(fds->sem);
+ }
+ }
+ }
+}
+#endif
+
+static size_t timerfd_get_unique_minor(void)
+{
+ static size_t minor;
+
+ return minor++;
+}
+
+static void timerfd_release_minor(size_t minor)
+{
+}
+
+static int timerfd_open(FAR struct file *filep)
+{
+ FAR struct inode *inode = filep->f_inode;
+ FAR struct timerfd_priv_s *priv = inode->i_private;
+ int ret;
+
+ /* Get exclusive access to the device structures */
+
+ ret = nxsem_wait(&priv->exclsem);
+ if (ret < 0)
+ {
+ return ret;
+ }
+
+ finfo("crefs: %d <%s>\n", priv->crefs, inode->i_name);
+
+ if (priv->crefs >= 255)
+ {
+ /* More than 255 opens; uint8_t would overflow to zero */
+
+ ret = -EMFILE;
+ }
+ else
+ {
+ /* Save the new open count on success */
+
+ priv->crefs += 1;
+ ret = OK;
+ }
+
+ nxsem_post(&priv->exclsem);
+ return ret;
+}
+
+static int timerfd_close(FAR struct file *filep)
+{
+ int ret;
+ FAR struct inode *inode = filep->f_inode;
+ FAR struct timerfd_priv_s *priv = inode->i_private;
+
+ /* devpath: TIMER_FD_VFS_PATH + /tfd (4) + %d (3) + null char (1) */
+
+ char devpath[sizeof(CONFIG_TIMER_FD_VFS_PATH) + 4 + 3 + 1];
Review comment:
3->10 like open
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org