[Xenomai-core] [patch 0/4] Support for select-like services.

[Xenomai-core] [patch 0/4] Support for select-like services.

[Gilles Chanteperdrix]

Hi,

here comes a third edition of the patchset adding support for select. 

The core support now depends on a CONFIG_XENO_OPT_SELECT option, whereas the
posix support depends on a CONFIG_XENO_OPT_POSIX_SELECT option.

The support for using rtdm_event_t and rtdm_sem_t objects is now built-in to
these objects without creating new objects types (only if
CONFIG_XENO_OPT_SELECT is enabled).

The rtdm_context_get modification, allowing to check that the caller of a service
belongs to the same process as the file descriptor it uses, is now made in
__rtdm_context_get while rtdm_context_get passes a NULL first argument to this
new service.

When implementing these modifications, a few questions arise:
- RTDM support currently depends on the CONFIG_XENO_OPT_SELECT option, should we
add an RTDM specific option ?
- in order to avoid numerous #ifdefs, and especially in drivers code, the new
callback added to device operations for binding RTDM file descriptors to
selector blocks is always there even if CONFIG_XENO_OPT_SELECT is disabled. Is
it the right thing to do ?

Regards.


-- 


					    Gilles Chanteperdrix.

[Xenomai-core] [patch 1/4] nucleus core support for select-like services.

[Gilles Chanteperdrix]

