[Xenomai-core] [PATCH] Auto-allocation of minor values for pipe objects

[Xenomai-core] [PATCH] Auto-allocation of minor values for pipe objects

[Dmitry Adamushko]

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



Hello,

enclosed please find a patch that hopefully adds so desired functionality.
I have made various tests with it just now and it seems to work fine.

A size of the bitmap is dependent on XNPIPE_NDEVS parameter in the same
vein as xnpipe_states depends on it; so hopefully that is what you have
meant Philippe (?)

A few comment later since I have to go now.

---
Best regards,
Dmitry

(See attached file: pipe.auto-minor-2.patch)

[-- Attachment #1.2: Type: text/html, Size: 763 bytes --]

[-- Attachment #2: pipe.auto-minor-2.patch --]
[-- Type: application/octet-stream, Size: 3998 bytes --]

diff -ur xenomai/include/nucleus/pipe.h xenomai-pipe2/include/nucleus/pipe.h
--- xenomai/include/nucleus/pipe.h	2005-10-07 23:11:23.000000000 +0200
+++ xenomai-pipe2/include/nucleus/pipe.h	2005-11-15 10:19:19.000000000 +0100
@@ -30,6 +30,8 @@
 #define XNPIPE_NORMAL 0x0
 #define XNPIPE_URGENT 0x1
 
+#define XNPIPE_MINOR_AUTO	-1
+
 #ifdef __KERNEL__
 
 #include <nucleus/queue.h>
diff -ur xenomai/nucleus/pipe.c xenomai-pipe2/nucleus/pipe.c
--- xenomai/nucleus/pipe.c	2005-11-09 19:20:47.000000000 +0100
+++ xenomai-pipe2/nucleus/pipe.c	2005-11-15 18:44:51.000000000 +0100
@@ -43,6 +43,9 @@
 
 xnpipe_state_t xnpipe_states[XNPIPE_NDEVS];
 
+#define XNPIPE_BITMAP_SIZE	((XNPIPE_NDEVS + BITS_PER_LONG - 1) / BITS_PER_LONG)
+static unsigned long xnpipe_bitmap[XNPIPE_BITMAP_SIZE];
+
 xnqueue_t xnpipe_sleepq, xnpipe_asyncq;
 
 int xnpipe_wakeup_apc;
@@ -57,6 +60,37 @@
    #define class_destroy class_simple_destroy
 #endif
 
+/* Allocation of minor values */
+
+static inline int xnpipe_minor_alloc(int minor)
+{
+    spl_t s;
+
+    if ((minor < 0 && minor != XNPIPE_MINOR_AUTO) || minor >= XNPIPE_NDEVS)
+	return -ENODEV;
+
+    xnlock_get_irqsave(&nklock,s);
+
+    if (minor == XNPIPE_MINOR_AUTO)
+	minor = find_first_zero_bit(xnpipe_bitmap,XNPIPE_NDEVS);
+
+    if (minor == XNPIPE_NDEVS || testbits(xnpipe_bitmap[minor / BITS_PER_LONG], 1 << (minor % BITS_PER_LONG)))
+        minor = -EBUSY;
+    else    
+	__setbits(xnpipe_bitmap[minor / BITS_PER_LONG], 1 << (minor % BITS_PER_LONG));
+
+    xnlock_put_irqrestore(&nklock,s);
+    return minor;
+}
+
+static inline void xnpipe_minor_free(int minor)
+{
+    if (minor < 0 || minor >= XNPIPE_NDEVS)
+	return;
+
+    clrbits(xnpipe_bitmap[minor / BITS_PER_LONG], 1 << (minor % BITS_PER_LONG));
+}
+
 /* Get nklock locked before using this macro */
 
 #define xnpipe_read_wait(state,flags)						\
@@ -215,8 +249,9 @@
     int need_sched = 0;
     spl_t s;
 
-    if (minor < 0 || minor >= XNPIPE_NDEVS)
-	return -ENODEV;
+    minor = xnpipe_minor_alloc(minor);
+    if (minor < 0)
+	return minor;
 
     state = &xnpipe_states[minor];
 
@@ -224,12 +259,6 @@
        to prevent using a partially created object */
     xnlock_get_irqsave(&nklock,s);
 
-    if (testbits(state->status,XNPIPE_KERN_CONN))
-	{
-	xnlock_put_irqrestore(&nklock,s);
-	return -EBUSY;
-	}
-    
     setbits(state->status,XNPIPE_KERN_CONN);
     
     xnsynch_init(&state->synchbase,XNSYNCH_FIFO);
@@ -261,7 +290,7 @@
     if (need_sched)
         xnpipe_schedule_request();
 
-    return 0;
+    return minor;
 }
 
 int xnpipe_disconnect (int minor)
@@ -323,6 +352,8 @@
 	    }
 	}
 
