Mailing List Archive

[RFC 2/4] firewire: dynamic cdev allocation below firewire major
This patch implements dynamic minor number allocation below the 171
major allocated for ieee1394. Since on today's systems one doesn't need
to have fixed device numbers any more we could just use any, but it's
probably still useful to use the ieee1394 major number for any firewire
related devices (like mem1394).

diff --git a/drivers/ieee1394/ieee1394_core.c b/drivers/ieee1394/ieee1394_core.c
index 25ef5a8..17afc3b 100644
--- a/drivers/ieee1394/ieee1394_core.c
+++ b/drivers/ieee1394/ieee1394_core.c
@@ -29,10 +29,12 @@
#include <linux/interrupt.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
+#include <linux/bitmap.h>
#include <linux/bitops.h>
#include <linux/kdev_t.h>
#include <linux/skbuff.h>
#include <linux/suspend.h>
+#include <linux/cdev.h>

#include <asm/byteorder.h>
#include <asm/semaphore.h>
@@ -1053,9 +1055,14 @@ static int hpsbpkt_thread(void *__hi)
complete_and_exit(&khpsbpkt_complete, 0);
}

+/* used further below, but needs to be here for initialisation */
+static spinlock_t used_minors_lock;
+
static int __init ieee1394_init(void)
{
int i, ret;
+
+ spin_lock_init(&used_minors_lock);

skb_queue_head_init(&hpsbpkt_queue);

@@ -1197,6 +1204,47 @@ static void __exit ieee1394_cleanup(void
module_init(ieee1394_init);
module_exit(ieee1394_cleanup);

+/* dynamic minor allocation functions */
+static DECLARE_BITMAP(used_minors, IEEE1394_MINOR_DYNAMIC_COUNT);
+
+int hpsb_cdev_add(struct cdev *chardev)
+{
+ int minor, ret;
+
+ spin_lock(&used_minors_lock);
+ minor = find_first_zero_bit(used_minors, IEEE1394_MINOR_DYNAMIC_COUNT);
+ if (minor >= IEEE1394_MINOR_DYNAMIC_COUNT) {
+ spin_unlock(&used_minors_lock);
+ return -ENODEV;
+ }
+ set_bit(minor, used_minors);
+ spin_unlock(&used_minors_lock);
+
+ minor += IEEE1394_MINOR_DYNAMIC_FIRST;
+ ret = cdev_add(chardev, MKDEV(IEEE1394_MAJOR, minor), 1);
+ if (unlikely(ret)) {
+ spin_lock(&used_minors_lock);
+ clear_bit(minor-IEEE1394_MINOR_DYNAMIC_FIRST, used_minors);
+ spin_unlock(&used_minors_lock);
+ }
+ return ret;
+}
+EXPORT_SYMBOL_GPL(hpsb_cdev_add);
+
+void hpsb_cdev_del(struct cdev *chardev)
+{
+ dev_t dev;
+
+ BUG_ON(MAJOR(chardev->dev) != IEEE1394_MAJOR);
+ dev = chardev->dev;
+ cdev_del(chardev);
+
+ spin_lock(&used_minors_lock);
+ clear_bit(MINOR(dev) - IEEE1394_MINOR_DYNAMIC_FIRST, used_minors);
+ spin_unlock(&used_minors_lock);
+}
+EXPORT_SYMBOL_GPL(hpsb_cdev_del);
+
/* Exported symbols */

/** hosts.c **/
diff --git a/drivers/ieee1394/ieee1394_core.h b/drivers/ieee1394/ieee1394_core.h
index b354660..d248ed7 100644
--- a/drivers/ieee1394/ieee1394_core.h
+++ b/drivers/ieee1394/ieee1394_core.h
@@ -186,19 +186,38 @@ void hpsb_packet_received(struct hpsb_ho
* 171:0-255, the various drivers must then cdev_add() their cdev
* objects to handle their respective sub-regions.
*
+ * Alternatively, drivers may use a dynamic minor number character
+ * device by using the functions hpsb_cdev_add and hpsb_cdev_del.
+ * hpsb_cdev_add requires an initialised struct cdev and will add
+ * it with cdev_add() automatically, reserving a new minor number
+ * for the new device (unless cdev_add() fails). It returns the
+ * status of cdev_add(), or -ENODEV if no minor could be allocated.
+ *
+ * Currently 64 minor numbers are reserved for that, if necessary
+ * this number can be increased by simply adjusting the constant
+ * IEEE1394_MINOR_DYNAMIC_FIRST.
+ *
* Minor device number block allocations:
*
* Block 0 ( 0- 15) raw1394
* Block 1 ( 16- 31) video1394
* Block 2 ( 32- 47) dv1394
*
- * Blocks 3-14 free for future allocation
+ * Blocks 3-10 free for future allocation
*
+ * Block 11 (176-191) dynamic allocation region
+ * Block 12 (192-207) dynamic allocation region
+ * Block 13 (208-223) dynamic allocation region
+ * Block 14 (224-239) dynamic allocation region
* Block 15 (240-255) reserved for drivers under development, etc.
*/

#define IEEE1394_MAJOR 171

+#define IEEE1394_MINOR_DYNAMIC_FIRST 176
+#define IEEE1394_MINOR_DYNAMIC_LAST 239
+#define IEEE1394_MINOR_DYNAMIC_COUNT (IEEE1394_MINOR_DYNAMIC_LAST-IEEE1394_MINOR_DYNAMIC_FIRST+1)
+
#define IEEE1394_MINOR_BLOCK_RAW1394 0
#define IEEE1394_MINOR_BLOCK_VIDEO1394 1
#define IEEE1394_MINOR_BLOCK_DV1394 2
@@ -218,6 +237,11 @@ static inline unsigned char ieee1394_fil
return file->f_dentry->d_inode->i_cindex;
}

+/* add a dynamic ieee1394 device */
+int hpsb_cdev_add(struct cdev *chardev);
+/* remove a dynamic ieee1394 device */
+void hpsb_cdev_del(struct cdev *chardev);
+
extern int hpsb_disable_irm;

/* Our sysfs bus entry */


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 2/4] firewire: dynamic cdev allocation below firewire major [ In reply to ]
Johannes Berg wrote:
> This patch implements dynamic minor number allocation below the 171
> major allocated for ieee1394. Since on today's systems one doesn't need
> to have fixed device numbers any more we could just use any, but it's
> probably still useful to use the ieee1394 major number for any firewire
> related devices (like mem1394).
[...]
> --- a/drivers/ieee1394/ieee1394_core.h
> +++ b/drivers/ieee1394/ieee1394_core.h
> @@ -186,19 +186,38 @@ void hpsb_packet_received(struct hpsb_ho
> * 171:0-255, the various drivers must then cdev_add() their cdev
> * objects to handle their respective sub-regions.
> *
> + * Alternatively, drivers may use a dynamic minor number character
> + * device by using the functions hpsb_cdev_add and hpsb_cdev_del.
> + * hpsb_cdev_add requires an initialised struct cdev and will add
> + * it with cdev_add() automatically, reserving a new minor number
> + * for the new device (unless cdev_add() fails). It returns the
> + * status of cdev_add(), or -ENODEV if no minor could be allocated.

This comment should be moved to the implementations of respective
functions and be formatted as described by
Documentation/kernel-doc-nano-HOWTO.txt. (We should eventually check all
exported ieee1394 symbols if they are documented that way.)

> + * Currently 64 minor numbers are reserved for that, if necessary
> + * this number can be increased by simply adjusting the constant
> + * IEEE1394_MINOR_DYNAMIC_FIRST.
> + *
> * Minor device number block allocations:
> *
> * Block 0 ( 0- 15) raw1394
> * Block 1 ( 16- 31) video1394
> * Block 2 ( 32- 47) dv1394
> *
> - * Blocks 3-14 free for future allocation
> + * Blocks 3-10 free for future allocation
> *
> + * Block 11 (176-191) dynamic allocation region
> + * Block 12 (192-207) dynamic allocation region
> + * Block 13 (208-223) dynamic allocation region
> + * Block 14 (224-239) dynamic allocation region
> * Block 15 (240-255) reserved for drivers under development, etc.
> */
>
> #define IEEE1394_MAJOR 171
>
> +#define IEEE1394_MINOR_DYNAMIC_FIRST 176
> +#define IEEE1394_MINOR_DYNAMIC_LAST 239
> +#define IEEE1394_MINOR_DYNAMIC_COUNT (IEEE1394_MINOR_DYNAMIC_LAST-IEEE1394_MINOR_DYNAMIC_FIRST+1)
> +
> #define IEEE1394_MINOR_BLOCK_RAW1394 0
> #define IEEE1394_MINOR_BLOCK_VIDEO1394 1
> #define IEEE1394_MINOR_BLOCK_DV1394 2
> @@ -218,6 +237,11 @@ static inline unsigned char ieee1394_fil
> return file->f_dentry->d_inode->i_cindex;
> }
>
> +/* add a dynamic ieee1394 device */
> +int hpsb_cdev_add(struct cdev *chardev);
> +/* remove a dynamic ieee1394 device */
> +void hpsb_cdev_del(struct cdev *chardev);
[...]

Again, better no comment here than these two which are not very
enlightening.
--
Stefan Richter
-=====-=-==- --=- --=-=
http://arcgraph.de/sr/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 2/4] firewire: dynamic cdev allocation below firewire major [ In reply to ]
On Thu, Feb 02, 2006 at 11:40:12PM +0100, Johannes Berg wrote:
> This patch implements dynamic minor number allocation below the 171
> major allocated for ieee1394. Since on today's systems one doesn't need
> to have fixed device numbers any more we could just use any, but it's
> probably still useful to use the ieee1394 major number for any firewire
> related devices (like mem1394).

I don't really like this. There's no benefit to using the 1394 major
number. I'd rather see an improved alloc_chrdev_region() that does
something like this but for the whole kernel (currently it "wastes" an
entire major even if you only want 1 minor, and for what you're doing,
grabbing 1 minor at a time makes the most sense.)

I'll get to that someday unless someone beats me to it (tm) because it
will be necessary for the raw1394 security improvements I'm (slowly)
working on. Unless someone convinces me that dynamic allocation under
171 is really the way to go.

Cheers,
Jody

>
> diff --git a/drivers/ieee1394/ieee1394_core.c b/drivers/ieee1394/ieee1394_core.c
> index 25ef5a8..17afc3b 100644
> --- a/drivers/ieee1394/ieee1394_core.c
> +++ b/drivers/ieee1394/ieee1394_core.c
> @@ -29,10 +29,12 @@
> #include <linux/interrupt.h>
> #include <linux/module.h>
> #include <linux/moduleparam.h>
> +#include <linux/bitmap.h>
> #include <linux/bitops.h>
> #include <linux/kdev_t.h>
> #include <linux/skbuff.h>
> #include <linux/suspend.h>
> +#include <linux/cdev.h>
>
> #include <asm/byteorder.h>
> #include <asm/semaphore.h>
> @@ -1053,9 +1055,14 @@ static int hpsbpkt_thread(void *__hi)
> complete_and_exit(&khpsbpkt_complete, 0);
> }
>
> +/* used further below, but needs to be here for initialisation */
> +static spinlock_t used_minors_lock;
> +
> static int __init ieee1394_init(void)
> {
> int i, ret;
> +
> + spin_lock_init(&used_minors_lock);
>
> skb_queue_head_init(&hpsbpkt_queue);
>
> @@ -1197,6 +1204,47 @@ static void __exit ieee1394_cleanup(void
> module_init(ieee1394_init);
> module_exit(ieee1394_cleanup);
>
> +/* dynamic minor allocation functions */
> +static DECLARE_BITMAP(used_minors, IEEE1394_MINOR_DYNAMIC_COUNT);
> +
> +int hpsb_cdev_add(struct cdev *chardev)
> +{
> + int minor, ret;
> +
> + spin_lock(&used_minors_lock);
> + minor = find_first_zero_bit(used_minors, IEEE1394_MINOR_DYNAMIC_COUNT);
> + if (minor >= IEEE1394_MINOR_DYNAMIC_COUNT) {
> + spin_unlock(&used_minors_lock);
> + return -ENODEV;
> + }
> + set_bit(minor, used_minors);
> + spin_unlock(&used_minors_lock);
> +
> + minor += IEEE1394_MINOR_DYNAMIC_FIRST;
> + ret = cdev_add(chardev, MKDEV(IEEE1394_MAJOR, minor), 1);
> + if (unlikely(ret)) {
> + spin_lock(&used_minors_lock);
> + clear_bit(minor-IEEE1394_MINOR_DYNAMIC_FIRST, used_minors);
> + spin_unlock(&used_minors_lock);
> + }
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(hpsb_cdev_add);
> +
> +void hpsb_cdev_del(struct cdev *chardev)
> +{
> + dev_t dev;
> +
> + BUG_ON(MAJOR(chardev->dev) != IEEE1394_MAJOR);
> + dev = chardev->dev;
> + cdev_del(chardev);
> +
> + spin_lock(&used_minors_lock);
> + clear_bit(MINOR(dev) - IEEE1394_MINOR_DYNAMIC_FIRST, used_minors);
> + spin_unlock(&used_minors_lock);
> +}
> +EXPORT_SYMBOL_GPL(hpsb_cdev_del);
> +
> /* Exported symbols */
>
> /** hosts.c **/
> diff --git a/drivers/ieee1394/ieee1394_core.h b/drivers/ieee1394/ieee1394_core.h
> index b354660..d248ed7 100644
> --- a/drivers/ieee1394/ieee1394_core.h
> +++ b/drivers/ieee1394/ieee1394_core.h
> @@ -186,19 +186,38 @@ void hpsb_packet_received(struct hpsb_ho
> * 171:0-255, the various drivers must then cdev_add() their cdev
> * objects to handle their respective sub-regions.
> *
> + * Alternatively, drivers may use a dynamic minor number character
> + * device by using the functions hpsb_cdev_add and hpsb_cdev_del.
> + * hpsb_cdev_add requires an initialised struct cdev and will add
> + * it with cdev_add() automatically, reserving a new minor number
> + * for the new device (unless cdev_add() fails). It returns the
> + * status of cdev_add(), or -ENODEV if no minor could be allocated.
> + *
> + * Currently 64 minor numbers are reserved for that, if necessary
> + * this number can be increased by simply adjusting the constant
> + * IEEE1394_MINOR_DYNAMIC_FIRST.
> + *
> * Minor device number block allocations:
> *
> * Block 0 ( 0- 15) raw1394
> * Block 1 ( 16- 31) video1394
> * Block 2 ( 32- 47) dv1394
> *
> - * Blocks 3-14 free for future allocation
> + * Blocks 3-10 free for future allocation
> *
> + * Block 11 (176-191) dynamic allocation region
> + * Block 12 (192-207) dynamic allocation region
> + * Block 13 (208-223) dynamic allocation region
> + * Block 14 (224-239) dynamic allocation region
> * Block 15 (240-255) reserved for drivers under development, etc.
> */
>
> #define IEEE1394_MAJOR 171
>
> +#define IEEE1394_MINOR_DYNAMIC_FIRST 176
> +#define IEEE1394_MINOR_DYNAMIC_LAST 239
> +#define IEEE1394_MINOR_DYNAMIC_COUNT (IEEE1394_MINOR_DYNAMIC_LAST-IEEE1394_MINOR_DYNAMIC_FIRST+1)
> +
> #define IEEE1394_MINOR_BLOCK_RAW1394 0
> #define IEEE1394_MINOR_BLOCK_VIDEO1394 1
> #define IEEE1394_MINOR_BLOCK_DV1394 2
> @@ -218,6 +237,11 @@ static inline unsigned char ieee1394_fil
> return file->f_dentry->d_inode->i_cindex;
> }
>
> +/* add a dynamic ieee1394 device */
> +int hpsb_cdev_add(struct cdev *chardev);
> +/* remove a dynamic ieee1394 device */
> +void hpsb_cdev_del(struct cdev *chardev);
> +
> extern int hpsb_disable_irm;
>
> /* Our sysfs bus entry */
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

--
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 2/4] firewire: dynamic cdev allocation below firewire major [ In reply to ]
On Sun, 2006-02-12 at 22:51 -0500, Jody McIntyre wrote:
> On Thu, Feb 02, 2006 at 11:40:12PM +0100, Johannes Berg wrote:
> > This patch implements dynamic minor number allocation below the 171
> > major allocated for ieee1394. Since on today's systems one doesn't need
> > to have fixed device numbers any more we could just use any, but it's
> > probably still useful to use the ieee1394 major number for any firewire
> > related devices (like mem1394).
>
> I don't really like this. There's no benefit to using the 1394 major
> number. I'd rather see an improved alloc_chrdev_region() that does
> something like this but for the whole kernel (currently it "wastes" an
> entire major even if you only want 1 minor, and for what you're doing,
> grabbing 1 minor at a time makes the most sense.)


why bother? There's a LOT of majors nowadays (12 bits) so... what's the
problem with keeping the kernel side simple?
(it's not as if userspace needs to care about the exact numbers anyway
for almost everything)

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 2/4] firewire: dynamic cdev allocation below firewire major [ In reply to ]
On Mon, 2006-02-13 at 08:32 +0100, Arjan van de Ven wrote:
> > I don't really like this. There's no benefit to using the 1394 major
> > number. I'd rather see an improved alloc_chrdev_region() that does
> > something like this but for the whole kernel (currently it "wastes" an
> > entire major even if you only want 1 minor, and for what you're doing,
> > grabbing 1 minor at a time makes the most sense.)
>
> why bother? There's a LOT of majors nowadays (12 bits) so... what's the
> problem with keeping the kernel side simple?
> (it's not as if userspace needs to care about the exact numbers anyway
> for almost everything)

Uh, ok. Seems pretty weird to effectively allocate 256 device numbers
for just a single device, but ok :)
I'll drop the patch and make it allocate a new major for every device
plugged in.