[-- Attachment #1: message body and .signature --]
[-- Type: text/plain, Size: 277 bytes --]


stats:
 include/nucleus/select.h |  124 ++++++++++++++
 ksrc/nucleus/Kconfig     |    3
 ksrc/nucleus/Makefile    |    3
 ksrc/nucleus/select.c    |  412 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 542 insertions(+)

-- 


					    Gilles Chanteperdrix.

[-- Attachment #2: xeno-select-core.diff --]
[-- Type: text/plain, Size: 16593 bytes --]

--- include/nucleus/select.h	(revision 0)
+++ include/nucleus/select.h	(revision 0)
@@ -0,0 +1,124 @@
+/*!\file select.h
+ * \brief file descriptors events multiplexing header.
+ * \author Gilles Chanteperdrix
+ *
+ * Copyright (C) 2008 Efixo <gilles.chanteperdrix@xenomai.org>
+ *
+ * Xenomai is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; either version 2 of the License,
+ * or (at your option) any later version.
+ *
+ * Xenomai is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Xenomai; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
+ * 02111-1307, USA.
+ *
+ * \ingroup select
+ */
+
+#ifndef XNSELECT_H
+#define XNSELECT_H
+
+/*! \addtogroup select
+ *@{*/
+
+#include <nucleus/thread.h>
+
+#define XNSELECT_READ      0
+#define XNSELECT_WRITE     1
+#define XNSELECT_EXCEPT    2
+#define XNSELECT_MAX_TYPES 3
+
+struct xnselector { 
+        xnsynch_t synchbase;
+	struct fds {
+		fd_set expected;
+		fd_set pending;
+	} fds [XNSELECT_MAX_TYPES];
+	xnqueue_t bindings; /* only used by xnselector_destroy */
+};
+
+#ifdef CONFIG_XENO_OPT_SELECT
+
+struct xnselect {
+	xnqueue_t bindings;
+};
+
+#define DECLARE_XNSELECT(name) struct xnselect name
+
+struct xnselect_binding {
+	struct xnselector *selector;
+	struct xnselect *fd;
+	unsigned type;
+	unsigned bit_index;
+	xnholder_t link;  /* link in selected fds list. */
+	xnholder_t slink; /* link in selector list */
+};
+
+#ifdef __cplusplus
+extern "C" {
+#endif /* __cplusplus */
+
+void xnselect_init(struct xnselect *select_block);
+
+int xnselect_bind(struct xnselect *select_block,
+		  struct xnselect_binding *binding,
+		  struct xnselector *selector,
+		  unsigned type,
+		  unsigned bit_index,
+		  unsigned state);
+
+int __xnselect_signal(struct xnselect *select_block, unsigned state);
+
+/** 
+ * Signal a file descriptor state change.
+ * 
+ * @param select_block pointer to an @a xnselect structure representing the file
+ * descriptor whose state changed;
+ * @param state new value of the state.
+ * 
+ * @retval 1 if rescheduling is needed;
+ * @retval 0 otherwise.
+ */
+static inline int
+xnselect_signal(struct xnselect *select_block, unsigned state)
+{
+	if (!emptyq_p(&select_block->bindings))
+		return __xnselect_signal(select_block, state);
+	return 0;
+}
+
+void xnselect_destroy(struct xnselect *select_block);
+
+int xnselector_init(struct xnselector *selector);
+
+int xnselect(struct xnselector *selector,
+	     fd_set *out_fds[XNSELECT_MAX_TYPES],
+	     fd_set *in_fds[XNSELECT_MAX_TYPES],
+	     int nfds,
+	     xnticks_t timeout, xntmode_t timeout_mode);
+
+void xnselector_destroy(struct xnselector *selector);
+
+#ifdef __cplusplus
+}
+#endif /* __cplusplus */
+
+#else /* !CONFIG_XENO_OPT_SELECT */
+struct xnselector;
+#define DECLARE_XNSELECT(name)
+#define xnselect_init(block)
+#define xnselect_bind(select_block,binding,selector,type,bit_index,state) -EBADF
+#define xnselect_signal(block, state) ({ 0; })
+#define xnselect_destroy(block)
+#endif /* !CONFIG_XENO_OPT_SELECT */
+
+/*@}*/
+
+#endif /* XNSELECT_H */
--- ksrc/nucleus/Kconfig	(revision 3460)
+++ ksrc/nucleus/Kconfig	(working copy)
@@ -228,6 +228,9 @@ config XENO_OPT_SHIRQ
 	are allowed to control dedicated hardware devices which are
 	configured to share the same interrupt line.
 
+config XENO_OPT_SELECT
+	bool
+
 menu "Timing"
 
 config XENO_OPT_TIMING_PERIODIC
--- ksrc/nucleus/select.c	(revision 0)
+++ ksrc/nucleus/select.c	(revision 0)
@@ -0,0 +1,412 @@
+/*!\file select.c
+ * \brief file descriptors events multiplexing.
+ * \author Gilles Chanteperdrix
+ *
+ * Copyright (C) 2008 Efixo <gilles.chanteperdrix@xenomai.org>
+ *
+ * Xenomai is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; either version 2 of the License,
+ * or (at your option) any later version.
+ *
+ * Xenomai is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Xenomai; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
+ * 02111-1307, USA.
+ *
+ * \ingroup select
+ */
+
+/*!
+ * \ingroup nucleus
+ * \defgroup select File descriptors events multiplexing services.
+ *
+ * File descriptors events multiplexing services.
+ *
+ * This module implements the services needed for implementing the posix
+ * "select" service, or any other events multiplexing services.
+ *
+ * Following the implementation of the posix select service, this module defines
+ * three types of events:
+ * - \a XNSELECT_READ meaning that a file descriptor is ready for reading;
+ * - \a XNSELECT_WRITE meaning that a file descriptor is ready for writing;
+ * - \a XNSELECT_EXCEPT meaning that a file descriptor received an exceptional
+ *   event.
+ *
+ * It works by defining two structures:
+ * - a @a struct @a xnselect structure, which should be added to every file
+ * descriptor for every event type (read, write, or except);
+ * - a @a struct @a xnselector structure, the selection structure,  passed by
+ * the thread calling the xnselect service, where this service does all its
+ * housekeeping.
+ *@{*/
+
+#include <nucleus/heap.h>
+#include <nucleus/pod.h>
+#include <nucleus/synch.h>
+#include <nucleus/select.h>
+#include <linux/types.h>
+#include <linux/bitops.h>	/* For hweight_long */
+
+#define link2binding(baddr, memb)				\
+	container_of(baddr, struct xnselect_binding, memb)
+
+/** 
+ * Initialize a @a struct @a xnselect structure.
+ *
+ * This service must be called to initialize a @a struct @a xnselect structure
+ * before it is bound to a selector by the means of xnselect_bind().
+ * 
+ * @param select_block pointer to the xnselect structure to be initialized
+ */
+void xnselect_init(struct xnselect *select_block)
+{
+	initq(&select_block->bindings);
+}
+EXPORT_SYMBOL(xnselect_init);
+
+static inline int xnselect_wakeup(struct xnselector *selector)
+{
+	return xnsynch_flush(&selector->synchbase, 0) == XNSYNCH_RESCHED;
+}
+
+/** 
+ * Bind a file descriptor (represented by its @a xnselect structure) to a
+ * selector block.
+ *
+ * @param select_block pointer to the @a struct @a xnselect to be bound;
+ *
+ * @param binding pointer to a newly allocated (using xnmalloc) @a struct
+ * @a xnselect_binding;
+ *
+ * @param selector pointer to the selector structure;
+ *
+ * @param type type of events (@a XNSELECT_READ, @a XNSELECT_WRITE, or @a
+ * XNSELECT_EXCEPT);
+ *
+ * @param index index of the file descriptor (represented by @a select_block) in the bit fields used by the @a selector structure;
+ *
+ * @param state current state of the file descriptor>.
+ * 
+ * @a select_block must have been initialized with xnselect_init(),
+ * the @a xnselector structure must have been initialized with
+ * xnselector_init(), @a binding may be uninitialized.
+ *
+ * This service must be called with nklock locked, irqs off. For this reason,
+ * the @a binding parameter must have been allocated by the caller outside the
+ * locking section.
+ * 
+ * @retval -EINVAL if @a type or @a index is invalid;
+ * @retval 0 otherwise.
+ */
+int xnselect_bind(struct xnselect *select_block,
+		  struct xnselect_binding *binding,
+		  struct xnselector *selector,
+		  unsigned type,
+		  unsigned index,
+		  unsigned state)
+{
+	if (type >= XNSELECT_MAX_TYPES || index > __FD_SETSIZE)
+		return -EINVAL;
+
+	binding->selector = selector;
+	binding->fd = select_block;
+	binding->type = type;
+	binding->bit_index = index;
+	inith(&binding->link);
+	inith(&binding->slink);
+	
+	appendq(&selector->bindings, &binding->slink);
+	appendq(&select_block->bindings, &binding->link);
+	__FD_SET(index, &selector->fds[type].expected);
+	if (state)
+		__FD_SET(index, &selector->fds[type].pending);
+
+	return 0;
+}
+EXPORT_SYMBOL(xnselect_bind);
+
+/* Must be called with nklock locked irqs off */
+int __xnselect_signal(struct xnselect *select_block, unsigned state)
+{
+	xnholder_t *holder;
+	int resched;
+
+	for(resched = 0, holder = getheadq(&select_block->bindings);
+	    holder; holder = nextq(&select_block->bindings, holder)) {
+		struct xnselect_binding *binding;
+		struct xnselector *selector;
+
+		binding = link2binding(holder, link);
+
+		selector = binding->selector;
+		if (state) {
+			if (!__FD_ISSET(binding->bit_index,
+					&selector->fds[binding->type].pending)) {
+				__FD_SET(binding->bit_index,
+					 &selector->fds[binding->type].pending);
+				if (xnselect_wakeup(selector))
+					resched = 1;
+			}
+		} else
+			__FD_CLR(binding->bit_index, 
+				 &selector->fds[binding->type].pending);
+	}
+
+	return resched;
+}
+EXPORT_SYMBOL(__xnselect_signal);
+
+/** 
+ * Destroy the @a xnselect structure associated with a file descriptor.
+ *
+ * Any binding with a @a xnselector block is destroyed.
+ *
+ * @param select_block pointer to the @a xnselect structure associated with a file descriptor
+ */
+void xnselect_destroy(struct xnselect *select_block)
+{
+	xnholder_t *holder;
+	spl_t s;
+
+	xnlock_get_irqsave(&nklock, s);
+	while ((holder = getq(&select_block->bindings))) {
+		struct xnselect_binding *binding;
+		struct xnselector *selector;
+
+		binding = link2binding(holder, link);
+		selector = binding->selector;
+
+		__FD_CLR(binding->bit_index,
+			 &selector->fds[binding->type].expected);
+		__FD_CLR(binding->bit_index,
+			 &selector->fds[binding->type].pending);
+		removeq(&selector->bindings, &binding->slink);
+		xnlock_put_irqrestore(&nklock, s);
+
+		xnfree(binding);
+		
+		xnlock_get_irqsave(&nklock, s);
+	}
+	xnlock_put_irqrestore(&nklock, s);
+}
+EXPORT_SYMBOL(xnselect_destroy);
+
+static unsigned
+fd_set_andnot(fd_set *result, fd_set *first, fd_set *second, unsigned n)
+{
+	unsigned i, not_empty = 0;
+
+	for (i = 0; i < __FDELT(n); i++)
+		if((result->fds_bits[i] =
+		    first->fds_bits[i] & ~(second->fds_bits[i])))
+			not_empty = 1;
+
+	if (i < __FDSET_LONGS
+	    && (result->fds_bits[i] =
+		first->fds_bits[i] & ~(second->fds_bits[i]) & (__FDMASK(n) - 1)))
+		not_empty = 1;
+
+	return not_empty;
+}
+
+static unsigned
+fd_set_and(fd_set *result, fd_set *first, fd_set *second, unsigned n)
+{
+	unsigned i, not_empty = 0;
+
+	for (i = 0; i < __FDELT(n); i++)
+		if((result->fds_bits[i] =
+		    first->fds_bits[i] & second->fds_bits[i]))
+			not_empty = 1;
+
+	if (i < __FDSET_LONGS
+	    && (result->fds_bits[i] =
+		first->fds_bits[i] & second->fds_bits[i] & (__FDMASK(n) - 1)))
+		not_empty = 1;
+
+	return not_empty;
+}
+
+static void fd_set_zerofill(fd_set *set, unsigned n)
+{
+	unsigned i;
+
+	i = __FDELT(n);
+
+	if (i < __FDSET_LONGS)
+		set->fds_bits[i] &= (__FDMASK(n) - 1);
+
+	for(i++; i < __FDSET_LONGS; i++)
+		set->fds_bits[i] = 0;
+}
+
+static unsigned fd_set_popcount(fd_set *set, unsigned n)
+{
+	unsigned count = 0, i;
+
+	for (i = 0; i < __FDELT(n); i++)
+		if (set->fds_bits[i])
+			count += hweight_long(set->fds_bits[i]);
+
+	if (i < __FDSET_LONGS && (set->fds_bits[i] & (__FDMASK(n) - 1)))
+		count += hweight_long(set->fds_bits[i] & (__FDMASK(n) - 1));
+
+	return count;
+}
+
+/** 
+ * Initialize a selector structure.
+ * 
+ * @param selector The selector structure to be initialized.
+ * 
+ * @retval 0
+ */
+int xnselector_init(struct xnselector *selector)
+{
+	unsigned i;
+
+	xnsynch_init(&selector->synchbase, XNSYNCH_FIFO | XNSYNCH_NOPIP);
+	for (i = 0; i < XNSELECT_MAX_TYPES; i++) {
+		__FD_ZERO(&selector->fds[i].expected);
+		__FD_ZERO(&selector->fds[i].pending);
+	}
+	initq(&selector->bindings);
+	return 0;
+}
+EXPORT_SYMBOL(xnselector_init);
+
+/** 
+ * Check the state of a number of file descriptors, wait for a state change if
+ * no descriptor is ready.
+ * 
+ * @param selector structure to check for pending events
+ * @param out_fds The set of descriptors with pending events if a strictly positive number is returned, or the set of descriptors not yet bound if -ECHRNG is returned;
+ * @param in_fds the set of descriptors which events should be checked
+ * @param nfds the highest-numbered descriptor in any of the @a in_fds sets, plus 1;
+ * @param timeout the timeout, whose meaning depends on @a timeout_mode, note
+ * that xnselect() pass @a timeout and @a timeout_mode unchanged to
+ * xnsynch_sleep_on, so passing a relative value different from XN_INFINITE as a
+ * timeout with @a timeout_mode set to XN_RELATIVE, will cause a longer sleep
+ * than expected if the sleep is interrupted.
+ * @param timeout_mode the mode of @a timeout.
+ * 
+ * @retval -EINVAL if @nfds is negative;
+ * @retval -ECHRNG if some of the descriptors passed in @a in_fds have not yet
+ * been registered with xnselect_bind(), @a out_fds contains the set of such
+ * descriptors;
+ * @retval -EINTR if @a xnselect was interrupted while waiting;
+ * @retval 0 in case of timeout.
+ * @retval the number of file descriptors having received an event.
+ */
+int xnselect(struct xnselector *selector,
+	     fd_set *out_fds[XNSELECT_MAX_TYPES],
+	     fd_set *in_fds[XNSELECT_MAX_TYPES],
+	     int nfds,
+	     xnticks_t timeout, xntmode_t timeout_mode)
+{
+	unsigned i, not_empty = 0;
+	xnthread_t *thread;
+	spl_t s;
+
+	if ((unsigned) nfds > __FD_SETSIZE)
+		return -EINVAL;
+
+	thread = xnpod_current_thread();	
+
+	xnlock_get_irqsave(&nklock, s);
+	for (i = 0; i < XNSELECT_MAX_TYPES; i++)
+		if (out_fds[i]
+		    && fd_set_andnot(out_fds[i], in_fds[i],
+				     &selector->fds[i].expected, nfds))
+			not_empty = 1;
+	xnlock_put_irqrestore(&nklock, s);
+
+	if (not_empty) {
+		for (i = 0; i < XNSELECT_MAX_TYPES; i++)
+			if (out_fds[i])
+				fd_set_zerofill(out_fds[i], nfds);
+		return -ECHRNG;
+	}
+
+	xnlock_get_irqsave(&nklock, s);
+	for (i = 0; i < XNSELECT_MAX_TYPES; i++)
+		if (out_fds[i]
+		    && fd_set_and(out_fds[i], in_fds[i],
+				  &selector->fds[i].pending, nfds))
+			not_empty = 1;
+	
+	while (!not_empty) {
+		xnsynch_sleep_on(&selector->synchbase, timeout, timeout_mode);
+
+		for (i = 0; i < XNSELECT_MAX_TYPES; i++)
+			if (out_fds[i]
+			    && fd_set_and(out_fds[i], in_fds[i],
+					  &selector->fds[i].pending, nfds))
+				not_empty = 1;
+
+		if (xnthread_test_info(thread, XNBREAK | XNTIMEO))
+			break;
+	}
+	xnlock_put_irqrestore(&nklock, s);
+
+	if (not_empty) {
+		unsigned count;
+
+		for (i = 0; i < XNSELECT_MAX_TYPES; i++)
+			if (out_fds[i])
+				fd_set_zerofill(out_fds[i], nfds);
+
+		for (count = 0, i = 0; i < XNSELECT_MAX_TYPES; i++)
+			if (out_fds[i])
+				count += fd_set_popcount(out_fds[i], nfds);
+
+		return count;
+	}
+
+	if (xnthread_test_info(thread, XNBREAK))
+		return -EINTR;
+
+	return 0; /* Timeout */
+}
+EXPORT_SYMBOL(xnselect);
+
+/** 
+ * Destroy a selector block.
+ *
+ * All bindings with file descriptor are destroyed.
+ *
+ * @param selector the selector block to be destroyed
+ */
+void xnselector_destroy(struct xnselector *selector)
+{
+	xnholder_t *holder;
+	spl_t s;
+
+	xnlock_get_irqsave(&nklock, s);
+	while ((holder = getq(&selector->bindings))) {
+		struct xnselect_binding *binding;
+		struct xnselect *fd;
+
+		binding = link2binding(holder, slink);
+		fd = binding->fd;
+		removeq(&fd->bindings, &binding->link);
+		xnlock_put_irqrestore(&nklock, s);
+
+		xnfree(binding);
+
+		xnlock_get_irqsave(&nklock, s);
+	}
+	xnlock_put_irqrestore(&nklock, s);
+
+	if (xnsynch_destroy(&selector->synchbase) == XNSYNCH_RESCHED)
+		xnpod_schedule();
+}
+EXPORT_SYMBOL(xnselector_destroy);
+
+/*@}*/
--- ksrc/nucleus/Makefile	(revision 3460)
+++ ksrc/nucleus/Makefile	(working copy)
@@ -14,6 +14,8 @@ xeno_nucleus-$(CONFIG_XENO_OPT_MAP) += m
 
 xeno_nucleus-$(CONFIG_XENO_OPT_REGISTRY) += registry.o
 
+xeno_nucleus-$(CONFIG_XENO_OPT_SELECT) += select.o
+
 EXTRA_CFLAGS += -D__IN_XENOMAI__ -Iinclude/xenomai
 
 else
@@ -33,6 +35,7 @@ opt_objs-$(CONFIG_XENO_OPT_PERVASIVE) +=
 opt_objs-$(CONFIG_XENO_OPT_PIPE) += pipe.o
 opt_objs-$(CONFIG_XENO_OPT_MAP) += map.o
 opt_objs-$(CONFIG_XENO_OPT_REGISTRY) += registry.o
+opt_objs-$(CONFIG_XENO_OPT_SELECT) += select.o
 
 xeno_nucleus-objs += $(opt_objs-y)
 

Re: [Xenomai-core] [patch 2/4] RTDM support for select-like service.

[Gilles Chanteperdrix]

[-- Attachment #1: message body and .signature --]
[-- Type: text/plain, Size: 387 bytes --]


stats:
 include/rtdm/rtdm.h        |    4 ++
 include/rtdm/rtdm_driver.h |   30 +++++++++++++++-
 ksrc/skins/rtdm/core.c     |   84 +++++++++++++++++++++++++++++++--------------
 ksrc/skins/rtdm/device.c   |   10 +++++
 ksrc/skins/rtdm/drvlib.c   |   76 ++++++++++++++++++++++++++++++++++++----
 5 files changed, 170 insertions(+), 34 deletions(-)

-- 


					    Gilles Chanteperdrix.

[-- Attachment #2: xeno-select-rtdm.diff --]
[-- Type: text/plain, Size: 12681 bytes --]

--- include/rtdm/rtdm_driver.h	(revision 3460)
+++ include/rtdm/rtdm_driver.h	(working copy)
@@ -38,6 +38,7 @@
 #include <nucleus/heap.h>
 #include <nucleus/pod.h>
 #include <nucleus/synch.h>
+#include <nucleus/select.h>
 #include <rtdm/rtdm.h>
 
 /* debug support */
@@ -262,6 +263,10 @@ 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
  */
@@ -304,6 +309,8 @@ 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 {
@@ -453,7 +460,8 @@ int rtdm_dev_unregister(struct rtdm_devi
 #define rtdm_getpeername	rt_dev_getpeername
 #define rtdm_shutdown		rt_dev_shutdown
 
-struct rtdm_dev_context *rtdm_context_get(int fd);
+struct rtdm_dev_context *__rtdm_context_get(rtdm_user_info_t *user_info, int fd);
+#define rtdm_context_get(fd) __rtdm_context_get(NULL, fd)
 
 #ifndef DOXYGEN_CPP /* Avoid static inline tags for RTDM in doxygen */
 static inline void rtdm_context_lock(struct rtdm_dev_context *context)
@@ -1002,11 +1010,20 @@ void rtdm_toseq_init(rtdm_toseq_t *timeo
 
 typedef struct {
 	xnsynch_t synch_base;
+	DECLARE_XNSELECT(select_block);
 } rtdm_event_t;
 
 #define RTDM_EVENT_PENDING		XNSYNCH_SPARE1
 
 void rtdm_event_init(rtdm_event_t *event, unsigned long pending);
+#ifdef CONFIG_XENO_OPT_SELECT
+int rtdm_event_select_bind(rtdm_event_t *event,
+			   struct xnselector *selector,
+			   unsigned type,
+			   unsigned bit_index);
+#else /* !CONFIG_XENO_OPT_SELECT */
+#define rtdm_event_select_bind(e, s, t, b) -EBADF
+#endif /* !CONFIG_XENO_OPT_SELECT */
 int rtdm_event_wait(rtdm_event_t *event);
 int rtdm_event_timedwait(rtdm_event_t *event, nanosecs_rel_t timeout,
 			 rtdm_toseq_t *timeout_seq);
@@ -1027,6 +1044,7 @@ static inline void rtdm_event_destroy(rt
 {
 	trace_mark(xn_rtdm_event_destroy, "event %p", event);
 	__rtdm_synch_flush(&event->synch_base, XNRMID);
+	xnselect_destroy(&event->select_block);
 }
 #endif /* !DOXYGEN_CPP */
 
@@ -1035,9 +1053,18 @@ static inline void rtdm_event_destroy(rt
 typedef struct {
 	unsigned long value;
 	xnsynch_t synch_base;
+	DECLARE_XNSELECT(select_block);
 } rtdm_sem_t;
 
 void rtdm_sem_init(rtdm_sem_t *sem, unsigned long value);
+#ifdef CONFIG_XENO_OPT_SELECT
+int rtdm_sem_select_bind(rtdm_sem_t *sem,
+			 struct xnselector *selector,
+			 unsigned type,
+			 unsigned bit_index);
+#else /* !CONFIG_XENO_OPT_SELECT */
+#define rtdm_sem_select_bind(s, se, t, b) -EBADF
+#endif /* !CONFIG_XENO_OPT_SELECT */
 int rtdm_sem_down(rtdm_sem_t *sem);
 int rtdm_sem_timeddown(rtdm_sem_t *sem, nanosecs_rel_t timeout,
 		       rtdm_toseq_t *timeout_seq);
@@ -1048,6 +1075,7 @@ static inline void rtdm_sem_destroy(rtdm
 {
 	trace_mark(xn_rtdm_sem_destroy, "sem %p", sem);
 	__rtdm_synch_flush(&sem->synch_base, XNRMID);
+	xnselect_destroy(&sem->select_block);
 }
 #endif /* !DOXYGEN_CPP */
 
--- include/rtdm/rtdm.h	(revision 3460)
+++ include/rtdm/rtdm.h	(working copy)
@@ -247,6 +247,10 @@ 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
--- ksrc/skins/rtdm/device.c	(revision 3460)
+++ ksrc/skins/rtdm/device.c	(working copy)
@@ -70,6 +70,14 @@ int rtdm_no_support(void)
 	return -ENOSYS;
 }
 
+int rtdm_select_bind_no_support(struct rtdm_dev_context *context,
+				struct xnselector *selector,
+				unsigned type,
+				unsigned index)
+{
+	return -EBADF;
+}
+  
 static inline int get_name_hash(const char *str, int limit, int hashkey_mask)
 {
 	int hash = 0;
@@ -242,6 +250,8 @@ int rtdm_dev_register(struct rtdm_device
 	SET_DEFAULT_OP_IF_NULL(device->ops, write);
 	SET_DEFAULT_OP_IF_NULL(device->ops, recvmsg);
 	SET_DEFAULT_OP_IF_NULL(device->ops, sendmsg);
+	if (!device->ops.select_bind)
+		device->ops.select_bind = rtdm_select_bind_no_support;
 
 	atomic_set(&device->reserved.refcount, 0);
 	device->reserved.exclusive_context = NULL;
--- ksrc/skins/rtdm/drvlib.c	(revision 3460)
+++ ksrc/skins/rtdm/drvlib.c	(working copy)
@@ -756,12 +756,39 @@ void rtdm_event_init(rtdm_event_t *event
 	xnsynch_init(&event->synch_base, XNSYNCH_PRIO);
 	if (pending)
 		xnsynch_set_flags(&event->synch_base, RTDM_EVENT_PENDING);
+	xnselect_init(&event->select_block);
 
 	xnlock_put_irqrestore(&nklock, s);
 }
 
 EXPORT_SYMBOL(rtdm_event_init);
 
+#ifdef CONFIG_XENO_OPT_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_SELECT */
+
 #ifdef DOXYGEN_CPP /* Only used for doxygen doc generation */
 /**
  * @brief Destroy an event
@@ -825,6 +852,7 @@ void rtdm_event_pulse(rtdm_event_t *even
  */
 void rtdm_event_signal(rtdm_event_t *event)
 {
+	int resched = 0;
 	spl_t s;
 
 	trace_mark(xn_rtdm_event_signal, "event %p", event);
@@ -833,6 +861,10 @@ void rtdm_event_signal(rtdm_event_t *eve
 
 	xnsynch_set_flags(&event->synch_base, RTDM_EVENT_PENDING);
 	if (xnsynch_flush(&event->synch_base, 0))
+		resched = 1;
+	if (xnselect_signal(&event->select_block, 1))
+		resched = 1;
+	if (resched)
 		xnpod_schedule();
 
 	xnlock_put_irqrestore(&nklock, s);
@@ -927,9 +959,10 @@ int rtdm_event_timedwait(rtdm_event_t *e
 	if (unlikely(testbits(event->synch_base.status, RTDM_SYNCH_DELETED)))
 		err = -EIDRM;
 	else if (likely(xnsynch_test_flags(&event->synch_base,
-					   RTDM_EVENT_PENDING)))
+					   RTDM_EVENT_PENDING))) {
 		xnsynch_clear_flags(&event->synch_base, RTDM_EVENT_PENDING);
-	else {
+		xnselect_signal(&event->select_block, 0);
+	} else {
 		/* non-blocking mode */
 		if (timeout < 0) {
 			err = -EWOULDBLOCK;
@@ -951,10 +984,11 @@ int rtdm_event_timedwait(rtdm_event_t *e
 		}
 
 		if (likely
-		    (!xnthread_test_info(thread, XNTIMEO | XNRMID | XNBREAK)))
+		    (!xnthread_test_info(thread, XNTIMEO | XNRMID | XNBREAK))) {
 			xnsynch_clear_flags(&event->synch_base,
 					    RTDM_EVENT_PENDING);
-		else if (xnthread_test_info(thread, XNTIMEO))
+			xnselect_signal(&event->select_block, 0);
+		} else if (xnthread_test_info(thread, XNTIMEO))
 			err = -ETIMEDOUT;
 		else if (xnthread_test_info(thread, XNRMID))
 			err = -EIDRM;
@@ -995,6 +1029,7 @@ void rtdm_event_clear(rtdm_event_t *even
 	xnlock_get_irqsave(&nklock, s);
 
 	xnsynch_clear_flags(&event->synch_base, RTDM_EVENT_PENDING);
+	xnselect_signal(&event->select_block, 0);
 
 	xnlock_put_irqrestore(&nklock, s);
 }
@@ -1034,12 +1069,37 @@ void rtdm_sem_init(rtdm_sem_t *sem, unsi
 
 	sem->value = value;
 	xnsynch_init(&sem->synch_base, XNSYNCH_PRIO);
+	xnselect_init(&sem->select_block);
 
 	xnlock_put_irqrestore(&nklock, s);
 }
 
 EXPORT_SYMBOL(rtdm_sem_init);
 
+#ifdef CONFIG_XENO_OPT_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);
+	xnlock_put_irqrestore(&nklock, s);
+
+	return err;
+}
+EXPORT_SYMBOL(rtdm_sem_select_bind);
+#endif /* CONFIG_XENO_OPT_SELECT */
+
 #ifdef DOXYGEN_CPP /* Only used for doxygen doc generation */
 /**
  * @brief Destroy a semaphore
@@ -1148,8 +1208,8 @@ int rtdm_sem_timeddown(rtdm_sem_t *sem, 
 
 	if (testbits(sem->synch_base.status, RTDM_SYNCH_DELETED))
 		err = -EIDRM;
-	else if (sem->value > 0)
-		sem->value--;
+	else if (sem->value > 0 && !--sem->value)
+			xnselect_signal(&sem->select_block, 0);
 	else if (timeout < 0) /* non-blocking mode */
 		err = -EWOULDBLOCK;
 	else {
@@ -1214,7 +1274,9 @@ void rtdm_sem_up(rtdm_sem_t *sem)
 	if (xnsynch_wakeup_one_sleeper(&sem->synch_base))
 		xnpod_schedule();
 	else
-		sem->value++;
+		if (sem->value++ == 0
+		    && xnselect_signal(&sem->select_block, 1))
+			xnpod_schedule();
 
 	xnlock_put_irqrestore(&nklock, s);
 }
--- ksrc/skins/rtdm/core.c	(revision 3460)
+++ ksrc/skins/rtdm/core.c	(working copy)
@@ -49,28 +49,7 @@ EXPORT_SYMBOL(rtdm_tbase);
 
 DEFINE_XNLOCK(rt_fildes_lock);
 
-/**
- * @brief Resolve file descriptor to device context
- *
- * @param[in] fd File descriptor
- *
- * @return Pointer to associated device context, or NULL on error
- *
- * @note The device context has to be unlocked using rtdm_context_unlock()
- * when it is no longer referenced.
- *
- * Environments:
- *
- * This service can be called from:
- *
- * - Kernel module initialization/cleanup code
- * - Interrupt service routine
- * - Kernel-based task
- * - User-space task (RT, non-RT)
- *
- * Rescheduling: never.
- */
-struct rtdm_dev_context *rtdm_context_get(int fd)
+struct rtdm_dev_context *__rtdm_context_get(rtdm_user_info_t *user_info, int fd)
 {
 	struct rtdm_dev_context *context;
 	spl_t s;
@@ -82,7 +61,9 @@ struct rtdm_dev_context *rtdm_context_ge
 
 	context = fildes_table[fd].context;
 	if (unlikely(!context ||
-		     test_bit(RTDM_CLOSING, &context->context_flags))) {
+		     test_bit(RTDM_CLOSING, &context->context_flags) ||
+		     context->reserved.owner != (user_info
+						 ? user_info->mm : NULL))) {
 		xnlock_put_irqrestore(&rt_fildes_lock, s);
 		return NULL;
 	}
@@ -94,7 +75,7 @@ struct rtdm_dev_context *rtdm_context_ge
 	return context;
 }
 
-EXPORT_SYMBOL(rtdm_context_get);
+EXPORT_SYMBOL(__rtdm_context_get);
 
 static int create_instance(struct rtdm_device *device,
 			   struct rtdm_dev_context **context_ptr,
@@ -294,6 +275,34 @@ 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(info, 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;
@@ -312,7 +321,8 @@ again:
 
 	context = fildes_table[fd].context;
 
-	if (unlikely(!context)) {
+	if (unlikely(!context ||
+		     (user_info && context->reserved.owner != user_info->mm))) {
 		xnlock_put_irqrestore(&rt_fildes_lock, s);
 		goto err_out;	/* -EBADF */
 	}
@@ -408,7 +418,7 @@ do {									\
 	struct rtdm_operations *ops;					\
 	int ret;							\
 									\
-	context = rtdm_context_get(fd);					\
+	context = __rtdm_context_get(user_info, fd);			\
 	ret = -EBADF;							\
 	if (unlikely(!context))						\
 		goto err_out;						\
@@ -517,6 +527,28 @@ ssize_t __rt_dev_sendmsg(rtdm_user_info_
 EXPORT_SYMBOL(__rt_dev_sendmsg);
 
 #ifdef DOXYGEN_CPP /* Only used for doxygen doc generation */
+/**
+ * @brief Resolve file descriptor to device context
+ *
+ * @param[in] fd File descriptor
+ *
+ * @return Pointer to associated device context, or NULL on error
+ *
+ * @note The device context has to be unlocked using rtdm_context_unlock()
+ * when it is no longer referenced.
+ *
+ * Environments:
+ *
+ * This service can be called from:
+ *
+ * - Kernel module initialization/cleanup code
+ * - Interrupt service routine
+ * - Kernel-based task
+ * - User-space task (RT, non-RT)
+ *
+ * Rescheduling: never.
+ */
+struct rtdm_dev_context *rtdm_context_get(int fd);
 
 /**
  * @brief Increment context reference counter

Re: [Xenomai-core] [patch 2/4] RTDM support for select-like service.

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 963 bytes --]

Gilles Chanteperdrix wrote:
> stats:
>  include/rtdm/rtdm.h        |    4 ++
>  include/rtdm/rtdm_driver.h |   30 +++++++++++++++-
>  ksrc/skins/rtdm/core.c     |   84 +++++++++++++++++++++++++++++++--------------
>  ksrc/skins/rtdm/device.c   |   10 +++++
>  ksrc/skins/rtdm/drvlib.c   |   76 ++++++++++++++++++++++++++++++++++++----
>  5 files changed, 170 insertions(+), 34 deletions(-)
> 

/me thinks that going for inline patches on this list is overdue...

> @@ -1148,8 +1208,8 @@ int rtdm_sem_timeddown(rtdm_sem_t *sem, 
>  
>  	if (testbits(sem->synch_base.status, RTDM_SYNCH_DELETED))
>  		err = -EIDRM;
> -	else if (sem->value > 0)
> -		sem->value--;
> +	else if (sem->value > 0 && !--sem->value)
> +			xnselect_signal(&sem->select_block, 0);
>  	else if (timeout < 0) /* non-blocking mode */
>  		err = -EWOULDBLOCK;
>  	else {

Uuuh, this doesn't look equivalent (for --sem->value < 0).

Otherwise it's OK with me.

Jan


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

Re: [Xenomai-core] [patch 2/4] RTDM support for select-like service.

[Gilles Chanteperdrix]

Jan Kiszka wrote:
 > Gilles Chanteperdrix wrote:
 > > stats:
 > >  include/rtdm/rtdm.h        |    4 ++
 > >  include/rtdm/rtdm_driver.h |   30 +++++++++++++++-
 > >  ksrc/skins/rtdm/core.c     |   84 +++++++++++++++++++++++++++++++--------------
 > >  ksrc/skins/rtdm/device.c   |   10 +++++
 > >  ksrc/skins/rtdm/drvlib.c   |   76 ++++++++++++++++++++++++++++++++++++----
 > >  5 files changed, 170 insertions(+), 34 deletions(-)
 > > 
 > 
 > /me thinks that going for inline patches on this list is overdue...
 > 
 > > @@ -1148,8 +1208,8 @@ int rtdm_sem_timeddown(rtdm_sem_t *sem, 
 > >  
 > >  	if (testbits(sem->synch_base.status, RTDM_SYNCH_DELETED))
 > >  		err = -EIDRM;
 > > -	else if (sem->value > 0)
 > > -		sem->value--;
 > > +	else if (sem->value > 0 && !--sem->value)
 > > +			xnselect_signal(&sem->select_block, 0);
 > >  	else if (timeout < 0) /* non-blocking mode */
 > >  		err = -EWOULDBLOCK;
 > >  	else {
 > 
 > Uuuh, this doesn't look equivalent (for --sem->value < 0).
 > 
 > Otherwise it's OK with me.

Yes, of course, what we need is:

if (sem->value > 0) {
     if (!--sem->value)
        xnselect_signal
} else 

...

-- 


					    Gilles Chanteperdrix.

Re: [Xenomai-core] [patch 2/4] RTDM support for select-like service.

[Gilles Chanteperdrix]

Jan Kiszka wrote:
 > Gilles Chanteperdrix wrote:
 > > stats:
 > >  include/rtdm/rtdm.h        |    4 ++
 > >  include/rtdm/rtdm_driver.h |   30 +++++++++++++++-
 > >  ksrc/skins/rtdm/core.c     |   84 +++++++++++++++++++++++++++++++--------------
 > >  ksrc/skins/rtdm/device.c   |   10 +++++
 > >  ksrc/skins/rtdm/drvlib.c   |   76 ++++++++++++++++++++++++++++++++++++----
 > >  5 files changed, 170 insertions(+), 34 deletions(-)
 > > 
 > 
 > /me thinks that going for inline patches on this list is overdue...

I guess it depends on the mail client, but with my mail client, I get
plain text attachments inlined in the mail body. I somehow assumed that
it was the case for everyone else.

-- 


					    Gilles Chanteperdrix.

Re: [Xenomai-core] [patch 2/4] RTDM support for select-like service.

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1137 bytes --]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>  > Gilles Chanteperdrix wrote:
>  > > stats:
>  > >  include/rtdm/rtdm.h        |    4 ++
>  > >  include/rtdm/rtdm_driver.h |   30 +++++++++++++++-
>  > >  ksrc/skins/rtdm/core.c     |   84 +++++++++++++++++++++++++++++++--------------
>  > >  ksrc/skins/rtdm/device.c   |   10 +++++
>  > >  ksrc/skins/rtdm/drvlib.c   |   76 ++++++++++++++++++++++++++++++++++++----
>  > >  5 files changed, 170 insertions(+), 34 deletions(-)
>  > > 
>  > 
>  > /me thinks that going for inline patches on this list is overdue...
> 
> I guess it depends on the mail client, but with my mail client, I get
> plain text attachments inlined in the mail body. I somehow assumed that
> it was the case for everyone else.

Mine (TB) shows the patches inlined, but refuses to let me comment on
them. Maybe there is a knob deep inside its configs, but I heards such
complaints from other (likely non-TB) users as well.

The reason for staying with attachments was that /someone/ (not you) had
problems with grabbing them from his client without causing whitespace
havoc... :)

Jan


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

Re: [Xenomai-core] [patch 2/4] RTDM support for select-like service.

[Gilles Chanteperdrix]

Jan Kiszka wrote:
 > Gilles Chanteperdrix wrote:
 > > Jan Kiszka wrote:
 > >  > Gilles Chanteperdrix wrote:
 > >  > > stats:
 > >  > >  include/rtdm/rtdm.h        |    4 ++
 > >  > >  include/rtdm/rtdm_driver.h |   30 +++++++++++++++-
 > >  > >  ksrc/skins/rtdm/core.c     |   84 +++++++++++++++++++++++++++++++--------------
 > >  > >  ksrc/skins/rtdm/device.c   |   10 +++++
 > >  > >  ksrc/skins/rtdm/drvlib.c   |   76 ++++++++++++++++++++++++++++++++++++----
 > >  > >  5 files changed, 170 insertions(+), 34 deletions(-)
 > >  > > 
 > >  > 
 > >  > /me thinks that going for inline patches on this list is overdue...
 > > 
 > > I guess it depends on the mail client, but with my mail client, I get
 > > plain text attachments inlined in the mail body. I somehow assumed that
 > > it was the case for everyone else.
 > 
 > Mine (TB) shows the patches inlined, but refuses to let me comment on
 > them. Maybe there is a knob deep inside its configs, but I heards such
 > complaints from other (likely non-TB) users as well.

There are even things in thunderbird that can only be configured through
its configuration files, not with its menus.

 > The reason for staying with attachments was that /someone/ (not you) had
 > problems with grabbing them from his client without causing whitespace
 > havoc... :)

Yes, I think I would have the same problem, my mail client (I feel
ashamed to tell: XEmacs VM) splits the line at 72 characters. So, if I
paste a patch, I must refrain from reformatting the mail, otherwise the
patch could be broken.

-- 


					    Gilles Chanteperdrix.

Re: [Xenomai-core] [patch 2/4] RTDM support for select-like service.

[Gilles Chanteperdrix]

Jan Kiszka wrote:
 > Gilles Chanteperdrix wrote:
 > > stats:
 > >  include/rtdm/rtdm.h        |    4 ++
 > >  include/rtdm/rtdm_driver.h |   30 +++++++++++++++-
 > >  ksrc/skins/rtdm/core.c     |   84 +++++++++++++++++++++++++++++++--------------
 > >  ksrc/skins/rtdm/device.c   |   10 +++++
 > >  ksrc/skins/rtdm/drvlib.c   |   76 ++++++++++++++++++++++++++++++++++++----
 > >  5 files changed, 170 insertions(+), 34 deletions(-)
 > > 
 > 
 > /me thinks that going for inline patches on this list is overdue...
 > 
 > > @@ -1148,8 +1208,8 @@ int rtdm_sem_timeddown(rtdm_sem_t *sem, 
 > >  
 > >  	if (testbits(sem->synch_base.status, RTDM_SYNCH_DELETED))
 > >  		err = -EIDRM;
 > > -	else if (sem->value > 0)
 > > -		sem->value--;
 > > +	else if (sem->value > 0 && !--sem->value)
 > > +			xnselect_signal(&sem->select_block, 0);
 > >  	else if (timeout < 0) /* non-blocking mode */
 > >  		err = -EWOULDBLOCK;
 > >  	else {
 > 
 > Uuuh, this doesn't look equivalent (for --sem->value < 0).
 > 
 > Otherwise it's OK with me.

Here is a repost (inlined this time) of the select patch for rtdm, with
the discussed changes and a minor change in rtdm_sem_select_bind (since
I did not understand that sem->value could be negative).

If everybody is OK, I'll commit the whole thing.

stats:
 include/rtdm/rtdm.h        |    4 ++
 include/rtdm/rtdm_driver.h |   30 +++++++++++++++-
 ksrc/skins/rtdm/Kconfig    |    8 ++++
 ksrc/skins/rtdm/core.c     |   84 +++++++++++++++++++++++++++++++--------------
 ksrc/skins/rtdm/device.c   |   10 +++++
 ksrc/skins/rtdm/drvlib.c   |   79 ++++++++++++++++++++++++++++++++++++++----
 6 files changed, 180 insertions(+), 35 deletions(-)

-- 


					    Gilles Chanteperdrix.
--- include/rtdm/rtdm_driver.h	(revision 3460)
+++ include/rtdm/rtdm_driver.h	(working copy)
@@ -38,6 +38,7 @@
 #include <nucleus/heap.h>
 #include <nucleus/pod.h>
 #include <nucleus/synch.h>
+#include <nucleus/select.h>
 #include <rtdm/rtdm.h>
 
 /* debug support */
@@ -262,6 +263,10 @@ 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
  */
@@ -304,6 +309,8 @@ 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 {
@@ -453,7 +460,8 @@ int rtdm_dev_unregister(struct rtdm_devi
 #define rtdm_getpeername	rt_dev_getpeername
 #define rtdm_shutdown		rt_dev_shutdown
 
-struct rtdm_dev_context *rtdm_context_get(int fd);
+struct rtdm_dev_context *__rtdm_context_get(rtdm_user_info_t *user_info, int fd);
+#define rtdm_context_get(fd) __rtdm_context_get(NULL, fd)
 
 #ifndef DOXYGEN_CPP /* Avoid static inline tags for RTDM in doxygen */
 static inline void rtdm_context_lock(struct rtdm_dev_context *context)
@@ -1002,11 +1010,20 @@ void rtdm_toseq_init(rtdm_toseq_t *timeo
 
 typedef struct {
 	xnsynch_t synch_base;
+	DECLARE_XNSELECT(select_block);
 } rtdm_event_t;
 
 #define RTDM_EVENT_PENDING		XNSYNCH_SPARE1
 
 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);
+#else /* !CONFIG_XENO_OPT_RTDM_SELECT */
+#define rtdm_event_select_bind(e, s, t, b) ({ -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,
 			 rtdm_toseq_t *timeout_seq);
@@ -1027,6 +1044,7 @@ static inline void rtdm_event_destroy(rt
 {
 	trace_mark(xn_rtdm_event_destroy, "event %p", event);
 	__rtdm_synch_flush(&event->synch_base, XNRMID);
+	xnselect_destroy(&event->select_block);
 }
 #endif /* !DOXYGEN_CPP */
 
@@ -1035,9 +1053,18 @@ static inline void rtdm_event_destroy(rt
 typedef struct {
 	unsigned long value;
 	xnsynch_t synch_base;
+	DECLARE_XNSELECT(select_block);
 } rtdm_sem_t;
 
 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);
+#else /* !CONFIG_XENO_OPT_RTDM_SELECT */
+#define rtdm_sem_select_bind(s, se, t, b) ({ -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,
 		       rtdm_toseq_t *timeout_seq);
@@ -1048,6 +1075,7 @@ static inline void rtdm_sem_destroy(rtdm
 {
 	trace_mark(xn_rtdm_sem_destroy, "sem %p", sem);
 	__rtdm_synch_flush(&sem->synch_base, XNRMID);
+	xnselect_destroy(&sem->select_block);
 }
 #endif /* !DOXYGEN_CPP */
 
--- include/rtdm/rtdm.h	(revision 3460)
+++ include/rtdm/rtdm.h	(working copy)
@@ -247,6 +247,10 @@ 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
--- ksrc/skins/rtdm/Kconfig	(revision 3460)
+++ ksrc/skins/rtdm/Kconfig	(working copy)
@@ -37,6 +37,14 @@ config XENO_OPT_RTDM_FILDES
 	a global resource all applications share, either via the RTDM skin
 	directly or via the embedded services of the POSIX skin.
 
+config XENO_OPT_RTDM_SELECT
+	bool "Select support for RTDM file descriptors"
+	select XENO_OPT_SELECT
+	help
+
+	This option allows RTDM-based file descriptors to be used with
+	select-like services.
+
 config XENO_OPT_DEBUG_RTDM
 	bool "RTDM debugging support"
 	depends on XENO_OPT_DEBUG
--- ksrc/skins/rtdm/device.c	(revision 3460)
+++ ksrc/skins/rtdm/device.c	(working copy)
@@ -70,6 +70,14 @@ int rtdm_no_support(void)
 	return -ENOSYS;
 }
 
+int rtdm_select_bind_no_support(struct rtdm_dev_context *context,
+				struct xnselector *selector,
+				unsigned type,
+				unsigned index)
+{
+	return -EBADF;
+}
+  
 static inline int get_name_hash(const char *str, int limit, int hashkey_mask)
 {
 	int hash = 0;
@@ -242,6 +250,8 @@ int rtdm_dev_register(struct rtdm_device
 	SET_DEFAULT_OP_IF_NULL(device->ops, write);
 	SET_DEFAULT_OP_IF_NULL(device->ops, recvmsg);
 	SET_DEFAULT_OP_IF_NULL(device->ops, sendmsg);
+	if (!device->ops.select_bind)
+		device->ops.select_bind = rtdm_select_bind_no_support;
 
 	atomic_set(&device->reserved.refcount, 0);
 	device->reserved.exclusive_context = NULL;
--- ksrc/skins/rtdm/drvlib.c	(revision 3460)
+++ ksrc/skins/rtdm/drvlib.c	(working copy)
@@ -756,12 +756,39 @@ void rtdm_event_init(rtdm_event_t *event
 	xnsynch_init(&event->synch_base, XNSYNCH_PRIO);
 	if (pending)
 		xnsynch_set_flags(&event->synch_base, RTDM_EVENT_PENDING);
+	xnselect_init(&event->select_block);
 
 	xnlock_put_irqrestore(&nklock, s);
 }
 
 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
@@ -825,6 +852,7 @@ void rtdm_event_pulse(rtdm_event_t *even
  */
 void rtdm_event_signal(rtdm_event_t *event)
 {
+	int resched = 0;
 	spl_t s;
 
 	trace_mark(xn_rtdm_event_signal, "event %p", event);
@@ -833,6 +861,10 @@ void rtdm_event_signal(rtdm_event_t *eve
 
 	xnsynch_set_flags(&event->synch_base, RTDM_EVENT_PENDING);
 	if (xnsynch_flush(&event->synch_base, 0))
+		resched = 1;
+	if (xnselect_signal(&event->select_block, 1))
+		resched = 1;
+	if (resched)
 		xnpod_schedule();
 
 	xnlock_put_irqrestore(&nklock, s);
@@ -927,9 +959,10 @@ int rtdm_event_timedwait(rtdm_event_t *e
 	if (unlikely(testbits(event->synch_base.status, RTDM_SYNCH_DELETED)))
 		err = -EIDRM;
 	else if (likely(xnsynch_test_flags(&event->synch_base,
-					   RTDM_EVENT_PENDING)))
+					   RTDM_EVENT_PENDING))) {
 		xnsynch_clear_flags(&event->synch_base, RTDM_EVENT_PENDING);
-	else {
+		xnselect_signal(&event->select_block, 0);
+	} else {
 		/* non-blocking mode */
 		if (timeout < 0) {
 			err = -EWOULDBLOCK;
@@ -951,10 +984,11 @@ int rtdm_event_timedwait(rtdm_event_t *e
 		}
 
 		if (likely
-		    (!xnthread_test_info(thread, XNTIMEO | XNRMID | XNBREAK)))
+		    (!xnthread_test_info(thread, XNTIMEO | XNRMID | XNBREAK))) {
 			xnsynch_clear_flags(&event->synch_base,
 					    RTDM_EVENT_PENDING);
-		else if (xnthread_test_info(thread, XNTIMEO))
+			xnselect_signal(&event->select_block, 0);
+		} else if (xnthread_test_info(thread, XNTIMEO))
 			err = -ETIMEDOUT;
 		else if (xnthread_test_info(thread, XNRMID))
 			err = -EIDRM;
@@ -995,6 +1029,7 @@ void rtdm_event_clear(rtdm_event_t *even
 	xnlock_get_irqsave(&nklock, s);
 
 	xnsynch_clear_flags(&event->synch_base, RTDM_EVENT_PENDING);
+	xnselect_signal(&event->select_block, 0);
 
 	xnlock_put_irqrestore(&nklock, s);
 }
@@ -1034,12 +1069,37 @@ void rtdm_sem_init(rtdm_sem_t *sem, unsi
 
 	sem->value = value;
 	xnsynch_init(&sem->synch_base, XNSYNCH_PRIO);
+	xnselect_init(&sem->select_block);
 
 	xnlock_put_irqrestore(&nklock, s);
 }
 
 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
@@ -1148,9 +1208,10 @@ int rtdm_sem_timeddown(rtdm_sem_t *sem, 
 
 	if (testbits(sem->synch_base.status, RTDM_SYNCH_DELETED))
 		err = -EIDRM;
-	else if (sem->value > 0)
-		sem->value--;
-	else if (timeout < 0) /* non-blocking mode */
+	else if (sem->value > 0) {
+		if(!--sem->value)
+			xnselect_signal(&sem->select_block, 0);
+	} else if (timeout < 0) /* non-blocking mode */
 		err = -EWOULDBLOCK;
 	else {
 		thread = xnpod_current_thread();
@@ -1214,7 +1275,9 @@ void rtdm_sem_up(rtdm_sem_t *sem)
 	if (xnsynch_wakeup_one_sleeper(&sem->synch_base))
 		xnpod_schedule();
 	else
-		sem->value++;
+		if (sem->value++ == 0
+		    && xnselect_signal(&sem->select_block, 1))
+			xnpod_schedule();
 
 	xnlock_put_irqrestore(&nklock, s);
 }
--- ksrc/skins/rtdm/core.c	(revision 3460)
+++ ksrc/skins/rtdm/core.c	(working copy)
@@ -49,28 +49,7 @@ EXPORT_SYMBOL(rtdm_tbase);
 
 DEFINE_XNLOCK(rt_fildes_lock);
 
-/**
- * @brief Resolve file descriptor to device context
- *
- * @param[in] fd File descriptor
- *
- * @return Pointer to associated device context, or NULL on error
- *
- * @note The device context has to be unlocked using rtdm_context_unlock()
- * when it is no longer referenced.
- *
- * Environments:
- *
- * This service can be called from:
- *
- * - Kernel module initialization/cleanup code
- * - Interrupt service routine
- * - Kernel-based task
- * - User-space task (RT, non-RT)
- *
- * Rescheduling: never.
- */
-struct rtdm_dev_context *rtdm_context_get(int fd)
+struct rtdm_dev_context *__rtdm_context_get(rtdm_user_info_t *user_info, int fd)
 {
 	struct rtdm_dev_context *context;
 	spl_t s;
@@ -82,7 +61,9 @@ struct rtdm_dev_context *rtdm_context_ge
 
 	context = fildes_table[fd].context;
 	if (unlikely(!context ||
-		     test_bit(RTDM_CLOSING, &context->context_flags))) {
+		     test_bit(RTDM_CLOSING, &context->context_flags) ||
+		     context->reserved.owner != (user_info
+						 ? user_info->mm : NULL))) {
 		xnlock_put_irqrestore(&rt_fildes_lock, s);
 		return NULL;
 	}
@@ -94,7 +75,7 @@ struct rtdm_dev_context *rtdm_context_ge
 	return context;
 }
 
-EXPORT_SYMBOL(rtdm_context_get);
+EXPORT_SYMBOL(__rtdm_context_get);
 
 static int create_instance(struct rtdm_device *device,
 			   struct rtdm_dev_context **context_ptr,
@@ -294,6 +275,34 @@ 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(info, 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;
@@ -312,7 +321,8 @@ again:
 
 	context = fildes_table[fd].context;
 
-	if (unlikely(!context)) {
+	if (unlikely(!context ||
+		     (user_info && context->reserved.owner != user_info->mm))) {
 		xnlock_put_irqrestore(&rt_fildes_lock, s);
 		goto err_out;	/* -EBADF */
 	}
@@ -408,7 +418,7 @@ do {									\
 	struct rtdm_operations *ops;					\
 	int ret;							\
 									\
-	context = rtdm_context_get(fd);					\
+	context = __rtdm_context_get(user_info, fd);			\
 	ret = -EBADF;							\
 	if (unlikely(!context))						\
 		goto err_out;						\
@@ -517,6 +527,28 @@ ssize_t __rt_dev_sendmsg(rtdm_user_info_
 EXPORT_SYMBOL(__rt_dev_sendmsg);
 
 #ifdef DOXYGEN_CPP /* Only used for doxygen doc generation */
+/**
+ * @brief Resolve file descriptor to device context
+ *
+ * @param[in] fd File descriptor
+ *
+ * @return Pointer to associated device context, or NULL on error
+ *
+ * @note The device context has to be unlocked using rtdm_context_unlock()
+ * when it is no longer referenced.
+ *
+ * Environments:
+ *
+ * This service can be called from:
+ *
+ * - Kernel module initialization/cleanup code
+ * - Interrupt service routine
+ * - Kernel-based task
+ * - User-space task (RT, non-RT)
+ *
+ * Rescheduling: never.
+ */
+struct rtdm_dev_context *rtdm_context_get(int fd);
 
 /**
  * @brief Increment context reference counter

Re: [Xenomai-core] [patch 2/4] RTDM support for select-like service.

[Gilles Chanteperdrix]

Jan Kiszka wrote:
 > Gilles Chanteperdrix wrote:
 > > stats:
 > >  include/rtdm/rtdm.h        |    4 ++
 > >  include/rtdm/rtdm_driver.h |   30 +++++++++++++++-
 > >  ksrc/skins/rtdm/core.c     |   84 +++++++++++++++++++++++++++++++--------------
 > >  ksrc/skins/rtdm/device.c   |   10 +++++
 > >  ksrc/skins/rtdm/drvlib.c   |   76 ++++++++++++++++++++++++++++++++++++----
 > >  5 files changed, 170 insertions(+), 34 deletions(-)
 > > 
 > 
 > /me thinks that going for inline patches on this list is overdue...
 > 
 > > @@ -1148,8 +1208,8 @@ int rtdm_sem_timeddown(rtdm_sem_t *sem, 
 > >  
 > >  	if (testbits(sem->synch_base.status, RTDM_SYNCH_DELETED))
 > >  		err = -EIDRM;
 > > -	else if (sem->value > 0)
 > > -		sem->value--;
 > > +	else if (sem->value > 0 && !--sem->value)
 > > +			xnselect_signal(&sem->select_block, 0);
 > >  	else if (timeout < 0) /* non-blocking mode */
 > >  		err = -EWOULDBLOCK;
 > >  	else {
 > 
 > Uuuh, this doesn't look equivalent (for --sem->value < 0).
 > 
 > Otherwise it's OK with me.

After testing, it appears that the "owner" of a file descriptor changed from 
current->mm 
to
container_of(xnshadow_ppd_get(__rtdm_muxid), struct rtdm_process, ppd)

So, I now need a ppd hash lookup in __rtdm_context_get.

-- 


					    Gilles Chanteperdrix.

Re: [Xenomai-core] [patch 2/4] RTDM support for select-like service.

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1744 bytes --]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>  > Gilles Chanteperdrix wrote:
>  > > stats:
>  > >  include/rtdm/rtdm.h        |    4 ++
>  > >  include/rtdm/rtdm_driver.h |   30 +++++++++++++++-
>  > >  ksrc/skins/rtdm/core.c     |   84 +++++++++++++++++++++++++++++++--------------
>  > >  ksrc/skins/rtdm/device.c   |   10 +++++
>  > >  ksrc/skins/rtdm/drvlib.c   |   76 ++++++++++++++++++++++++++++++++++++----
>  > >  5 files changed, 170 insertions(+), 34 deletions(-)
>  > > 
>  > 
>  > /me thinks that going for inline patches on this list is overdue...
>  > 
>  > > @@ -1148,8 +1208,8 @@ int rtdm_sem_timeddown(rtdm_sem_t *sem, 
>  > >  
>  > >  	if (testbits(sem->synch_base.status, RTDM_SYNCH_DELETED))
>  > >  		err = -EIDRM;
>  > > -	else if (sem->value > 0)
>  > > -		sem->value--;
>  > > +	else if (sem->value > 0 && !--sem->value)
>  > > +			xnselect_signal(&sem->select_block, 0);
>  > >  	else if (timeout < 0) /* non-blocking mode */
>  > >  		err = -EWOULDBLOCK;
>  > >  	else {
>  > 
>  > Uuuh, this doesn't look equivalent (for --sem->value < 0).
>  > 
>  > Otherwise it's OK with me.
> 
> After testing, it appears that the "owner" of a file descriptor changed from 
> current->mm 
> to
> container_of(xnshadow_ppd_get(__rtdm_muxid), struct rtdm_process, ppd)

Yes, indeed, forgot about it. We have a full-blown owner structure now
to track comm and pid.

> 
> So, I now need a ppd hash lookup in __rtdm_context_get.
> 

You could add mm to rtdm_process. This means another indirection and
makes things worse (more cache misses in the worst case...). Could you
live without the check until we have per-process fd tabled, or was it
essential for the select thing?

Jan


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

Re: [Xenomai-core] [patch 2/4] RTDM support for select-like service.

[Gilles Chanteperdrix]

Jan Kiszka wrote:
 > Gilles Chanteperdrix wrote:
 > > Jan Kiszka wrote:
 > >  > Gilles Chanteperdrix wrote:
 > >  > > stats:
 > >  > >  include/rtdm/rtdm.h        |    4 ++
 > >  > >  include/rtdm/rtdm_driver.h |   30 +++++++++++++++-
 > >  > >  ksrc/skins/rtdm/core.c     |   84 +++++++++++++++++++++++++++++++--------------
 > >  > >  ksrc/skins/rtdm/device.c   |   10 +++++
 > >  > >  ksrc/skins/rtdm/drvlib.c   |   76 ++++++++++++++++++++++++++++++++++++----
 > >  > >  5 files changed, 170 insertions(+), 34 deletions(-)
 > >  > > 
 > >  > 
 > >  > /me thinks that going for inline patches on this list is overdue...
 > >  > 
 > >  > > @@ -1148,8 +1208,8 @@ int rtdm_sem_timeddown(rtdm_sem_t *sem, 
 > >  > >  
 > >  > >  	if (testbits(sem->synch_base.status, RTDM_SYNCH_DELETED))
 > >  > >  		err = -EIDRM;
 > >  > > -	else if (sem->value > 0)
 > >  > > -		sem->value--;
 > >  > > +	else if (sem->value > 0 && !--sem->value)
 > >  > > +			xnselect_signal(&sem->select_block, 0);
 > >  > >  	else if (timeout < 0) /* non-blocking mode */
 > >  > >  		err = -EWOULDBLOCK;
 > >  > >  	else {
 > >  > 
 > >  > Uuuh, this doesn't look equivalent (for --sem->value < 0).
 > >  > 
 > >  > Otherwise it's OK with me.
 > > 
 > > After testing, it appears that the "owner" of a file descriptor changed from 
 > > current->mm 
 > > to
 > > container_of(xnshadow_ppd_get(__rtdm_muxid), struct rtdm_process, ppd)
 > 
 > Yes, indeed, forgot about it. We have a full-blown owner structure now
 > to track comm and pid.
 > 
 > > 
 > > So, I now need a ppd hash lookup in __rtdm_context_get.
 > > 
 > 
 > You could add mm to rtdm_process. This means another indirection and
 > makes things worse (more cache misses in the worst case...).

Would not it be simpler to put a pointer to the task_struct ? After all,
it already has a pid, comm and mm, and a file descriptor will not
survive a task_struct thanks to automatic closing of file descriptors.

 Could you
 > live without the check until we have per-process fd tabled, or was it
 > essential for the select thing?

An application which I ported to Xenomai (and which uses the select
call) closes all file descriptors in a for loop. The purpose of this
loop is, I guess, to avoid leaving a file descriptor opened that was
passed through exec.

-- 


					    Gilles Chanteperdrix.

Re: [Xenomai-core] [patch 2/4] RTDM support for select-like service.

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2613 bytes --]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>  > Gilles Chanteperdrix wrote:
>  > > Jan Kiszka wrote:
>  > >  > Gilles Chanteperdrix wrote:
>  > >  > > stats:
>  > >  > >  include/rtdm/rtdm.h        |    4 ++
>  > >  > >  include/rtdm/rtdm_driver.h |   30 +++++++++++++++-
>  > >  > >  ksrc/skins/rtdm/core.c     |   84 +++++++++++++++++++++++++++++++--------------
>  > >  > >  ksrc/skins/rtdm/device.c   |   10 +++++
>  > >  > >  ksrc/skins/rtdm/drvlib.c   |   76 ++++++++++++++++++++++++++++++++++++----
>  > >  > >  5 files changed, 170 insertions(+), 34 deletions(-)
>  > >  > > 
>  > >  > 
>  > >  > /me thinks that going for inline patches on this list is overdue...
>  > >  > 
>  > >  > > @@ -1148,8 +1208,8 @@ int rtdm_sem_timeddown(rtdm_sem_t *sem, 
>  > >  > >  
>  > >  > >  	if (testbits(sem->synch_base.status, RTDM_SYNCH_DELETED))
>  > >  > >  		err = -EIDRM;
>  > >  > > -	else if (sem->value > 0)
>  > >  > > -		sem->value--;
>  > >  > > +	else if (sem->value > 0 && !--sem->value)
>  > >  > > +			xnselect_signal(&sem->select_block, 0);
>  > >  > >  	else if (timeout < 0) /* non-blocking mode */
>  > >  > >  		err = -EWOULDBLOCK;
>  > >  > >  	else {
>  > >  > 
>  > >  > Uuuh, this doesn't look equivalent (for --sem->value < 0).
>  > >  > 
>  > >  > Otherwise it's OK with me.
>  > > 
>  > > After testing, it appears that the "owner" of a file descriptor changed from 
>  > > current->mm 
>  > > to
>  > > container_of(xnshadow_ppd_get(__rtdm_muxid), struct rtdm_process, ppd)
>  > 
>  > Yes, indeed, forgot about it. We have a full-blown owner structure now
>  > to track comm and pid.
>  > 
>  > > 
>  > > So, I now need a ppd hash lookup in __rtdm_context_get.
>  > > 
>  > 
>  > You could add mm to rtdm_process. This means another indirection and
>  > makes things worse (more cache misses in the worst case...).
> 
> Would not it be simpler to put a pointer to the task_struct ? After all,
> it already has a pid, comm and mm, and a file descriptor will not
> survive a task_struct thanks to automatic closing of file descriptors.

Hmm, hmm, hmmmm... Sounds reasonable, should be safe.

> 
>  Could you
>  > live without the check until we have per-process fd tabled, or was it
>  > essential for the select thing?
> 
> An application which I ported to Xenomai (and which uses the select
> call) closes all file descriptors in a for loop. The purpose of this
> loop is, I guess, to avoid leaving a file descriptor opened that was
> passed through exec.

OK.

So, will you change rtdm_process too? Thanks.

Jan


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

Re: [Xenomai-core] [patch 2/4] RTDM support for select-like service.

[Gilles Chanteperdrix]

Jan Kiszka wrote:
 > Gilles Chanteperdrix wrote:
 > > Jan Kiszka wrote:
 > >  > Gilles Chanteperdrix wrote:
 > >  > > Jan Kiszka wrote:
 > >  > >  > Gilles Chanteperdrix wrote:
 > >  > >  > > stats:
 > >  > >  > >  include/rtdm/rtdm.h        |    4 ++
 > >  > >  > >  include/rtdm/rtdm_driver.h |   30 +++++++++++++++-
 > >  > >  > >  ksrc/skins/rtdm/core.c     |   84 +++++++++++++++++++++++++++++++--------------
 > >  > >  > >  ksrc/skins/rtdm/device.c   |   10 +++++
 > >  > >  > >  ksrc/skins/rtdm/drvlib.c   |   76 ++++++++++++++++++++++++++++++++++++----
 > >  > >  > >  5 files changed, 170 insertions(+), 34 deletions(-)
 > >  > >  > > 
 > >  > >  > 
 > >  > >  > /me thinks that going for inline patches on this list is overdue...
 > >  > >  > 
 > >  > >  > > @@ -1148,8 +1208,8 @@ int rtdm_sem_timeddown(rtdm_sem_t *sem, 
 > >  > >  > >  
 > >  > >  > >  	if (testbits(sem->synch_base.status, RTDM_SYNCH_DELETED))
 > >  > >  > >  		err = -EIDRM;
 > >  > >  > > -	else if (sem->value > 0)
 > >  > >  > > -		sem->value--;
 > >  > >  > > +	else if (sem->value > 0 && !--sem->value)
 > >  > >  > > +			xnselect_signal(&sem->select_block, 0);
 > >  > >  > >  	else if (timeout < 0) /* non-blocking mode */
 > >  > >  > >  		err = -EWOULDBLOCK;
 > >  > >  > >  	else {
 > >  > >  > 
 > >  > >  > Uuuh, this doesn't look equivalent (for --sem->value < 0).
 > >  > >  > 
 > >  > >  > Otherwise it's OK with me.
 > >  > > 
 > >  > > After testing, it appears that the "owner" of a file descriptor changed from 
 > >  > > current->mm 
 > >  > > to
 > >  > > container_of(xnshadow_ppd_get(__rtdm_muxid), struct rtdm_process, ppd)
 > >  > 
 > >  > Yes, indeed, forgot about it. We have a full-blown owner structure now
 > >  > to track comm and pid.
 > >  > 
 > >  > > 
 > >  > > So, I now need a ppd hash lookup in __rtdm_context_get.
 > >  > > 
 > >  > 
 > >  > You could add mm to rtdm_process. This means another indirection and
 > >  > makes things worse (more cache misses in the worst case...).
 > > 
 > > Would not it be simpler to put a pointer to the task_struct ? After all,
 > > it already has a pid, comm and mm, and a file descriptor will not
 > > survive a task_struct thanks to automatic closing of file descriptors.
 > 
 > Hmm, hmm, hmmmm... Sounds reasonable, should be safe.
 > 
 > > 
 > >  Could you
 > >  > live without the check until we have per-process fd tabled, or was it
 > >  > essential for the select thing?
 > > 
 > > An application which I ported to Xenomai (and which uses the select
 > > call) closes all file descriptors in a for loop. The purpose of this
 > > loop is, I guess, to avoid leaving a file descriptor opened that was
 > > passed through exec.
 > 
 > OK.
 > 
 > So, will you change rtdm_process too? Thanks.

Ok, but I will send the new patch tomorrow.

-- 


					    Gilles Chanteperdrix.

Re: [Xenomai-core] [patch 2/4] RTDM support for select-like service.

[Gilles Chanteperdrix]

Jan Kiszka wrote:
 > Gilles Chanteperdrix wrote:
 > > Would not it be simpler to put a pointer to the task_struct ? After all,
 > > it already has a pid, comm and mm, and a file descriptor will not
 > > survive a task_struct thanks to automatic closing of file descriptors.
 > 
 > Hmm, hmm, hmmmm... Sounds reasonable, should be safe.

Actually no, we can not do that because a task_struct may well disappear
and the rtdm_process continue to exist as long as another thread uses the
same mm.

 > 
 > > 
 > >  Could you
 > >  > live without the check until we have per-process fd tabled, or was it
 > >  > essential for the select thing?
 > > 
 > > An application which I ported to Xenomai (and which uses the select
 > > call) closes all file descriptors in a for loop. The purpose of this
 > > loop is, I guess, to avoid leaving a file descriptor opened that was
 > > passed through exec.
 > 
 > OK.
 > 
 > So, will you change rtdm_process too? Thanks.

I commited the select support, without any change to rtdm_context_get or
rtdm_process. So, now, how do you prefer this to be fixed, by adding an
mm struct to the rtdm_process struct ? By the way, after thinking about
it I can live without this fix: I just have to stop the loop closing
file descriptors at 768, so that it will not try to close RTDM file
descriptors.

While commiting the support for select, I also had a dependency problem
in Kconfig: when support for posix select is enabled the posix module
uses a function defined in the RTDM module. So, there is one invalid
configuration: posix built-in with support for select and rtdm built as
a module. I could not find a way to express this condition in the
Kconfig language, so I just made a comment depend on this condition, but
would be happy if anyone found a better solution.

-- 


					    Gilles Chanteperdrix.

Re: [Xenomai-core] [patch 2/4] RTDM support for select-like service.

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2255 bytes --]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>  > Gilles Chanteperdrix wrote:
>  > > Would not it be simpler to put a pointer to the task_struct ? After all,
>  > > it already has a pid, comm and mm, and a file descriptor will not
>  > > survive a task_struct thanks to automatic closing of file descriptors.
>  > 
>  > Hmm, hmm, hmmmm... Sounds reasonable, should be safe.
> 
> Actually no, we can not do that because a task_struct may well disappear
> and the rtdm_process continue to exist as long as another thread uses the
> same mm.

Because we cleanup on mm exit, not task exit, right? OK, looks like I 
originally spent a few more thoughts than this time :-/.

> 
>  > 
>  > > 
>  > >  Could you
>  > >  > live without the check until we have per-process fd tabled, or was it
>  > >  > essential for the select thing?
>  > > 
>  > > An application which I ported to Xenomai (and which uses the select
>  > > call) closes all file descriptors in a for loop. The purpose of this
>  > > loop is, I guess, to avoid leaving a file descriptor opened that was
>  > > passed through exec.
>  > 
>  > OK.
>  > 
>  > So, will you change rtdm_process too? Thanks.
> 
> I commited the select support, without any change to rtdm_context_get or
> rtdm_process. So, now, how do you prefer this to be fixed, by adding an
> mm struct to the rtdm_process struct ? By the way, after thinking about
> it I can live without this fix: I just have to stop the loop closing
> file descriptors at 768, so that it will not try to close RTDM file
> descriptors.

If you can live with it, I would vote for fixing it by the intended 
redesign via per-process fds.

> 
> While commiting the support for select, I also had a dependency problem
> in Kconfig: when support for posix select is enabled the posix module
> uses a function defined in the RTDM module. So, there is one invalid
> configuration: posix built-in with support for select and rtdm built as
> a module. I could not find a way to express this condition in the
> Kconfig language, so I just made a comment depend on this condition, but
> would be happy if anyone found a better solution.

I would say:

config POSIX
	select RTDM if OPT_SELECT

Jan


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

Re: [Xenomai-core] [patch 2/4] RTDM support for select-like service.

[Gilles Chanteperdrix]

Jan Kiszka wrote:
 > Gilles Chanteperdrix wrote:
 > > Jan Kiszka wrote:
 > >  > Gilles Chanteperdrix wrote:
 > >  > > Would not it be simpler to put a pointer to the task_struct ? After all,
 > >  > > it already has a pid, comm and mm, and a file descriptor will not
 > >  > > survive a task_struct thanks to automatic closing of file descriptors.
 > >  > 
 > >  > Hmm, hmm, hmmmm... Sounds reasonable, should be safe.
 > > 
 > > Actually no, we can not do that because a task_struct may well disappear
 > > and the rtdm_process continue to exist as long as another thread uses the
 > > same mm.
 > 
 > Because we cleanup on mm exit, not task exit, right? OK, looks like I 
 > originally spent a few more thoughts than this time :-/.
 > 
 > > 
 > >  > 
 > >  > > 
 > >  > >  Could you
 > >  > >  > live without the check until we have per-process fd tabled, or was it
 > >  > >  > essential for the select thing?
 > >  > > 
 > >  > > An application which I ported to Xenomai (and which uses the select
 > >  > > call) closes all file descriptors in a for loop. The purpose of this
 > >  > > loop is, I guess, to avoid leaving a file descriptor opened that was
 > >  > > passed through exec.
 > >  > 
 > >  > OK.
 > >  > 
 > >  > So, will you change rtdm_process too? Thanks.
 > > 
 > > I commited the select support, without any change to rtdm_context_get or
 > > rtdm_process. So, now, how do you prefer this to be fixed, by adding an
 > > mm struct to the rtdm_process struct ? By the way, after thinking about
 > > it I can live without this fix: I just have to stop the loop closing
 > > file descriptors at 768, so that it will not try to close RTDM file
 > > descriptors.
 > 
 > If you can live with it, I would vote for fixing it by the intended 
 > redesign via per-process fds.

Ok.

 > 
 > > 
 > > While commiting the support for select, I also had a dependency problem
 > > in Kconfig: when support for posix select is enabled the posix module
 > > uses a function defined in the RTDM module. So, there is one invalid
 > > configuration: posix built-in with support for select and rtdm built as
 > > a module. I could not find a way to express this condition in the
 > > Kconfig language, so I just made a comment depend on this condition, but
 > > would be happy if anyone found a better solution.
 > 
 > I would say:
 > 
 > config POSIX
 > 	select RTDM if OPT_SELECT

But in this case, I can not have posix with select and without rtdm.

-- 


					    Gilles Chanteperdrix.

Re: [Xenomai-core] [patch 2/4] RTDM support for select-like service.

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2902 bytes --]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>  > Gilles Chanteperdrix wrote:
>  > > Jan Kiszka wrote:
>  > >  > Gilles Chanteperdrix wrote:
>  > >  > > Would not it be simpler to put a pointer to the task_struct ? After all,
>  > >  > > it already has a pid, comm and mm, and a file descriptor will not
>  > >  > > survive a task_struct thanks to automatic closing of file descriptors.
>  > >  > 
>  > >  > Hmm, hmm, hmmmm... Sounds reasonable, should be safe.
>  > > 
>  > > Actually no, we can not do that because a task_struct may well disappear
>  > > and the rtdm_process continue to exist as long as another thread uses the
>  > > same mm.
>  > 
>  > Because we cleanup on mm exit, not task exit, right? OK, looks like I 
>  > originally spent a few more thoughts than this time :-/.
>  > 
>  > > 
>  > >  > 
>  > >  > > 
>  > >  > >  Could you
>  > >  > >  > live without the check until we have per-process fd tabled, or was it
>  > >  > >  > essential for the select thing?
>  > >  > > 
>  > >  > > An application which I ported to Xenomai (and which uses the select
>  > >  > > call) closes all file descriptors in a for loop. The purpose of this
>  > >  > > loop is, I guess, to avoid leaving a file descriptor opened that was
>  > >  > > passed through exec.
>  > >  > 
>  > >  > OK.
>  > >  > 
>  > >  > So, will you change rtdm_process too? Thanks.
>  > > 
>  > > I commited the select support, without any change to rtdm_context_get or
>  > > rtdm_process. So, now, how do you prefer this to be fixed, by adding an
>  > > mm struct to the rtdm_process struct ? By the way, after thinking about
>  > > it I can live without this fix: I just have to stop the loop closing
>  > > file descriptors at 768, so that it will not try to close RTDM file
>  > > descriptors.
>  > 
>  > If you can live with it, I would vote for fixing it by the intended 
>  > redesign via per-process fds.
> 
> Ok.
> 
>  > 
>  > > 
>  > > While commiting the support for select, I also had a dependency problem
>  > > in Kconfig: when support for posix select is enabled the posix module
>  > > uses a function defined in the RTDM module. So, there is one invalid
>  > > configuration: posix built-in with support for select and rtdm built as
>  > > a module. I could not find a way to express this condition in the
>  > > Kconfig language, so I just made a comment depend on this condition, but
>  > > would be happy if anyone found a better solution.
>  > 
>  > I would say:
>  > 
>  > config POSIX
>  > 	select RTDM if OPT_SELECT
> 
> But in this case, I can not have posix with select and without rtdm.

Then you need to isolate those services that POSIX needs from RTDM. The 
above just expresses the dependency you described. In the end this just 
shows that we have to define the common fd-ground for both skins in the 
core.

Jan


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

Re: [Xenomai-core] [patch 2/4] RTDM support for select-like service.

[Gilles Chanteperdrix]

Jan Kiszka wrote:
 > Gilles Chanteperdrix wrote:
 > > Jan Kiszka wrote:
 > >  > Gilles Chanteperdrix wrote:
 > >  > > Jan Kiszka wrote:
 > >  > >  > Gilles Chanteperdrix wrote:
 > >  > >  > > Would not it be simpler to put a pointer to the task_struct ? After all,
 > >  > >  > > it already has a pid, comm and mm, and a file descriptor will not
 > >  > >  > > survive a task_struct thanks to automatic closing of file descriptors.
 > >  > >  > 
 > >  > >  > Hmm, hmm, hmmmm... Sounds reasonable, should be safe.
 > >  > > 
 > >  > > Actually no, we can not do that because a task_struct may well disappear
 > >  > > and the rtdm_process continue to exist as long as another thread uses the
 > >  > > same mm.
 > >  > 
 > >  > Because we cleanup on mm exit, not task exit, right? OK, looks like I 
 > >  > originally spent a few more thoughts than this time :-/.
 > >  > 
 > >  > > 
 > >  > >  > 
 > >  > >  > > 
 > >  > >  > >  Could you
 > >  > >  > >  > live without the check until we have per-process fd tabled, or was it
 > >  > >  > >  > essential for the select thing?
 > >  > >  > > 
 > >  > >  > > An application which I ported to Xenomai (and which uses the select
 > >  > >  > > call) closes all file descriptors in a for loop. The purpose of this
 > >  > >  > > loop is, I guess, to avoid leaving a file descriptor opened that was
 > >  > >  > > passed through exec.
 > >  > >  > 
 > >  > >  > OK.
 > >  > >  > 
 > >  > >  > So, will you change rtdm_process too? Thanks.
 > >  > > 
 > >  > > I commited the select support, without any change to rtdm_context_get or
 > >  > > rtdm_process. So, now, how do you prefer this to be fixed, by adding an
 > >  > > mm struct to the rtdm_process struct ? By the way, after thinking about
 > >  > > it I can live without this fix: I just have to stop the loop closing
 > >  > > file descriptors at 768, so that it will not try to close RTDM file
 > >  > > descriptors.
 > >  > 
 > >  > If you can live with it, I would vote for fixing it by the intended 
 > >  > redesign via per-process fds.
 > > 
 > > Ok.
 > > 
 > >  > 
 > >  > > 
 > >  > > While commiting the support for select, I also had a dependency problem
 > >  > > in Kconfig: when support for posix select is enabled the posix module
 > >  > > uses a function defined in the RTDM module. So, there is one invalid
 > >  > > configuration: posix built-in with support for select and rtdm built as
 > >  > > a module. I could not find a way to express this condition in the
 > >  > > Kconfig language, so I just made a comment depend on this condition, but
 > >  > > would be happy if anyone found a better solution.
 > >  > 
 > >  > I would say:
 > >  > 
 > >  > config POSIX
 > >  > 	select RTDM if OPT_SELECT
 > > 
 > > But in this case, I can not have posix with select and without rtdm.
 > 
 > Then you need to isolate those services that POSIX needs from RTDM. The 
 > above just expresses the dependency you described. 

Then I was wrong in what I described: the problem is that if posix is
built-in and RTDM is enabled, then RTDM must be built-in.

In the end this just 
 > shows that we have to define the common fd-ground for both skins in the 
 > core.

I also have the choice of defining the service needed
(rt_dev_select_bind) as a callback in the posix module, the RTDM module
setting this callback when loaded (like what the rtcap module does with
rtnet). But I wanted something simple, so I aimed at Kconfig stuff.

-- 


					    Gilles Chanteperdrix.

Re: [Xenomai-core] [patch 2/4] RTDM support for select-like service.

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 4014 bytes --]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>  > Gilles Chanteperdrix wrote:
>  > > Jan Kiszka wrote:
>  > >  > Gilles Chanteperdrix wrote:
>  > >  > > Jan Kiszka wrote:
>  > >  > >  > Gilles Chanteperdrix wrote:
>  > >  > >  > > Would not it be simpler to put a pointer to the task_struct ? After all,
>  > >  > >  > > it already has a pid, comm and mm, and a file descriptor will not
>  > >  > >  > > survive a task_struct thanks to automatic closing of file descriptors.
>  > >  > >  > 
>  > >  > >  > Hmm, hmm, hmmmm... Sounds reasonable, should be safe.
>  > >  > > 
>  > >  > > Actually no, we can not do that because a task_struct may well disappear
>  > >  > > and the rtdm_process continue to exist as long as another thread uses the
>  > >  > > same mm.
>  > >  > 
>  > >  > Because we cleanup on mm exit, not task exit, right? OK, looks like I 
>  > >  > originally spent a few more thoughts than this time :-/.
>  > >  > 
>  > >  > > 
>  > >  > >  > 
>  > >  > >  > > 
>  > >  > >  > >  Could you
>  > >  > >  > >  > live without the check until we have per-process fd tabled, or was it
>  > >  > >  > >  > essential for the select thing?
>  > >  > >  > > 
>  > >  > >  > > An application which I ported to Xenomai (and which uses the select
>  > >  > >  > > call) closes all file descriptors in a for loop. The purpose of this
>  > >  > >  > > loop is, I guess, to avoid leaving a file descriptor opened that was
>  > >  > >  > > passed through exec.
>  > >  > >  > 
>  > >  > >  > OK.
>  > >  > >  > 
>  > >  > >  > So, will you change rtdm_process too? Thanks.
>  > >  > > 
>  > >  > > I commited the select support, without any change to rtdm_context_get or
>  > >  > > rtdm_process. So, now, how do you prefer this to be fixed, by adding an
>  > >  > > mm struct to the rtdm_process struct ? By the way, after thinking about
>  > >  > > it I can live without this fix: I just have to stop the loop closing
>  > >  > > file descriptors at 768, so that it will not try to close RTDM file
>  > >  > > descriptors.
>  > >  > 
>  > >  > If you can live with it, I would vote for fixing it by the intended 
>  > >  > redesign via per-process fds.
>  > > 
>  > > Ok.
>  > > 
>  > >  > 
>  > >  > > 
>  > >  > > While commiting the support for select, I also had a dependency problem
>  > >  > > in Kconfig: when support for posix select is enabled the posix module
>  > >  > > uses a function defined in the RTDM module. So, there is one invalid
>  > >  > > configuration: posix built-in with support for select and rtdm built as
>  > >  > > a module. I could not find a way to express this condition in the
>  > >  > > Kconfig language, so I just made a comment depend on this condition, but
>  > >  > > would be happy if anyone found a better solution.
>  > >  > 
>  > >  > I would say:
>  > >  > 
>  > >  > config POSIX
>  > >  > 	select RTDM if OPT_SELECT
>  > > 
>  > > But in this case, I can not have posix with select and without rtdm.
>  > 
>  > Then you need to isolate those services that POSIX needs from RTDM. The 
>  > above just expresses the dependency you described. 
> 
> Then I was wrong in what I described: the problem is that if posix is
> built-in and RTDM is enabled, then RTDM must be built-in.

That should be covered by the kconfig rule automatically.

> 
> In the end this just 
>  > shows that we have to define the common fd-ground for both skins in the 
>  > core.
> 
> I also have the choice of defining the service needed
> (rt_dev_select_bind) as a callback in the posix module, the RTDM module
> setting this callback when loaded (like what the rtcap module does with
> rtnet). But I wanted something simple, so I aimed at Kconfig stuff.
> 

If you can live with the callback being NULL, you could also perfectly 
wrap some ifdef CONFIG_...RTDM[_MODULE] around the current invocations. 
Then you don't need the dependency above. I think I have to look at the 
code...

Jan


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

Re: [Xenomai-core] [patch 2/4] RTDM support for select-like service.

[Jan Kiszka]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> I also have the choice of defining the service needed
>> (rt_dev_select_bind) as a callback in the posix module, the RTDM module
>> setting this callback when loaded (like what the rtcap module does with
>> rtnet). But I wanted something simple, so I aimed at Kconfig stuff.
>>
> 
> If you can live with the callback being NULL, you could also perfectly
> wrap some ifdef CONFIG_...RTDM[_MODULE] around the current invocations.
> Then you don't need the dependency above. I think I have to look at the
> code...
> 

I understood the core of your problem meanwhile. Here is a better
suggestion:

Index: ksrc/skins/posix/Kconfig
===================================================================
--- ksrc/skins/posix/Kconfig	(Revision 3499)
+++ ksrc/skins/posix/Kconfig	(Arbeitskopie)
@@ -47,9 +47,15 @@ config XENO_OPT_POSIX_INTR
 	RTDM skin is the preferred way of implementing drivers), leave this
 	option unselected.
 
+if XENO_SKIN_RTDM = m && XENO_SKIN_RTDM != XENO_SKIN_POSIX
+	comment "Note: Select support only available if the POSIX skin is built"
+	comment "as module or the RTDM skin is built into the kernel as well."
+endif
+
 config XENO_OPT_POSIX_SELECT
 	bool "Select syscall"
 	select XENO_OPT_SELECT
+	depends on XENO_SKIN_RTDM != m || XENO_SKIN_RTDM = XENO_SKIN_POSIX
 	help
 
 	This option allows applications using the Xenomai POSIX skin in
@@ -65,10 +71,3 @@ config XENO_OPT_DEBUG_POSIX
 	cleaned up POSIX objects it detects on process termination.
 
 endif
-
-if XENO_OPT_POSIX_SELECT && XENO_SKIN_POSIX = y && XENO_SKIN_RTDM = m
-	comment "This configuration will not work ! If you enable the POSIX"
-	comment "select syscall while the POSIX skin is built-in the kernel,"
-	comment "you have to compile the POSIX skin as a module or the RTDM"
-	comment "skin built-in the kernel."
-endif

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

Re: [Xenomai-core] [patch 2/4] RTDM support for select-like service.

[Gilles Chanteperdrix]

On Tue, Feb 12, 2008 at 11:33 AM, Jan Kiszka <jan.kiszka@domain.hid> wrote:
> Jan Kiszka wrote:
>  > Gilles Chanteperdrix wrote:
>
> >> I also have the choice of defining the service needed
>  >> (rt_dev_select_bind) as a callback in the posix module, the RTDM module
>  >> setting this callback when loaded (like what the rtcap module does with
>  >> rtnet). But I wanted something simple, so I aimed at Kconfig stuff.
>  >>
>  >
>  > If you can live with the callback being NULL, you could also perfectly
>  > wrap some ifdef CONFIG_...RTDM[_MODULE] around the current invocations.
>  > Then you don't need the dependency above. I think I have to look at the
>  > code...
>  >
>
>  I understood the core of your problem meanwhile. Here is a better
>  suggestion:
>
>  Index: ksrc/skins/posix/Kconfig
>  ===================================================================
>  --- ksrc/skins/posix/Kconfig    (Revision 3499)
>  +++ ksrc/skins/posix/Kconfig    (Arbeitskopie)
>  @@ -47,9 +47,15 @@ config XENO_OPT_POSIX_INTR
>         RTDM skin is the preferred way of implementing drivers), leave this
>         option unselected.
>
>  +if XENO_SKIN_RTDM = m && XENO_SKIN_RTDM != XENO_SKIN_POSIX
>  +       comment "Note: Select support only available if the POSIX skin is built"
>  +       comment "as module or the RTDM skin is built into the kernel as well."
>  +endif
>  +
>   config XENO_OPT_POSIX_SELECT
>         bool "Select syscall"
>         select XENO_OPT_SELECT
>  +       depends on XENO_SKIN_RTDM != m || XENO_SKIN_RTDM = XENO_SKIN_POSIX
>         help
>
>         This option allows applications using the Xenomai POSIX skin in
>  @@ -65,10 +71,3 @@ config XENO_OPT_DEBUG_POSIX
>         cleaned up POSIX objects it detects on process termination.
>
>   endif
>  -
>  -if XENO_OPT_POSIX_SELECT && XENO_SKIN_POSIX = y && XENO_SKIN_RTDM = m
>  -       comment "This configuration will not work ! If you enable the POSIX"
>  -       comment "select syscall while the POSIX skin is built-in the kernel,"
>  -       comment "you have to compile the POSIX skin as a module or the RTDM"
>  -       comment "skin built-in the kernel."
>  -endif

This looks much better, thanks, I will use that approach.

-- 
 Gilles Chanteperdrix

[Xenomai-core] [patch 3/4] posix skin kernel-space support for select.

[Gilles Chanteperdrix]

[-- Attachment #1: message body and .signature --]
[-- Type: text/plain, Size: 427 bytes --]


stats:
 include/posix/syscall.h     |    1
 ksrc/skins/posix/Kconfig    |    9 ++
 ksrc/skins/posix/internal.h |   26 ++++++
 ksrc/skins/posix/mq.c       |   79 ++++++++++++++++++-
 ksrc/skins/posix/syscall.c  |  181 ++++++++++++++++++++++++++++++++++++++++++++
 ksrc/skins/posix/thread.c   |    7 +
 ksrc/skins/posix/thread.h   |    4
 7 files changed, 305 insertions(+), 2 deletions(-)

-- 


					    Gilles Chanteperdrix.

[-- Attachment #2: xeno-select-posix-kernel.diff --]
[-- Type: text/plain, Size: 13277 bytes --]

--- include/posix/syscall.h	(revision 3460)
+++ include/posix/syscall.h	(working copy)
@@ -100,6 +100,7 @@
 #define __pse51_condattr_setpshared   74
 #define __pse51_thread_getschedparam  75
 #define __pse51_thread_kill           76
+#define __pse51_select                77
 
 #ifdef __KERNEL__
 
--- ksrc/skins/posix/Kconfig	(revision 3460)
+++ ksrc/skins/posix/Kconfig	(working copy)
@@ -47,6 +47,15 @@ config XENO_OPT_POSIX_INTR
 	RTDM skin is the preferred way of implementing drivers), leave this
 	option unselected.
 
+config XENO_OPT_POSIX_SELECT
+	bool "Select syscall"
+	select XENO_OPT_SELECT
+	help
+
+	This option allows applications using the Xenomai POSIX skin in
+	user-space to use the "select" syscall with Xenomai POSIXK skin file
+	descriptors.
+
 config XENO_OPT_DEBUG_POSIX
 	bool "Debugging support"
 	default y
--- ksrc/skins/posix/syscall.c	(revision 3468)
+++ ksrc/skins/posix/syscall.c	(working copy)
@@ -34,6 +34,9 @@
 #include <posix/sem.h>
 #include <posix/shm.h>
 #include <posix/timer.h>
+#ifdef CONFIG_XENO_SKIN_RTDM
+#include <rtdm/core.h>
+#endif /* CONFIG_XENO_SKIN_RTDM */
 
 int pse51_muxid;
 
@@ -1877,6 +1880,183 @@ static int __timer_getoverrun(struct pt_
 	return rc >= 0 ? rc : -thread_get_errno();
 }
 
+#ifdef CONFIG_XENO_OPT_POSIX_SELECT
+static int fd_valid_p(int fd)
+{
+	pse51_assoc_t *assoc;
+#ifdef CONFIG_XENO_SKIN_RTDM
+	const int rtdm_fd_start = FD_SETSIZE - RTDM_FD_MAX;
+	
+	if (fd >= rtdm_fd_start) {
+		struct rtdm_dev_context *ctx;
+		ctx = __rtdm_context_get(current, fd - rtdm_fd_start);
+		if (ctx) {
+			rtdm_context_unlock(ctx);
+			return 1;
+		}
+		return 0;
+	}
+#endif /* CONFIG_XENO_SKIN_RTDM */
+
+	assoc = pse51_assoc_lookup(&pse51_queues()->uqds, fd);
+	return assoc != NULL;
+}
+
+static int first_fd_valid_p(fd_set *fds[XNSELECT_MAX_TYPES], int nfds)
+{
+	int i, fd;
+
+	for (i = 0; i < XNSELECT_MAX_TYPES; i++)
+		if (fds[i]
+		    && (fd = find_first_bit(fds[i]->fds_bits, nfds)) < nfds)
+			return fd_valid_p(fd);
+
+	return 1;
+}
+
+static int select_bind_one(struct xnselector *selector, unsigned type, int fd)
+{
+	pse51_assoc_t *assoc;
+#ifdef CONFIG_XENO_SKIN_RTDM
+	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);
+#endif /* CONFIG_XENO_SKIN_RTDM */
+
+	assoc = pse51_assoc_lookup(&pse51_queues()->uqds, fd);
+	if (!assoc)
+		return -EBADF;
+
+	return pse51_mq_select_bind(assoc2ufd(assoc)->kfd, selector, type, fd);
+}
+
+static int select_bind_all(struct xnselector *selector,
+			   fd_set *fds[XNSELECT_MAX_TYPES], int nfds)
+{
+	unsigned fd, type;
+	int err;
+
+	for (type = 0; type < XNSELECT_MAX_TYPES; type++) {
+		fd_set *set = fds[type];
+		if (set)
+			for (fd = find_first_bit(set->fds_bits, nfds);
+			     fd < nfds;
+			     fd = find_next_bit(set->fds_bits, nfds, fd + 1)) {
+				err = select_bind_one(selector, type, fd);
+				if (err)
+					return err;
+			}
+	}
+
+	return 0;
+}
+
+/* int select(int, fd_set *, fd_set *, fd_set *, struct timeval *) */
+static int __select(struct pt_regs *regs)
+{
+	fd_set __user *ufd_sets[XNSELECT_MAX_TYPES] = {
+		[XNSELECT_READ] = (fd_set __user *) __xn_reg_arg2(regs),
+		[XNSELECT_WRITE] = (fd_set __user *) __xn_reg_arg3(regs),
+		[XNSELECT_EXCEPT] = (fd_set __user *) __xn_reg_arg4(regs)
+	};
+	fd_set *in_fds[XNSELECT_MAX_TYPES] = {NULL, NULL, NULL};
+	fd_set *out_fds[XNSELECT_MAX_TYPES] = {NULL, NULL, NULL};
+	fd_set in_fds_storage[XNSELECT_MAX_TYPES],
+		out_fds_storage[XNSELECT_MAX_TYPES];
+	xnticks_t timeout = XN_INFINITE;
+	xntmode_t mode = XN_RELATIVE;
+	struct xnselector *selector;
+	struct timeval tv;
+	pthread_t thread;
+	int i, err, nfds;
+
+	thread = pse51_current_thread();
+	if (!thread)
+		return -EPERM;
+
+	if (__xn_reg_arg5(regs)) {
+		if (__xn_copy_from_user(&tv,
+					(void __user *)__xn_reg_arg5(regs),
+					sizeof(tv)))
+			return -EFAULT;
+
+		if (tv.tv_usec > 1000000)
+			return -EINVAL;
+
+		timeout = clock_get_ticks(CLOCK_MONOTONIC) + tv2ticks_ceil(&tv);
+		mode = XN_ABSOLUTE;
+	}
+
+	nfds = __xn_reg_arg1(regs);
+
+	for (i = 0; i < XNSELECT_MAX_TYPES; i++)
+		if (ufd_sets[i]) {
+			in_fds[i] = &in_fds_storage[i];
+			out_fds[i] = & out_fds_storage[i];
+			if (__xn_copy_from_user(in_fds[i],
+						(void __user *) ufd_sets[i],
+						__FDELT(nfds + __NFDBITS - 1)
+						* sizeof(long)))
+				return -EFAULT;
+		}
+
+	selector = thread->selector;
+	if (!selector) {
+		/* This function may be called from pure Linux fd_sets, we want
+		   to avoid the xnselector allocation in this case, so, we do a
+		   simple test: test if the first file descriptor we find in the
+		   fd_set is an RTDM descriptor or a message queue descriptor. */
+		if (!first_fd_valid_p(in_fds, nfds))
+			return -EBADF;
+
+		if (!(selector = xnmalloc(sizeof(*thread->selector))))
+			return -ENOMEM;
+		xnselector_init(selector);
+		thread->selector = selector;
+
+		/* Bind directly the file descriptors, we do not need to go
+		   through xnselect returning -ECHRNG */
+		if ((err = select_bind_all(selector, in_fds, nfds)))
+			return err;
+	}
+
+	do {
+		err = xnselect(selector, out_fds, in_fds, nfds, timeout, mode);
+
+		if (err == -ECHRNG) {
+			int err = select_bind_all(selector, out_fds, nfds);
+			if (err)
+				return err;
+		}
+	} while (err == -ECHRNG);
+
+	if (__xn_reg_arg5(regs) && (err > 0 || err == -EINTR)) {
+		xnsticks_t diff = timeout - clock_get_ticks(CLOCK_MONOTONIC);
+		if (diff > 0)
+			ticks2tv(&tv, diff);
+		else
+			tv.tv_sec = tv.tv_usec = 0;
+
+		if (__xn_copy_to_user((void __user *)__xn_reg_arg5(regs),
+				      &tv, sizeof(tv)))
+			return -EFAULT;
+	}
+
+	if (err > 0)
+		for (i = 0; i < XNSELECT_MAX_TYPES; i++)
+			if (ufd_sets[i]
+			    && __xn_copy_to_user((void __user *) ufd_sets[i],
+						 out_fds[i], sizeof(fd_set)))
+				return -EFAULT;
+	return err;
+}
+#else /* !CONFIG_XENO_OPT_POSIX_SELECT */
+#define __select __pse51_call_not_available
+#endif /* !CONFIG_XENO_OPT_POSIX_SELECT */
+
 #ifdef CONFIG_XENO_OPT_POSIX_SHM
 
 /* shm_open(name, oflag, mode, ufd) */
@@ -2275,6 +2455,7 @@ static xnsysent_t __systab[] = {
 	    {&__pthread_condattr_getpshared, __xn_exec_any},
 	[__pse51_condattr_setpshared] =
 	    {&__pthread_condattr_setpshared, __xn_exec_any},
+	[__pse51_select] = {&__select, __xn_exec_primary},
 };
 
 static void __shadow_delete_hook(xnthread_t *thread)
--- ksrc/skins/posix/mq.c	(revision 3468)
+++ ksrc/skins/posix/mq.c	(working copy)
@@ -65,6 +65,8 @@ struct pse51_mq {
 
 	xnholder_t link;	/* link in mqq */
 
+	DECLARE_XNSELECT(read_select);
+	DECLARE_XNSELECT(write_select);
 #define link2mq(laddr) \
     ((pse51_mq_t *) (((char *)laddr) - offsetof(pse51_mq_t, link)))
 };
@@ -140,6 +142,8 @@ static int pse51_mq_init(pse51_mq_t * mq
 
 	mq->attr = *attr;
 	mq->target = NULL;
+	xnselect_init(&mq->read_select);
+	xnselect_init(&mq->write_select);
 
 	return 0;
 }
@@ -154,6 +158,8 @@ static void pse51_mq_destroy(pse51_mq_t 
 	resched = (xnsynch_destroy(&mq->senders) == XNSYNCH_RESCHED) || resched;
 	removeq(&pse51_mqq, &mq->link);
 	xnlock_put_irqrestore(&nklock, s);
+	xnselect_destroy(&mq->read_select);
+	xnselect_destroy(&mq->write_select);
 #ifdef __KERNEL__
 	if (!xnpod_root_p())
 		pse51_schedule_lostage(PSE51_LO_FREE_REQ, mq->mem, mq->memsize);
@@ -480,6 +486,9 @@ static pse51_msg_t *pse51_mq_trysend(pse
 	if (!msg)
 		return ERR_PTR(-EAGAIN);
 
+	if (countq(&mq->avail) == 0)
+		xnselect_signal(&mq->write_select, 0);
+
 	*mqp = mq;
 	mq->nodebase.refcount++;
 	return msg;
@@ -504,6 +513,9 @@ static pse51_msg_t *pse51_mq_tryrcv(pse5
 	if (!(holder = getpq(&mq->queued)))
 		return ERR_PTR(-EAGAIN);
 
+	if (countpq(&mq->queued) == 0)
+		xnselect_signal(&mq->read_select, 0);
+
 	*mqp = mq;
 	mq->nodebase.refcount++;
 	return any2msg(holder, link);
@@ -595,13 +607,16 @@ int pse51_mq_finish_send(mqd_t fd, pse51
 	}
 
 	insertpqf(&mq->queued, &msg->link, msg->link.prio);
+	if (countpq(&mq->queued) == 1)
+		resched = xnselect_signal(&mq->read_select, 1);
 
 	if (xnsynch_wakeup_one_sleeper(&mq->receivers))
 		resched = 1;
 	else if (mq->target && countpq(&mq->queued) == 1) {
 		/* First message ? no pending reader ? attempt
 		   to send a signal if mq_notify was called. */
-		resched = pse51_sigqueue_inner(mq->target, &mq->si);
+		if (pse51_sigqueue_inner(mq->target, &mq->si))
+			resched = 1;
 		mq->target = NULL;
 	}
 
@@ -626,6 +641,9 @@ int pse51_mq_finish_send(mqd_t fd, pse51
 	   pool and wakeup any waiting sender. */;
 	pse51_mq_msg_free(mq, msg);
 
+	if (countq(&mq->avail) == 1)
+		resched = xnselect_signal(&mq->write_select, 1);
+
 	if (xnsynch_wakeup_one_sleeper(&mq->senders))
 		resched = 1;
 	goto unref;
@@ -714,6 +732,9 @@ int pse51_mq_finish_rcv(mqd_t fd, pse51_
 
 	pse51_mq_msg_free(mq, msg);
 
+	if (countq(&mq->avail) == 1)
+		resched = xnselect_signal(&mq->write_select, 1);
+
 	if (xnsynch_wakeup_one_sleeper(&mq->senders))
 		resched = 1;
 
@@ -1210,6 +1231,62 @@ int mq_notify(mqd_t fd, const struct sig
 	return -1;
 }
 
+#ifdef CONFIG_XENO_OPT_POSIX_SELECT
+int pse51_mq_select_bind(mqd_t fd, struct xnselector *selector,
+			 unsigned type, unsigned index)
+{
+	struct xnselect_binding *binding;
+	pse51_desc_t *desc;
+	pse51_mq_t *mq;
+	int err;
+	spl_t s;
+	
+	if (type == XNSELECT_READ || type == XNSELECT_WRITE) {
+		binding = xnmalloc(sizeof(*binding));
+		if (!binding)
+			return -ENOMEM;
+	} else
+		return -EBADF;
+
+	xnlock_get_irqsave(&nklock, s);
+	err = -pse51_desc_get(&desc, fd, PSE51_MQ_MAGIC);
+	if (err)
+		goto unlock_and_error;
+
+	mq = node2mq(pse51_desc_node(desc));
+
+	switch(type) {
+	case XNSELECT_READ:
+		err = -EBADF;
+		if ((pse51_desc_getflags(desc) & PSE51_PERMS_MASK) == O_WRONLY)
+			goto unlock_and_error;
+
+		err = xnselect_bind(&mq->read_select, binding,
+				    selector, type, index, countpq(&mq->queued));
+		if (err)
+			goto unlock_and_error;
+		break;
+
+	case XNSELECT_WRITE:
+		err = -EBADF;
+		if ((pse51_desc_getflags(desc) & PSE51_PERMS_MASK) == O_RDONLY)
+			goto unlock_and_error;
+
+		err = xnselect_bind(&mq->write_select, binding,
+				    selector, type, index, countq(&mq->avail));
+		if (err)
+			goto unlock_and_error;
+		break;
+	}
+	xnlock_put_irqrestore(&nklock, s);
+	return 0;
+
+      unlock_and_error:
+	xnlock_put_irqrestore(&nklock, s);
+	return err;
+}
+#endif /* CONFIG_XENO_OPT_POSIX_SELECT */
+
 #ifdef CONFIG_XENO_OPT_PERVASIVE
 static void uqd_cleanup(pse51_assoc_t *assoc)
 {
--- ksrc/skins/posix/thread.c	(revision 3460)
+++ ksrc/skins/posix/thread.c	(working copy)
@@ -84,6 +84,12 @@ static void thread_delete_hook(xnthread_
 	pse51_mark_deleted(thread);
 	pse51_signal_cleanup_thread(thread);
 	pse51_timer_cleanup_thread(thread);
+#ifdef CONFIG_XENO_OPT_POSIX_SELECT
+	if (thread->selector) {
+		xnselector_destroy(thread->selector);
+		thread->selector = NULL;
+	}
+#endif /* CONFIG_XENO_OPT_POSIX_SELECT */
 
 	switch (thread_getdetachstate(thread)) {
 	case PTHREAD_CREATE_DETACHED:
@@ -216,6 +222,7 @@ int pthread_create(pthread_t *tid,
 	pse51_signal_init_thread(thread, cur);
 	pse51_tsd_init_thread(thread);
 	pse51_timer_init_thread(thread);
+	thread->selector = NULL;
 
 	if (thread->attr.policy == SCHED_RR) {
 		xnthread_time_slice(&thread->threadbase) = pse51_time_slice;
--- ksrc/skins/posix/thread.h	(revision 3460)
+++ ksrc/skins/posix/thread.h	(working copy)
@@ -21,6 +21,7 @@
 #define _POSIX_THREAD_H
 
 #include <posix/internal.h>
+#include <nucleus/select.h>
 
 typedef unsigned long long pse51_sigset_t;
 
@@ -90,6 +91,9 @@ struct pse51_thread {
     /* For timers. */
     xnqueue_t timersq;
     
+    /* For select. */
+    struct xnselector *selector;
+
 #ifdef CONFIG_XENO_OPT_PERVASIVE
     struct pse51_hkey hkey;
 #endif /* CONFIG_XENO_OPT_PERVASIVE */
--- ksrc/skins/posix/internal.h	(revision 3468)
+++ ksrc/skins/posix/internal.h	(working copy)
@@ -22,6 +22,7 @@
 #include <nucleus/xenomai.h>
 #include <nucleus/core.h>
 #include <nucleus/ppd.h>
+#include <nucleus/select.h>
 #include <posix/posix.h>
 #include <posix/registry.h>
 
@@ -152,12 +153,32 @@ static inline xnticks_t ts2ticks_ceil(co
 	return rem ? ticks+1 : ticks;
 }
 
+static inline xnticks_t tv2ticks_ceil(const struct timeval *tv)
+{
+	xntime_t nsecs = tv->tv_usec * 1000;
+	unsigned long rem;
+	xnticks_t ticks;
+	if(tv->tv_sec)
+		nsecs += (xntime_t) tv->tv_sec * ONE_BILLION;
+	ticks = xnarch_ulldiv(nsecs, xntbase_get_tickval(pse51_tbase), &rem);
+	return rem ? ticks+1 : ticks;
+}
+
+static inline void ticks2tv(struct timeval *tv, xnticks_t ticks)
+{
+	unsigned long nsecs;
+	tv->tv_sec = xnarch_uldivrem(xntbase_ticks2ns(pse51_tbase, ticks),
+				     ONE_BILLION,
+				     &nsecs);
+	tv->tv_usec = nsecs / 1000;
+}
+
 static inline xnticks_t clock_get_ticks(clockid_t clock_id)
 {
 	if(clock_id == CLOCK_REALTIME)
 		return xntbase_get_time(pse51_tbase);
 	else
-		return xntbase_ns2ticks(pse51_tbase, xnpod_get_cpu_time());
+		return xntbase_get_jiffies(pse51_tbase);
 }
 
 static inline int clock_flag(int flag, clockid_t clock_id)
@@ -178,4 +199,7 @@ static inline int clock_flag(int flag, c
 	return -EINVAL;
 }
 
+int pse51_mq_select_bind(mqd_t fd, struct xnselector *selector,
+			 unsigned type, unsigned index);
+
 #endif /* !_POSIX_INTERNAL_H */

[Xenomai-core] [patch 4/4] posix skin user-space support for select.

[Gilles Chanteperdrix]

[-- Attachment #1: message body and .signature --]
[-- Type: text/plain, Size: 341 bytes --]


stats:
 include/posix/sys/select.h     |   23 +++++++++++++++++++++++
 src/skins/posix/Makefile.am    |    1 +
 src/skins/posix/posix.wrappers |    1 +
 src/skins/posix/select.c       |   26 ++++++++++++++++++++++++++
 src/skins/posix/wrappers.c     |    9 +++++++++
 5 files changed, 60 insertions(+)

-- 


					    Gilles Chanteperdrix.

[-- Attachment #2: xeno-select-posix-user.diff --]
[-- Type: text/plain, Size: 2365 bytes --]

--- include/posix/sys/select.h	(revision 0)
+++ include/posix/sys/select.h	(revision 0)
@@ -0,0 +1,23 @@
+#ifndef _XENO_POSIX_SELECT_H
+#define _XENO_POSIX_SELECT_H
+
+#if !(defined(__KERNEL__) || defined(__XENO_SIM__))
+
+#include_next <sys/select.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+extern int __real_select (int __nfds, fd_set *__restrict __readfds,
+			  fd_set *__restrict __writefds,
+			  fd_set *__restrict __exceptfds,
+			  struct timeval *__restrict __timeout);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* !(__KERNEL__ || __XENO_SIM__) */
+
+#endif /* _XENO_POSIX_SELECT_H */
--- src/skins/posix/wrappers.c	(revision 3460)
+++ src/skins/posix/wrappers.c	(working copy)
@@ -30,6 +30,7 @@
 #include <unistd.h>
 #include <sys/ioctl.h>
 #include <sys/mman.h>
+#include <sys/select.h>
 
 /* sched */
 int __real_pthread_setschedparam(pthread_t thread,
@@ -235,3 +236,11 @@ int __real_munmap(void *addr, size_t len
 {
 	return munmap(addr, len);
 }
+
+int __real_select (int __nfds, fd_set *__restrict __readfds,
+		   fd_set *__restrict __writefds,
+		   fd_set *__restrict __exceptfds,
+		   struct timeval *__restrict __timeout)
+{
+	return select(__nfds, __readfds, __writefds, __exceptfds, __timeout);
+}
--- src/skins/posix/select.c	(revision 0)
+++ src/skins/posix/select.c	(revision 0)
@@ -0,0 +1,26 @@
+#include <errno.h>
+#include <posix/syscall.h>
+#include <sys/select.h>
+
+extern int __pse51_muxid;
+
+int __wrap_select (int __nfds, fd_set *__restrict __readfds,
+		   fd_set *__restrict __writefds,
+		   fd_set *__restrict __exceptfds,
+		   struct timeval *__restrict __timeout)
+{
+	int err;
+
+	err = XENOMAI_SKINCALL5(__pse51_muxid, __pse51_select, __nfds,
+				__readfds, __writefds, __exceptfds, __timeout);
+
+	if (err == -EBADF || err == -EPERM || err == -ENOSYS)
+		return __real_select(__nfds, __readfds,
+				     __writefds, __exceptfds, __timeout);
+
+	if (err > 0)
+		return err;
+
+	errno = -err;
+	return -1;
+}
--- src/skins/posix/posix.wrappers	(revision 3460)
+++ src/skins/posix/posix.wrappers	(working copy)
@@ -87,3 +87,4 @@
 --wrap shm_unlink
 --wrap mmap
 --wrap munmap
+--wrap select
--- src/skins/posix/Makefile.am	(revision 3460)
+++ src/skins/posix/Makefile.am	(working copy)
@@ -15,6 +15,7 @@ libpthread_rt_la_SOURCES = \
 	mutex.c \
 	shm.c \
 	interrupt.c \
+	select.c \
 	rtdm.c \
 	wrappers.c
 

Re: [Xenomai-core] [patch 0/4] Support for select-like services.

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1484 bytes --]

Gilles Chanteperdrix wrote:
> Hi,
> 
> here comes a third edition of the patchset adding support for select. 
> 
> The core support now depends on a CONFIG_XENO_OPT_SELECT option, whereas the
> posix support depends on a CONFIG_XENO_OPT_POSIX_SELECT option.
> 
> The support for using rtdm_event_t and rtdm_sem_t objects is now built-in to
> these objects without creating new objects types (only if
> CONFIG_XENO_OPT_SELECT is enabled).
> 
> The rtdm_context_get modification, allowing to check that the caller of a service
> belongs to the same process as the file descriptor it uses, is now made in
> __rtdm_context_get while rtdm_context_get passes a NULL first argument to this
> new service.
> 
> When implementing these modifications, a few questions arise:
> - RTDM support currently depends on the CONFIG_XENO_OPT_SELECT option, should we
> add an RTDM specific option ?

IIRC, the policy is so far that the skin selects the nucleus features it
wants/needs, so we may better add the visible switch to RTDM as well.

> - in order to avoid numerous #ifdefs, and especially in drivers code, the new
> callback added to device operations for binding RTDM file descriptors to
> selector blocks is always there even if CONFIG_XENO_OPT_SELECT is disabled. Is
> it the right thing to do ?

That's ok, otherwise ifdef'ery would crawl into the drivers as well.
Given the conversion you posted for RTnet, we don't add noticeable bloat
this way.

Jan


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

Re: [Xenomai-core] [patch 0/4] Support for select-like services.

[Johan Borkhuis]

Gilles,

Gilles Chanteperdrix wrote:
> Hi,
>
> here comes a third edition of the patchset adding support for select. 
>   
Could you tell me against which version this patch is tested? Would it 
work with 2.4.1, or do I need the latest version from SVN?

Kind regards,
    Johan Borkhuis

-- 
Johan Borkhuis                                  Dutch Space BV
email:        j.borkhuis@domain.hid          Newtonweg 1
phone:        071-5245788                       Leiden
fax:          071-5245499                       The Netherlands

Re: [Xenomai-core] [patch 0/4] Support for select-like services.

[Gilles Chanteperdrix]

On Mon, Feb 11, 2008 at 8:28 AM, Johan Borkhuis
<j.borkhuis@domain.hid> wrote:
> Gilles,
>
>
>  Gilles Chanteperdrix wrote:
>  > Hi,
>  >
>  > here comes a third edition of the patchset adding support for select.
>  >
>  Could you tell me against which version this patch is tested? Would it
>  work with 2.4.1, or do I need the latest version from SVN?

The patch is against trunk. We would need to rework it a bit to adapt
it to 2.4.1, but since it adds a new syscall, I do not know if it will
be backported, though adding a syscall does not really break the ABI.

-- 
                                               Gilles Chanteperdrix

Re: [Xenomai-core] [patch 0/4] Support for select-like services.

[Gilles Chanteperdrix]

On Mon, Feb 11, 2008 at 8:28 AM, Johan Borkhuis
<j.borkhuis@domain.hid> wrote:
> Gilles,
>
>
>  Gilles Chanteperdrix wrote:
>  > Hi,
>  >
>  > here comes a third edition of the patchset adding support for select.
>  >
>  Could you tell me against which version this patch is tested? Would it
>  work with 2.4.1, or do I need the latest version from SVN?

Hi Johan,

I am getting back to this issue, how much would you need the select
service ? It is a posix only service for now, is it ok for you ?

Regards.

-- 
 Gilles Chanteperdrix

[Xenomai-core] [PATCH 0/4] Support for select-like services.

[Gilles Chanteperdrix]

Hi,

the following patchset adds core support to Xenomai for select-like
services, modifies the rtdm skin to allow running easily these services
for rtdm file descriptors and implement the select service for the posix
skin. select is an important service on low-end platform where it allows
more efficient implementation of network server than with a threaded
approach.


It was only compile-tested for now, and will be tested around next week,
so is submitted here for review only.

Regards.

-- 


					    Gilles Chanteperdrix.

Re: [Xenomai-core] [PATCH 0/4] Support for select-like services.

[Gilles Chanteperdrix]

[-- Attachment #1: message body and .signature --]
[-- Type: text/plain, Size: 292 bytes --]

Gilles Chanteperdrix wrote:
 > 
 > Hi,
 > 
 > the following patchset adds core support to Xenomai for select-like
 > services,

The attached patch is a quick and dirty patch of RTnet to allow using
rtnet sockets of all types with the new select service.

-- 


					    Gilles Chanteperdrix.

[-- Attachment #2: rtnet-select.diff --]
[-- Type: text/plain, Size: 4317 bytes --]

Index: stack/ipv4/af_inet.c
===================================================================
--- stack/ipv4/af_inet.c	(r���vision 6193)
+++ stack/ipv4/af_inet.c	(copie de travail)
@@ -259,8 +259,6 @@ static void rt_ip_ifdown(struct rtnet_de
     rt_ip_route_del_all(rtdev);
 }
 
-
-
 static struct rtdev_event_hook  rtdev_hook = {
     register_device:    NULL,
     unregister_device:  NULL,
@@ -294,6 +292,7 @@ static struct rtdm_device ipv4_device = 
         ioctl_nrt:      rt_udp_ioctl,
         recvmsg_rt:     rt_udp_recvmsg,
         sendmsg_rt:     rt_udp_sendmsg,
+	select_bind:    rt_socket_select_bind,
 #ifdef CONFIG_RTNET_RTDM_SELECT
         poll_rt:        rt_udp_poll,
         /* there should be only the function poll() */
Index: stack/include/rtnet_socket.h
===================================================================
--- stack/include/rtnet_socket.h	(r���vision 6193)
+++ stack/include/rtnet_socket.h	(copie de travail)
@@ -51,7 +51,8 @@ struct rtsocket {
     unsigned int            priority;
     nanosecs_rel_t          timeout;    /* receive timeout, 0 for infinite */
 
-    rtdm_sem_t              pending_sem;
+    rtdm_select_sem_t       pending_sem;
+    struct xnselect         write_select;
 #ifdef CONFIG_RTNET_RTDM_SELECT
     wait_queue_primitive_t  *wakeup_select; /* for selecting calls - this
                                                SHOULD be the head of a wait
@@ -96,6 +97,10 @@ static inline struct rtdm_dev_context *r
     atomic_dec(&(rt_socket_context(sock)->close_lock_count))
 
 extern int rt_socket_init(struct rtdm_dev_context *context);
+extern int rt_socket_select_bind(struct rtdm_dev_context *context,
+				 struct xnselector *selector,
+				 unsigned type,
+				 unsigned index);
 extern int rt_socket_cleanup(struct rtdm_dev_context *context);
 extern int rt_socket_common_ioctl(struct rtdm_dev_context *context,
                                   rtdm_user_info_t *user_info,
Index: stack/packet/af_packet.c
===================================================================
--- stack/packet/af_packet.c	(r���vision 6193)
+++ stack/packet/af_packet.c	(copie de travail)
@@ -467,7 +467,8 @@ static struct rtdm_device   packet_proto
         ioctl_rt:       rt_packet_ioctl,
         ioctl_nrt:      rt_packet_ioctl,
         recvmsg_rt:     rt_packet_recvmsg,
-        sendmsg_rt:     rt_packet_sendmsg
+        sendmsg_rt:     rt_packet_sendmsg,
+	select_bind:    rt_socket_select_bind,	
     },
 
     device_class:       RTDM_CLASS_NETWORK,
Index: stack/socket.c
===================================================================
--- stack/socket.c	(r���vision 6193)
+++ stack/socket.c	(copie de travail)
@@ -68,6 +68,7 @@ int rt_socket_init(struct rtdm_dev_conte
 
     rtdm_lock_init(&sock->param_lock);
     rtdm_sem_init(&sock->pending_sem, 0);
+    xnselect_init(&sock->write_select);
 
     if (test_bit(RTDM_CREATED_IN_NRT, &sockctx->context_flags))
         pool_size = rtskb_pool_init(&sock->skb_pool, socket_rtskbs);
@@ -87,7 +88,44 @@ int rt_socket_init(struct rtdm_dev_conte
     return 0;
 }
 
+int rt_socket_select_bind(struct rtdm_dev_context *context,
+			  struct xnselector *selector,
+			  unsigned type,
+			  unsigned index)
+{
+    struct rtsocket     *sock = (struct rtsocket *)&context->dev_private;
+
+    switch(type) {
+    case XNSELECT_READ:
+	return rtdm_select_sem_bind(&sock->pending_sem, selector, type, index);
+
+    case XNSELECT_WRITE: {
+	struct xnselect_binding *binding;
+	int err;
+	spl_t s;
+
+	binding = xnmalloc(sizeof(*binding));
+	if (!binding)
+	    return -ENOMEM;
+
+	xnlock_get_irqsave(&nklock, s);
+	/* Writing to a socket is always possible, so the state is 1, forever. */
+	err = xnselect_bind(&sock->write_select,
+			    binding, selector, type, index, 1);
+	xnlock_put_irqrestore(&nklock, s);
+
+	if (err)
+	    xnfree(binding);
+	return err;
+    }
 
+    case XNSELECT_EXCEPT:
+	return -EBADF;
+    }
+
+    return -EINVAL;
+}
+EXPORT_SYMBOL(rt_socket_select_bind);
 
 /***
  *  rt_socket_cleanup - releases resources allocated for the socket
@@ -100,6 +138,7 @@ int rt_socket_cleanup(struct rtdm_dev_co
 
 
     rtdm_sem_destroy(&sock->pending_sem);
+    xnselect_destroy(&sock->write_select);
 
     rtdm_lock_get_irqsave(&sock->param_lock, context);