+    xnpipe_minor_free(minor);
+
     xnlock_put_irqrestore(&nklock,s);
 
     if (need_sched)
diff -ur xenomai/skins/native/pipe.c xenomai-pipe2/skins/native/pipe.c
--- xenomai/skins/native/pipe.c	2005-11-14 17:11:22.000000000 +0100
+++ xenomai-pipe2/skins/native/pipe.c	2005-11-15 18:35:52.000000000 +0100
@@ -227,12 +227,11 @@
 		    const char *name,
 		    int minor)
 {
-    int err;
+    int err = 0;
 
     if (xnpod_asynch_p())
 	return -EPERM;
 
-    pipe->minor = minor;
     pipe->buffer = NULL;
     pipe->fillsz = 0;
     pipe->flushable = 0;
@@ -240,13 +239,16 @@
     pipe->magic = XENO_PIPE_MAGIC;
     xnobject_copy_name(pipe->name,name);
 
-    err = xnpipe_connect(minor,
+    minor = xnpipe_connect(minor,
 			 &__pipe_output_handler,
 			 NULL,
 			 &__pipe_alloc_handler,
 			 pipe);
-    if (err)
-	return err;
+
+    if (minor < 0)
+	return minor;
+
+    pipe->minor = minor;
 
 #ifdef CONFIG_XENO_OPT_PERVASIVE
     pipe->cpid = 0;
diff -ur xenomai/skins/native/pipe.h xenomai-pipe2/skins/native/pipe.h
--- xenomai/skins/native/pipe.h	2005-11-14 16:01:06.000000000 +0100
+++ xenomai-pipe2/skins/native/pipe.h	2005-11-15 11:34:00.000000000 +0100
@@ -29,6 +29,8 @@
 #define P_NORMAL  XNPIPE_NORMAL
 #define P_URGENT  XNPIPE_URGENT
 
+#define P_MINOR_AUTO	XNPIPE_MINOR_AUTO
+
 typedef struct rt_pipe_placeholder {
     rt_handle_t opaque;
 } RT_PIPE_PLACEHOLDER;

[Xenomai-core] Re: [PATCH] Auto-allocation of minor values for pipe objects

[Philippe Gerum]

Dmitry Adamushko wrote:
> 
> Hello,
> 
> enclosed please find a patch that hopefully adds so desired 
> functionality. I have made various tests with it just now and it seems 
> to work fine.
> 

Sounds good.

> A size of the bitmap is dependent on XNPIPE_NDEVS parameter in the same 
> vein as xnpipe_states depends on it; so hopefully that is what you have 
> meant Philippe (?)
>

And NDEVS still does not depend on BITS_PER_LONG - yes, that's ok.

Two minor missing points :

- Doc update for rt_pipe_create() describing P_MINOR_AUTO
- ChangeLog frag

> A few comment later since I have to go now.
> 
> ---
> Best regards,
> Dmitry
> 
> /(See attached file: pipe.auto-minor-2.patch)/
> 


-- 

Philippe.

[Xenomai-core] Re: [PATCH] Auto-allocation of minor values for pipe objects

[Dmitry Adamushko]

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


Philippe Gerum <rpm@xenomai.org> wrote on 15.11.2005 23:17:50:

> Dmitry Adamushko wrote:
> >
> > Hello,
> >
> > enclosed please find a patch that hopefully adds so desired
> > functionality. I have made various tests with it just now and it seems
> > to work fine.
> >
>
> Sounds good.
>
> > A size of the bitmap is dependent on XNPIPE_NDEVS parameter in the same

> > vein as xnpipe_states depends on it; so hopefully that is what you have

> > meant Philippe (?)
> >
>
> And NDEVS still does not depend on BITS_PER_LONG - yes, that's ok.
>
> Two minor missing points :
>
> - Doc update for rt_pipe_create() describing P_MINOR_AUTO
> - ChangeLog frag

Enclosed a final patch.

One thing I wanted to point out is that the exteneded interface is not
usable for the "rtai" skin since rtf_create() must know a real minor before
calling xnpipe_connect() (at least it's implemented this way at the
moment).

int rtf_create (unsigned minor, int size)

{

...
    fifo = __fifo_table + minor;    <---- That's the reason!

    err = xnpipe_connect(minor,
                         &__fifo_output_handler,
                         &__fifo_exec_handler,
                         NULL,
                         fifo);     <---- it's already dependent on minor.

And I don't think, of course, it's a good idea to export a separate
interface for allocation of minor values from the nucleus. So either:

- rtf_create() should be rewritten differently;

- keep it as is since it looks like the real rtai interface doesn't require
such a functionality.


> --
>
> Philippe.


---
Best regards,
Dmitry

(See attached file: pipe.auto-minor-3.patch)

[-- Attachment #1.2: Type: text/html, Size: 2474 bytes --]

[-- Attachment #2: pipe.auto-minor-3.patch --]
[-- Type: application/octet-stream, Size: 5395 bytes --]

diff -ur xenomai/ChangeLog xenomai-pipe2/ChangeLog
--- xenomai/ChangeLog	2005-11-14 16:01:06.000000000 +0100
+++ xenomai-pipe2/ChangeLog	2005-11-16 12:11:05.000000000 +0100
@@ -1,3 +1,14 @@
+
+2005-11-16  Dmitry Adamushko <dmitry.adamushko@gmail.com>
+
+	* nucleus/pipe.c, skins/native/pipe.c: Support for auto-allocation
+	of minor values for the pipe objects. Passing P_MINOR_AUTO as a third 
+	argument to the rt_pipe_create() causes the minor value to be
+	auto-allocated. In such a case, standard linux processes must
+	make use of the valid pipe name exported through the registry
+	interface in order to get connected to the pipe
+	(i.e. /proc/xenomai/registry/pipes/*).
+
 2005-11-14  Philippe Gerum  <rpm@xenomai.org>
 
 	* skins/native/pipe.h: Declare rt_pipe_flush() in kernel section.
diff -ur xenomai/include/nucleus/pipe.h xenomai-pipe2/include/nucleus/pipe.h
--- xenomai/include/nucleus/pipe.h	2005-10-07 23:11:23.000000000 +0200
+++ xenomai-pipe2/include/nucleus/pipe.h	2005-11-15 10:19:19.000000000 +0100
@@ -30,6 +30,8 @@
 #define XNPIPE_NORMAL 0x0
 #define XNPIPE_URGENT 0x1
 
+#define XNPIPE_MINOR_AUTO	-1
+
 #ifdef __KERNEL__
 
 #include <nucleus/queue.h>
diff -ur xenomai/nucleus/pipe.c xenomai-pipe2/nucleus/pipe.c
--- xenomai/nucleus/pipe.c	2005-11-09 19:20:47.000000000 +0100
+++ xenomai-pipe2/nucleus/pipe.c	2005-11-15 18:44:51.000000000 +0100
@@ -43,6 +43,9 @@
 
 xnpipe_state_t xnpipe_states[XNPIPE_NDEVS];
 
+#define XNPIPE_BITMAP_SIZE	((XNPIPE_NDEVS + BITS_PER_LONG - 1) / BITS_PER_LONG)
+static unsigned long xnpipe_bitmap[XNPIPE_BITMAP_SIZE];
+
 xnqueue_t xnpipe_sleepq, xnpipe_asyncq;
 
 int xnpipe_wakeup_apc;
@@ -57,6 +60,37 @@
    #define class_destroy class_simple_destroy
 #endif
 
+/* Allocation of minor values */
+
+static inline int xnpipe_minor_alloc(int minor)
+{
+    spl_t s;
+
+    if ((minor < 0 && minor != XNPIPE_MINOR_AUTO) || minor >= XNPIPE_NDEVS)
+	return -ENODEV;
+
+    xnlock_get_irqsave(&nklock,s);
+
+    if (minor == XNPIPE_MINOR_AUTO)
+	minor = find_first_zero_bit(xnpipe_bitmap,XNPIPE_NDEVS);
+
+    if (minor == XNPIPE_NDEVS || testbits(xnpipe_bitmap[minor / BITS_PER_LONG], 1 << (minor % BITS_PER_LONG)))
+        minor = -EBUSY;
+    else    
+	__setbits(xnpipe_bitmap[minor / BITS_PER_LONG], 1 << (minor % BITS_PER_LONG));
+
+    xnlock_put_irqrestore(&nklock,s);
+    return minor;
+}
+
+static inline void xnpipe_minor_free(int minor)
+{
+    if (minor < 0 || minor >= XNPIPE_NDEVS)
+	return;
+
+    clrbits(xnpipe_bitmap[minor / BITS_PER_LONG], 1 << (minor % BITS_PER_LONG));
+}
+
 /* Get nklock locked before using this macro */
 
 #define xnpipe_read_wait(state,flags)						\
@@ -215,8 +249,9 @@
     int need_sched = 0;
     spl_t s;
 
-    if (minor < 0 || minor >= XNPIPE_NDEVS)
-	return -ENODEV;
+    minor = xnpipe_minor_alloc(minor);
+    if (minor < 0)
+	return minor;
 
     state = &xnpipe_states[minor];
 
@@ -224,12 +259,6 @@
        to prevent using a partially created object */
     xnlock_get_irqsave(&nklock,s);
 
-    if (testbits(state->status,XNPIPE_KERN_CONN))
-	{
-	xnlock_put_irqrestore(&nklock,s);
-	return -EBUSY;
-	}
-    
     setbits(state->status,XNPIPE_KERN_CONN);
     
     xnsynch_init(&state->synchbase,XNSYNCH_FIFO);
@@ -261,7 +290,7 @@
     if (need_sched)
         xnpipe_schedule_request();
 
-    return 0;
+    return minor;
 }
 
 int xnpipe_disconnect (int minor)
@@ -323,6 +352,8 @@
 	    }
 	}
 
+    xnpipe_minor_free(minor);
+
     xnlock_put_irqrestore(&nklock,s);
 
     if (need_sched)
diff -ur xenomai/skins/native/pipe.c xenomai-pipe2/skins/native/pipe.c
--- xenomai/skins/native/pipe.c	2005-11-14 17:11:22.000000000 +0100
+++ xenomai-pipe2/skins/native/pipe.c	2005-11-16 12:12:46.000000000 +0100
@@ -194,6 +194,12 @@
  * the current system configuration.
  *
  * @param minor The minor number of the device associated with the pipe.
+ * Passing P_MINOR_AUTO causes the minor number to be auto-allocated. In
+ * such a case, the @a name parameter must be valid and the only way
+ * standard linux processes may connect to the pipe is by following
+ * a symbolic link from /proc/xenomai/registry/pipes/@a name since
+ * the minor number is unknown and directly accessing the associated
+ * special device (i.e. /dev/rtp*) is not possible.
  *
  * @return 0 is returned upon success. Otherwise:
  *
@@ -227,12 +233,11 @@
 		    const char *name,
 		    int minor)
 {
-    int err;
+    int err = 0;
 
     if (xnpod_asynch_p())
 	return -EPERM;
 
-    pipe->minor = minor;
     pipe->buffer = NULL;
     pipe->fillsz = 0;
     pipe->flushable = 0;
@@ -240,13 +245,16 @@
     pipe->magic = XENO_PIPE_MAGIC;
     xnobject_copy_name(pipe->name,name);
 
-    err = xnpipe_connect(minor,
+    minor = xnpipe_connect(minor,
 			 &__pipe_output_handler,
 			 NULL,
 			 &__pipe_alloc_handler,
 			 pipe);
-    if (err)
-	return err;
+
+    if (minor < 0)
+	return minor;
+
+    pipe->minor = minor;
 
 #ifdef CONFIG_XENO_OPT_PERVASIVE
     pipe->cpid = 0;
diff -ur xenomai/skins/native/pipe.h xenomai-pipe2/skins/native/pipe.h
--- xenomai/skins/native/pipe.h	2005-11-14 16:01:06.000000000 +0100
+++ xenomai-pipe2/skins/native/pipe.h	2005-11-15 11:34:00.000000000 +0100
@@ -29,6 +29,8 @@
 #define P_NORMAL  XNPIPE_NORMAL
 #define P_URGENT  XNPIPE_URGENT
 
+#define P_MINOR_AUTO	XNPIPE_MINOR_AUTO
+
 typedef struct rt_pipe_placeholder {
     rt_handle_t opaque;
 } RT_PIPE_PLACEHOLDER;

[Xenomai-core] Re: [PATCH] Auto-allocation of minor values for pipe objects

[Philippe Gerum]

Dmitry Adamushko wrote:
> Philippe Gerum <rpm@xenomai.org> wrote on 15.11.2005 23:17:50:
> 
>  > Dmitry Adamushko wrote:
>  > >
>  > > Hello,
>  > >
>  > > enclosed please find a patch that hopefully adds so desired
>  > > functionality. I have made various tests with it just now and it seems
>  > > to work fine.
>  > >
>  >
>  > Sounds good.
>  >
>  > > A size of the bitmap is dependent on XNPIPE_NDEVS parameter in the 
> same
>  > > vein as xnpipe_states depends on it; so hopefully that is what you 
> have
>  > > meant Philippe (?)
>  > >
>  >
>  > And NDEVS still does not depend on BITS_PER_LONG - yes, that's ok.
>  >
>  > Two minor missing points :
>  >
>  > - Doc update for rt_pipe_create() describing P_MINOR_AUTO
>  > - ChangeLog frag
> 
> Enclosed a final patch.
>

Applied, thanks.

> One thing I wanted to point out is that the exteneded interface is not 
> usable for the "rtai" skin since rtf_create() must know a real minor 
> before calling xnpipe_connect() (at least it's implemented this way at 
> the moment).
> 
> int rtf_create (unsigned minor, int size)
> 
> {
> 
> ...
>    fifo = __fifo_table + minor; <---- That's the reason!
> 
>    err = xnpipe_connect(minor,
>                         &__fifo_output_handler,
>                         &__fifo_exec_handler,
>                         NULL,
>                         fifo); <---- it's already dependent on minor.
> 
> And I don't think, of course, it's a good idea to export a separate 
> interface for allocation of minor values from the nucleus. So either:
> 
> - rtf_create() should be rewritten differently;
> 
> - keep it as is since it looks like the real rtai interface doesn't 
> require such a functionality.
> 
> 

We need to keep the things as they are for the RTAI skin. The art of emulating an 
interface includes mimicking its shortcomings.

-- 

Philippe.