[Xenomai-core] [PATCH] RTDM: refactor and document select API extension

[Xenomai-core] [PATCH] RTDM: refactor and document select API extension

[Jan Kiszka]

[-- Attachment #1.1: Type: text/plain, Size: 414 bytes --]

Hi Gilles,

unfortunately I missed (or more likely forgot) that the RTDM select 
extensions already made it into stable and thus now also into a release. 
However, here is an overdue cleanup and documentation patch. Please 
check if I happen to have written nonsense or broke something (only 
compile-tested). Don't hesitate to suggest better names for types, 
fields etc. if you find any.

Thanks,
Jan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: rtdm-select-refactor-and-document.patch --]
[-- Type: text/x-patch; name="rtdm-select-refactor-and-document.patch", Size: 17350 bytes --]

---
 ChangeLog                      |    5 +
 doc/doxygen/Doxyfile-common.in |    1 
 include/rtdm/rtdm.h            |    8 -
 include/rtdm/rtdm_driver.h     |   67 +++++++++++-----
 ksrc/skins/posix/syscall.c     |    7 -
 ksrc/skins/rtdm/core.c         |   84 +++++++++++++-------
 ksrc/skins/rtdm/drvlib.c       |  169 ++++++++++++++++++++++++++++-------------
 7 files changed, 234 insertions(+), 107 deletions(-)

Index: b/include/rtdm/rtdm.h
===================================================================
--- a/include/rtdm/rtdm.h
+++ b/include/rtdm/rtdm.h
@@ -31,7 +31,7 @@
  * RT/non-RT systems like Xenomai. RTDM conforms to POSIX
  * semantics (IEEE Std 1003.1) where available and applicable.
  *
- * @b API @b Revision: 6
+ * @b API @b Revision: 7
  */
 
 /*!
@@ -75,7 +75,7 @@ typedef struct task_struct rtdm_user_inf
  * @anchor api_versioning @name API Versioning
  * @{ */
 /** Common user and driver API version */
-#define RTDM_API_VER			6
+#define RTDM_API_VER			7
 
 /** Minimum API revision compatible with the current release */
 #define RTDM_API_MIN_COMPAT_VER		6
@@ -247,10 +247,6 @@ ssize_t __rt_dev_recvmsg(rtdm_user_info_
 			 struct msghdr *msg, int flags);
 ssize_t __rt_dev_sendmsg(rtdm_user_info_t *user_info, int fd,
 			 const struct msghdr *msg, int flags);
-struct xnselector;
-int __rt_dev_select_bind(rtdm_user_info_t *user_info, int fd,
-			 struct xnselector *selector,
-			 unsigned type, unsigned index);
 #endif /* __KERNEL__ */
 
 /* Define RTDM_NO_DEFAULT_USER_API to switch off the default rt_dev_xxx
Index: b/include/rtdm/rtdm_driver.h
===================================================================
--- a/include/rtdm/rtdm_driver.h
+++ b/include/rtdm/rtdm_driver.h
@@ -4,6 +4,7 @@
  *
  * @note Copyright (C) 2005-2007 Jan Kiszka <jan.kiszka@domain.hid>
  * @note Copyright (C) 2005 Joerg Langenberg <joerg.langenberg@domain.hid>
+ * @note Copyright (C) 2008 Gilles Chanteperdrix <gilles.chanteperdrix@xenomai.orgmail.com>
  *
  * Xenomai is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by
@@ -49,6 +50,8 @@
 #endif
 
 struct rtdm_dev_context;
+typedef struct xnselector rtdm_selector_t;
+enum rtdm_selecttype;
 
 /*!
  * @addtogroup devregister
@@ -98,7 +101,7 @@ struct rtdm_dev_context;
  * @{
  */
 /** Version of struct rtdm_device */
-#define RTDM_DEVICE_STRUCT_VER		4
+#define RTDM_DEVICE_STRUCT_VER		5
 
 /** Version of struct rtdm_dev_context */
 #define RTDM_CONTEXT_STRUCT_VER		3
