Skip to contentSkip to conversation

NBD: raise max number of nbd devices to 1024

PATCH In review Paul Clements 2 participants 5 messages Jan 29, 2008 – Jan 30, 2008

Subsystems (2)
Block layer Network block device (NBD)
lore
Share & tools
lore

Paul Clements

author
#0 lore raw

PATCH 1/1 NBD: raise max number of nbd devices to 1024

The limit was 128. This changes it to 1024.

--
Paul

Signed-off-by Paul Clements <[email protected]>
include/linux/nbd.h renamed from include/linux/nbd.h.TIMEOUT +1 -1
@@ -35,7 +35,7 @@ enum {
 };
 
 #define nbd_cmd(req) ((req)->cmd[0])
-#define MAX_NBD 128
+#define MAX_NBD 1024
 
 /* userspace doesn't need the nbd_device structure */
 #ifdef __KERNEL__
#1 lore raw
Quoted 2 lines · On Tue, 29 Jan 2008 16:30:58 -0500 Paul Clements <[email protected]>…

The limit was 128. This changes it to 1024.

Please include the "why" in changelogs as well as the "what". It's fairly easy to guess why this needs to be increased, but it would be useful and interesting to hear about the operational scenarios in which 128 was insufficient.

include/linux/nbd.h enum { · line 41

Quoted 13 lines
include/linux/nbd.h renamed from include/linux/nbd.h.TIMEOUT +1 -1
@@ -35,7 +35,7 @@ enum {
 };
 
 #define nbd_cmd(req) ((req)->cmd[0])
-#define MAX_NBD 128
+#define MAX_NBD 1024
 
 /* userspace doesn't need the nbd_device structure */
 #ifdef __KERNEL__

But one still needs to specify the nbds_max module parameter.

Can't we just nuke MAX_NBD altogether?

drivers/block/nbd.c renamed from drivers/block/nbd.c~a +5 -7
@@ -54,7 +54,7 @@ static unsigned int debugflags;
 #endif /* NDEBUG */
 
 static unsigned int nbds_max = 16;
-static struct nbd_device nbd_dev[MAX_NBD];
+static struct nbd_device *nbd_dev;
 
 /*
  * Use just one lock (or at most 1 per NIC). Two arguments for this:

@@ -651,12 +651,6 @@ static int __init nbd_init(void)
 
 	BUILD_BUG_ON(sizeof(struct nbd_request) != 28);
 
-	if (nbds_max > MAX_NBD) {
-		printk(KERN_CRIT "nbd: cannot allocate more than %u nbds; %u requested.\n", MAX_NBD,
-				nbds_max);
-		return -EINVAL;
-	}
-
 	for (i = 0; i < nbds_max; i++) {
 		struct gendisk *disk = alloc_disk(1);
 		if (!disk)

@@ -715,6 +709,10 @@ out:
 static void __exit nbd_cleanup(void)
 {
 	int i;
+
+	nbd_dev = kcalloc(nbds_max, sizeof(*nbd_dev), GFP_KERNEL);
+	if (!nbd_dev)
+		return -ENOMEM;
 	for (i = 0; i < nbds_max; i++) {
 		struct gendisk *disk = nbd_dev[i].disk;
 		nbd_dev[i].magic = 0;
include/linux/nbd.h renamed from include/linux/nbd.h~a +0 -1
diff -puN include/linux/nbd.h~a include/linux/nbd.h
@@ -35,7 +35,6 @@ enum {
 };
 
 #define nbd_cmd(req) ((req)->cmd[0])
-#define MAX_NBD 128
 
 /* userspace doesn't need the nbd_device structure */
 #ifdef __KERNEL__

_

much nicer?

author
#2 lore raw

Andrew Morton wrote:

<snip>

Quoted 1 line

much nicer?

OK, yes, I was going for quick and easy, but you've got a point...

We do need to move the kcalloc into nbd_init instead of nbd_cleanup, though -- that works a little better. Patch is attached. Thanks for the suggestion.

--
Paul

Signed-off-by Paul Clements <[email protected]>
include/linux/nbd.h renamed from include/linux/nbd.h.TIMEOUT +0 -1
@@ -35,7 +35,6 @@ enum {
 };
 
 #define nbd_cmd(req) ((req)->cmd[0])
-#define MAX_NBD 128
 
 /* userspace doesn't need the nbd_device structure */
 #ifdef __KERNEL__
drivers/block/nbd.c renamed from drivers/block/nbd.c.ORIG +4 -6
@@ -53,7 +53,7 @@ static unsigned int debugflags;
 #endif /* NDEBUG */
 
 static unsigned int nbds_max = 16;
-static struct nbd_device nbd_dev[MAX_NBD];
+static struct nbd_device *nbd_dev;
 
 /*
  * Use just one lock (or at most 1 per NIC). Two arguments for this:

@@ -659,11 +659,9 @@ static int __init nbd_init(void)
 
 	BUILD_BUG_ON(sizeof(struct nbd_request) != 28);
 
-	if (nbds_max > MAX_NBD) {
-		printk(KERN_CRIT "nbd: cannot allocate more than %u nbds; %u requested.\n", MAX_NBD,
-				nbds_max);
-		return -EINVAL;
-	}
+	nbd_dev = kcalloc(nbds_max, sizeof(*nbd_dev), GFP_KERNEL);
+	if (!nbd_dev)
+		return -ENOMEM;
 
 	for (i = 0; i < nbds_max; i++) {
 		struct gendisk *disk = alloc_disk(1);
#3 lore raw

include/linux/nbd.h

Quoted 22 lines · On Tue, 29 Jan 2008 21:04:05 -0500 Paul Clements <[email protected]>…

Andrew Morton wrote:

<snip>

Quoted 1 line

much nicer?

OK, yes, I was going for quick and easy, but you've got a point...

We do need to move the kcalloc into nbd_init instead of nbd_cleanup, though -- that works a little better. Patch is attached. Thanks for the suggestion.

--
Paul

[nbd_max_nbd_killed.diff text/x-patch (1.2KB)]

Signed-off-by Paul Clements <[email protected]>
include/linux/nbd.h renamed from include/linux/nbd.h.TIMEOUT +0 -0

Could we have a complete changelog please? As I mentioned in the earlier email?

author
#4 lore raw
Quoted 17 lines · Andrew Morton
Quoted 15 lines · On Tue, 29 Jan 2008 21:04:05 -0500 Paul Clements <[email protected]>…

Andrew Morton wrote:

<snip>

Quoted 1 line

much nicer?

OK, yes, I was going for quick and easy, but you've got a point...

We do need to move the kcalloc into nbd_init instead of nbd_cleanup, though -- that works a little better. Patch is attached. Thanks for the suggestion.

--
Paul

[nbd_max_nbd_killed.diff text/x-patch (1.2KB)]

Quoted 8 lines

include/linux/nbd.h

Quoted 4 lines
Signed-off-by Paul Clements <[email protected]>
include/linux/nbd.h renamed from include/linux/nbd.h.TIMEOUT +0 -0

Could we have a complete changelog please? As I mentioned in the earlier email?

[PATCH 1/1] NBD: remove limit on max number of nbd devices

Remove the arbitrary 128 device limit for NBD. nbds_max can now be set to any number. In certain scenarios where devices are used sparsely we have run into the 128 device limit.

Signed-off-by Paul Clements <[email protected]>
include/linux/nbd.h renamed from include/linux/nbd.h.TIMEOUT +0 -1
@@ -35,7 +35,6 @@ enum {
 };
 
 #define nbd_cmd(req) ((req)->cmd[0])
-#define MAX_NBD 128
 
 /* userspace doesn't need the nbd_device structure */
 #ifdef __KERNEL__
drivers/block/nbd.c renamed from drivers/block/nbd.c.ORIG +4 -6
@@ -53,7 +53,7 @@ static unsigned int debugflags;
 #endif /* NDEBUG */
 
 static unsigned int nbds_max = 16;
-static struct nbd_device nbd_dev[MAX_NBD];
+static struct nbd_device *nbd_dev;
 
 /*
  * Use just one lock (or at most 1 per NIC). Two arguments for this:

@@ -659,11 +659,9 @@ static int __init nbd_init(void)
 
 	BUILD_BUG_ON(sizeof(struct nbd_request) != 28);
 
-	if (nbds_max > MAX_NBD) {
-		printk(KERN_CRIT "nbd: cannot allocate more than %u nbds; %u requested.\n", MAX_NBD,
-				nbds_max);
-		return -EINVAL;
-	}
+	nbd_dev = kcalloc(nbds_max, sizeof(*nbd_dev), GFP_KERNEL);
+	if (!nbd_dev)
+		return -ENOMEM;
 
 	for (i = 0; i < nbds_max; i++) {
 		struct gendisk *disk = alloc_disk(1);