johannes
Re: [RFC 2/4] firewire: dynamic cdev allocation below firewire major [ In reply to ]
Johannes Berg wrote:
> Seems pretty weird to effectively allocate 256 device numbers
> for just a single device, but ok :)
> I'll drop the patch and make it allocate a new major for every device
> plugged in.

Your driver could internally dispatch 256 minor device numbers after
it got itself its own dynamic major number. This should even be
acceptable as a hard limit of mem1394 devices. One would need quite a
lot of 1394 cards (or 1394.1 bridges) to get access to 256 FireWire
nodes at once.
--
Stefan Richter
-=====-=-==- --=- -==-=
http://arcgraph.de/sr/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 2/4] firewire: dynamic cdev allocation below firewire major [ In reply to ]
On Mon, 2006-02-13 at 13:02 +0100, Johannes Berg wrote:
> On Mon, 2006-02-13 at 08:32 +0100, Arjan van de Ven wrote:
> > > I don't really like this. There's no benefit to using the 1394 major
> > > number. I'd rather see an improved alloc_chrdev_region() that does
> > > something like this but for the whole kernel (currently it "wastes" an
> > > entire major even if you only want 1 minor, and for what you're doing,
> > > grabbing 1 minor at a time makes the most sense.)
> >
> > why bother? There's a LOT of majors nowadays (12 bits) so... what's the
> > problem with keeping the kernel side simple?
> > (it's not as if userspace needs to care about the exact numbers anyway
> > for almost everything)
>
> Uh, ok. Seems pretty weird to effectively allocate 256 device numbers
> for just a single device, but ok :)

it's not 256 it's 2^20.... but still :)
(eg there are 20 bits to a minor, 12 to a major)

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 2/4] firewire: dynamic cdev allocation below firewire major [ In reply to ]
On Mon, 2006-02-13 at 22:10 +0100, Arjan van de Ven wrote:

> it's not 256 it's 2^20.... but still :)
> (eg there are 20 bits to a minor, 12 to a major)

Umm, right. But I guess Stefan's right too, I should instead allocate a
single major and use it's minors.

johannes