@@ -187,6 +190,22 @@ typedef int (*rtdm_ioctl_handler_t)(stru
 				    unsigned int request, void __user *arg);
 
 /**
+ * Select binding handler
+ *
+ * @param[in] context Context structure associated with opened device instance
+ * @param[in,out] selector Object that shall be bound to the given event
+ * @param[in] type Event type the selector is interested in
+ * @param[in] fd_index Opaque value, to be passed to rtdm_event_select_bind or
+ * rtdm_sem_select_bind unmodfied
+ *
+ * @return 0 on success, otherwise negative error code
+ */
+typedef int (*rtdm_select_bind_handler_t)(struct rtdm_dev_context *context,
+					  rtdm_selector_t *selector,
+					  enum rtdm_selecttype type,
+					  unsigned fd_index);
+
+/**
  * Read handler
  *
  * @param[in] context Context structure associated with opened device instance
@@ -262,11 +281,6 @@ typedef ssize_t (*rtdm_sendmsg_handler_t
 
 typedef int (*rtdm_rt_handler_t)(struct rtdm_dev_context *context,
 				 rtdm_user_info_t *user_info, void *arg);
-
-typedef int (*rtdm_select_bind_handler_t)(struct rtdm_dev_context *context,
-					  struct xnselector *selector,
-					  unsigned type,
-					  unsigned index);
 /**
  * Device operations
  */
@@ -282,6 +296,9 @@ struct rtdm_operations {
 	rtdm_ioctl_handler_t ioctl_rt;
 	/** IOCTL from non-real-time context (optional) */
 	rtdm_ioctl_handler_t ioctl_nrt;
+
+	/** Select binding handler for any context (optional) */
+	rtdm_select_bind_handler_t select_bind;
 	/** @} Common Operations */
 
 	/*! @name Stream-Oriented Device Operations
@@ -309,8 +326,6 @@ struct rtdm_operations {
 	/** Transmit message handler for non-real-time context (optional) */
 	rtdm_sendmsg_handler_t sendmsg_nrt;
 	/** @} Message-Oriented Device Operations */
-
-	rtdm_select_bind_handler_t select_bind;
 };
 
 struct rtdm_devctx_reserved {
@@ -495,6 +510,26 @@ static inline nanosecs_abs_t rtdm_clock_
  */
 
 /*!
+ * @anchor RTDM_SELECTTYPE_xxx   @name RTDM_SELECTTYPE_xxx
+ * Event types select can bind to
+ * @{
+ */
+enum rtdm_selecttype {
+	/** Select input data availability events */
+	RTDM_SELECTTYPE_READ = XNSELECT_READ,
+
+	/** Select ouput buffer availability events */
+	RTDM_SELECTTYPE_WRITE = XNSELECT_WRITE,
+
+	/** Select exceptional events */
+	RTDM_SELECTTYPE_EXCEPT = XNSELECT_EXCEPT
+};
+/** @} RTDM_SELECTTYPE_xxx */
+
+int rtdm_select_bind(int fd, rtdm_selector_t *selector,
+		     enum rtdm_selecttype type, unsigned fd_index);
+
+/*!
  * @name Global Lock across Scheduler Invocation
  * @{
  */
@@ -1016,12 +1051,10 @@ typedef struct {
 
 void rtdm_event_init(rtdm_event_t *event, unsigned long pending);
 #ifdef CONFIG_XENO_OPT_RTDM_SELECT
-int rtdm_event_select_bind(rtdm_event_t *event,
-			   struct xnselector *selector,
-			   unsigned type,
-			   unsigned bit_index);
+int rtdm_event_select_bind(rtdm_event_t *event, rtdm_selector_t *selector,
+			   enum rtdm_selecttype type, unsigned fd_index);
 #else /* !CONFIG_XENO_OPT_RTDM_SELECT */
-#define rtdm_event_select_bind(e, s, t, b) ({ -EBADF; })
+#define rtdm_event_select_bind(e, s, t, i) ({ -EBADF; })
 #endif /* !CONFIG_XENO_OPT_RTDM_SELECT */
 int rtdm_event_wait(rtdm_event_t *event);
 int rtdm_event_timedwait(rtdm_event_t *event, nanosecs_rel_t timeout,
@@ -1057,12 +1090,10 @@ typedef struct {
 
 void rtdm_sem_init(rtdm_sem_t *sem, unsigned long value);
 #ifdef CONFIG_XENO_OPT_RTDM_SELECT
-int rtdm_sem_select_bind(rtdm_sem_t *sem,
-			 struct xnselector *selector,
-			 unsigned type,
-			 unsigned bit_index);
+int rtdm_sem_select_bind(rtdm_sem_t *sem, rtdm_selector_t *selector,
+			 enum rtdm_selecttype type, unsigned fd_index);
 #else /* !CONFIG_XENO_OPT_RTDM_SELECT */
-#define rtdm_sem_select_bind(s, se, t, b) ({ -EBADF; })
+#define rtdm_sem_select_bind(s, se, t, i) ({ -EBADF; })
 #endif /* !CONFIG_XENO_OPT_RTDM_SELECT */
 int rtdm_sem_down(rtdm_sem_t *sem);
 int rtdm_sem_timeddown(rtdm_sem_t *sem, nanosecs_rel_t timeout,
Index: b/ksrc/skins/posix/syscall.c
===================================================================
--- a/ksrc/skins/posix/syscall.c
+++ b/ksrc/skins/posix/syscall.c
@@ -1922,11 +1922,10 @@ static int select_bind_one(struct xnsele
 	pse51_assoc_t *assoc;
 #if defined(CONFIG_XENO_SKIN_RTDM) || defined (CONFIG_XENO_SKIN_RTDM_MODULE)
 	const int rtdm_fd_start = FD_SETSIZE - RTDM_FD_MAX;
-	
+
 	if (fd >= rtdm_fd_start)
-		return __rt_dev_select_bind(current,
-					    fd - rtdm_fd_start,
-					    selector, type, fd);
+		return rtdm_select_bind(fd - rtdm_fd_start,
+					selector, type, fd);
 #endif /* CONFIG_XENO_SKIN_RTDM */
 
 	assoc = pse51_assoc_lookup(&pse51_queues()->uqds, fd);
Index: b/ksrc/skins/rtdm/core.c
===================================================================
--- a/ksrc/skins/rtdm/core.c
+++ b/ksrc/skins/rtdm/core.c
@@ -294,34 +294,6 @@ err_out:
 
 EXPORT_SYMBOL(__rt_dev_socket);
 
-int __rt_dev_select_bind(rtdm_user_info_t *info, int fd,
-			 struct xnselector *selector,
-			 unsigned type, unsigned index)
-{
-	struct rtdm_dev_context *context;
-	struct rtdm_operations  *ops;
-	int ret;
-
-	context = rtdm_context_get(fd);
-
-	ret = -EBADF;
-	if (unlikely(!context))
-		goto err_out;
-
-	ops = context->ops;
-
-	ret = ops->select_bind(context, selector, type, index);
-
-	XENO_ASSERT(RTDM, !rthal_local_irq_test(), rthal_local_irq_enable(););
-
-	rtdm_context_unlock(context);
-
-  err_out:
-	return ret;
-}
-
-EXPORT_SYMBOL(__rt_dev_select_bind);
-
 int __rt_dev_close(rtdm_user_info_t *user_info, int fd)
 {
 	struct rtdm_dev_context *context;
@@ -544,6 +516,62 @@ ssize_t __rt_dev_sendmsg(rtdm_user_info_
 
 EXPORT_SYMBOL(__rt_dev_sendmsg);
 
+/**
+ * @brief Bind a selector to specified event types of a given file descriptor
+ * @internal
+ *
+ * This function is invoked by higher RTOS layers implementing select-like
+ * services. It shall not be called directly by RTDM drivers.
+ *
+ * @param[in] fd File descriptor to bind to
+ * @param[in,out] selector Selector object that shall be bound to the given
+ * event
+ * @param[in] type Event type the caller is interested in
+ * @param[in] fd_index Index in the file descriptor set of the caller
+ *
+ * @return 0 on success, otherwise:
+ *
+ * - -EBADF is returned if the file descriptor @a fd cannot be resolved.
+ *
+ * - -EINVAL is returned if @a type or @a fd_index are invalid.
+ *
+ * Environments:
+ *
+ * This service can be called from:
+ *
+ * - Kernel module initialization/cleanup code
+ * - Kernel-based task
+ * - User-space task (RT, non-RT)
+ *
+ * Rescheduling: never.
+ */
+int rtdm_select_bind(int fd, rtdm_selector_t *selector,
+		     enum rtdm_selecttype type, unsigned fd_index)
+{
+	struct rtdm_dev_context *context;
+	struct rtdm_operations  *ops;
+	int ret;
+
+	context = rtdm_context_get(fd);
+
+	ret = -EBADF;
+	if (unlikely(!context))
+		goto err_out;
+
+	ops = context->ops;
+
+	ret = ops->select_bind(context, selector, type, fd_index);
+
+	XENO_ASSERT(RTDM, !rthal_local_irq_test(), rthal_local_irq_enable(););
+
+	rtdm_context_unlock(context);
+
+  err_out:
+	return ret;
+}
+
+EXPORT_SYMBOL(rtdm_select_bind);
+
 #ifdef DOXYGEN_CPP /* Only used for doxygen doc generation */
 
 /**
Index: b/ksrc/skins/rtdm/drvlib.c
===================================================================
--- a/ksrc/skins/rtdm/drvlib.c
+++ b/ksrc/skins/rtdm/drvlib.c
@@ -4,6 +4,7 @@
  *
  * @note Copyright (C) 2005-2007 Jan Kiszka <jan.kiszka@domain.hid>
  * @note Copyright (C) 2005 Joerg Langenberg <joerg.langenberg@domain.hid>
+ * @note Copyright (C) 2008 Gilles Chanteperdrix <gilles.chanteperdrix@xenomai.orgmail.com>
  *
  * Xenomai is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by
@@ -764,32 +765,6 @@ void rtdm_event_init(rtdm_event_t *event
 
 EXPORT_SYMBOL(rtdm_event_init);
 
-#ifdef CONFIG_XENO_OPT_RTDM_SELECT
-int rtdm_event_select_bind(rtdm_event_t *event,
-			   struct xnselector *selector,
-			   unsigned type,
-			   unsigned bit_index)
-{
-	struct xnselect_binding *binding;
-	int err;
-	spl_t s;
-
-	binding = xnmalloc(sizeof(*binding));
-	if (!binding)
-		return -ENOMEM;
-
-	xnlock_get_irqsave(&nklock, s);
-	err = xnselect_bind(&event->select_block,
-			    binding, selector, type,bit_index,
-			    xnsynch_test_flags(&event->synch_base,
-					       RTDM_EVENT_PENDING));
-	xnlock_put_irqrestore(&nklock, s);
-
-	return err;
-}
-EXPORT_SYMBOL(rtdm_event_select_bind);
-#endif /* CONFIG_XENO_OPT_RTDM_SELECT */
-
 #ifdef DOXYGEN_CPP /* Only used for doxygen doc generation */
 /**
  * @brief Destroy an event
@@ -1036,6 +1011,65 @@ void rtdm_event_clear(rtdm_event_t *even
 }
 
 EXPORT_SYMBOL(rtdm_event_clear);
+
+#ifdef CONFIG_XENO_OPT_RTDM_SELECT
+/**
+ * @brief Bind a selector to an event
+ *
+ * This functions binds the given selector to an event so that the former is
+ * notified when the event state changes. Typically the select binding handler
+ * will invoke this service.
+ *
+ * @param[in,out] event Event handle as returned by rtdm_event_init()
+ * @param[in,out] selector Selector as passed to the select binding handler
+ * @param[in] type Type of the bound event as passed to the select binding handler
+ * @param[in] fd_index File descriptor index as passed to the select binding
+ * handler
+ *
+ * @return 0 on success, otherwise:
+ *
+ * - -EIDRM is returned if @a event has been destroyed.
+ *
+ * - -ENOMEM is returned if there is insufficient memory to establish the
+ * dynamic binding.
+ *
+ * - -EINVAL is returned if @a type or @a fd_index are invalid.
+ *
+ * Environments:
+ *
+ * This service can be called from:
+ *
+ * - Kernel module initialization/cleanup code
+ * - Kernel-based task
+ * - User-space task (RT, non-RT)
+ *
+ * Rescheduling: never.
+ */
+int rtdm_event_select_bind(rtdm_event_t *event, rtdm_selector_t *selector,
+			   enum rtdm_selecttype type, unsigned fd_index)
+{
+	struct xnselect_binding *binding;
+	int err;
+	spl_t s;
+
+	binding = xnmalloc(sizeof(*binding));
+	if (!binding)
+		return -ENOMEM;
+
+	xnlock_get_irqsave(&nklock, s);
+	if (unlikely(testbits(event->synch_base.status, RTDM_SYNCH_DELETED)))
+		err = -EIDRM;
+	else
+		err = xnselect_bind(&event->select_block,
+				    binding, selector, type, fd_index,
+				    xnsynch_test_flags(&event->synch_base,
+						       RTDM_EVENT_PENDING));
+	xnlock_put_irqrestore(&nklock, s);
+
+	return err;
+}
+EXPORT_SYMBOL(rtdm_event_select_bind);
+#endif /* CONFIG_XENO_OPT_RTDM_SELECT */
 /** @} */
 
 /*!
@@ -1077,30 +1111,6 @@ void rtdm_sem_init(rtdm_sem_t *sem, unsi
 
 EXPORT_SYMBOL(rtdm_sem_init);
 
-#ifdef CONFIG_XENO_OPT_RTDM_SELECT
-int rtdm_sem_select_bind(rtdm_sem_t *sem,
-			 struct xnselector *selector,
-			 unsigned type,
-			 unsigned bit_index)
-{
-	struct xnselect_binding *binding;
-	int err;
-	spl_t s;
-
-	binding = xnmalloc(sizeof(*binding));
-	if (!binding)
-		return -ENOMEM;
-
-	xnlock_get_irqsave(&nklock, s);
-	err = xnselect_bind(&sem->select_block,
-			    binding, selector, type, bit_index, sem->value > 0);
-	xnlock_put_irqrestore(&nklock, s);
-
-	return err;
-}
-EXPORT_SYMBOL(rtdm_sem_select_bind);
-#endif /* CONFIG_XENO_OPT_RTDM_SELECT */
-
 #ifdef DOXYGEN_CPP /* Only used for doxygen doc generation */
 /**
  * @brief Destroy a semaphore
@@ -1207,7 +1217,7 @@ int rtdm_sem_timeddown(rtdm_sem_t *sem, 
 
 	xnlock_get_irqsave(&nklock, s);
 
-	if (testbits(sem->synch_base.status, RTDM_SYNCH_DELETED))
+	if (unlikely(testbits(sem->synch_base.status, RTDM_SYNCH_DELETED)))
 		err = -EIDRM;
 	else if (sem->value > 0) {
 		if(!--sem->value)
@@ -1284,6 +1294,63 @@ void rtdm_sem_up(rtdm_sem_t *sem)
 }
 
 EXPORT_SYMBOL(rtdm_sem_up);
+
+#ifdef CONFIG_XENO_OPT_RTDM_SELECT
+/**
+ * @brief Bind a selector to a semaphore
+ *
+ * This functions binds the given selector to the semaphore so that the former
+ * is notified when the semaphore state changes. Typically the select binding
+ * handler will invoke this service.
+ *
+ * @param[in,out] sem Semaphore handle as returned by rtdm_sem_init()
+ * @param[in,out] selector Selector as passed to the select binding handler
+ * @param[in] type Type of the bound event as passed to the select binding handler
+ * @param[in] fd_index File descriptor index as passed to the select binding
+ * handler
+ *
+ * @return 0 on success, otherwise:
+ *
+ * - -EIDRM is returned if @a sem has been destroyed.
+ *
+ * - -ENOMEM is returned if there is insufficient memory to establish the
+ * dynamic binding.
+ *
+ * - -EINVAL is returned if @a type or @a fd_index are invalid.
+ *
+ * Environments:
+ *
+ * This service can be called from:
+ *
+ * - Kernel module initialization/cleanup code
+ * - Kernel-based task
+ * - User-space task (RT, non-RT)
+ *
+ * Rescheduling: never.
+ */
+int rtdm_sem_select_bind(rtdm_sem_t *sem, rtdm_selector_t *selector,
+			 enum rtdm_selecttype type, unsigned fd_index)
+{
+	struct xnselect_binding *binding;
+	int err;
+	spl_t s;
+
+	binding = xnmalloc(sizeof(*binding));
+	if (!binding)
+		return -ENOMEM;
+
+	xnlock_get_irqsave(&nklock, s);
+	if (unlikely(testbits(sem->synch_base.status, RTDM_SYNCH_DELETED)))
+		err = -EIDRM;
+	else
+		err = xnselect_bind(&sem->select_block, binding, selector,
+				    type, fd_index, (sem->value > 0));
+	xnlock_put_irqrestore(&nklock, s);
+
+	return err;
+}
+EXPORT_SYMBOL(rtdm_sem_select_bind);
+#endif /* CONFIG_XENO_OPT_RTDM_SELECT */
 /** @} */
 
 /*!
Index: b/doc/doxygen/Doxyfile-common.in
===================================================================
--- a/doc/doxygen/Doxyfile-common.in
+++ b/doc/doxygen/Doxyfile-common.in
@@ -691,6 +691,7 @@ PREDEFINED             = DOXYGEN_CPP \
         CONFIG_XENO_OPT_RTAI_FIFO \
         CONFIG_XENO_OPT_RTAI_SEM \
         CONFIG_XENO_OPT_RTAI_SHM \
+        CONFIG_XENO_OPT_RTDM_SELECT \
         CONFIG_GENERIC_CLOCKEVENTS
 
 # If the MACRO_EXPANSION and EXPAND_ONLY_PREDEF tags are set to YES then 
Index: b/ChangeLog
===================================================================
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2008-03-28  Jan Kiszka  <jan.kiszka@domain.hid>
+
+	* include/rtdm, ksrc/skins/rtdm: Refactor and document select API.
+	Finally increment API version.
+
 2008-03-24  Stephane Fillod  <stephane.fillod@domain.hid>
 
 	* ksrc/skins/native/heap.c: Output oustanding number of heap

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 254 bytes --]

Re: [Xenomai-core] [PATCH] RTDM: refactor and document select API extension

[Gilles Chanteperdrix]

Jan Kiszka wrote:
 > Hi Gilles,
 > 
 > unfortunately I missed (or more likely forgot) that the RTDM select 
 > extensions already made it into stable and thus now also into a
 > release.

My bad, I did not tell publically that I was going to do it.

 > However, here is an overdue cleanup and documentation patch. Please 
 > check if I happen to have written nonsense or broke something (only 
 > compile-tested). Don't hesitate to suggest better names for types, 
 > fields etc. if you find any.

I am Ok with the patch. Please commit.

-- 


					    Gilles.