[PATCH 0/4] Remove un-needed 'major' registration when alloc_disk(0) is used.

[PATCH 0/4] Remove un-needed 'major' registration when alloc_disk(0) is used.

[NeilBrown]

When alloc_disk(0) is used, the ->major number is ignored and
irrelevant.  Yet several drivers register a major number anyway.

This series of patches removes the pointless registrations.  The pmem
driver also does this, but a patch has already been sent for that
driver.

Note that I am not in a position to test these beyond simple compile
testing.

Thanks,
NeilBrown


---

NeilBrown (4):
      nvdimm/blk: don't allocate unused major device number
      nvdimm/btt: don't allocate unused major device number
      memstick: don't allocate unused major for ms_block
      NVMe: don't allocate unused nvme_major


 drivers/memstick/core/ms_block.c |   17 ++---------------
 drivers/nvdimm/blk.c             |   18 +-----------------
 drivers/nvdimm/btt.c             |   19 ++-----------------
 drivers/nvme/host/core.c         |   16 +---------------
 4 files changed, 6 insertions(+), 64 deletions(-)

--
Signature

[PATCH 2/4] nvdimm/btt: don't allocate unused major device number

[NeilBrown]

alloc_disk(0) does not require or use a ->major number,
all devices are allocated with a major of BLOCK_EXT_MAJOR.

So don't allocate btt_major.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/nvdimm/btt.c |   19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index efb2c1ceef98..c32cbb593600 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -31,8 +31,6 @@ enum log_ent_request {
 	LOG_OLD_ENT
 };
 
-static int btt_major;
-
 static int arena_read_bytes(struct arena_info *arena, resource_size_t offset,
 		void *buf, size_t n)
 {
@@ -1246,7 +1244,6 @@ static int btt_blk_init(struct btt *btt)
 
 	nvdimm_namespace_disk_name(ndns, btt->btt_disk->disk_name);
 	btt->btt_disk->driverfs_dev = &btt->nd_btt->dev;
-	btt->btt_disk->major = btt_major;
 	btt->btt_disk->first_minor = 0;
 	btt->btt_disk->fops = &btt_fops;
 	btt->btt_disk->private_data = btt;
@@ -1423,22 +1420,11 @@ EXPORT_SYMBOL(nvdimm_namespace_detach_btt);
 
 static int __init nd_btt_init(void)
 {
-	int rc;
-
-	btt_major = register_blkdev(0, "btt");
-	if (btt_major < 0)
-		return btt_major;
+	int rc = 0;
 
 	debugfs_root = debugfs_create_dir("btt", NULL);
-	if (IS_ERR_OR_NULL(debugfs_root)) {
+	if (IS_ERR_OR_NULL(debugfs_root))
 		rc = -ENXIO;
-		goto err_debugfs;
-	}
-
-	return 0;
-
- err_debugfs:
-	unregister_blkdev(btt_major, "btt");
 
 	return rc;
 }
@@ -1446,7 +1432,6 @@ static int __init nd_btt_init(void)
 static void __exit nd_btt_exit(void)
 {
 	debugfs_remove_recursive(debugfs_root);
-	unregister_blkdev(btt_major, "btt");
 }
 
 MODULE_ALIAS_ND_DEVICE(ND_DEVICE_BTT);

Re: [PATCH 2/4] nvdimm/btt: don't allocate unused major device number

[Johannes Thumshirn]

On Thu, Mar 10, 2016 at 08:59:28AM +1100, NeilBrown wrote:
> alloc_disk(0) does not require or use a ->major number,
> all devices are allocated with a major of BLOCK_EXT_MAJOR.
> 
> So don't allocate btt_major.
> 
> Signed-off-by: NeilBrown <neilb@suse.com>

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 1/4] nvdimm/blk: don't allocate unused major device number

[NeilBrown]

When alloc_disk(0) is used ->major is completely ignored, all devices
are allocated with a "major" of BLOCK_EXT_MAJOR.

So don't allocate nd_blk_major

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/nvdimm/blk.c |   18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/drivers/nvdimm/blk.c b/drivers/nvdimm/blk.c
index 91a336ea8c4f..e9ff9229d942 100644
--- a/drivers/nvdimm/blk.c
+++ b/drivers/nvdimm/blk.c
@@ -31,8 +31,6 @@ struct nd_blk_device {
 	u32 internal_lbasize;
 };
 
-static int nd_blk_major;
-
 static u32 nd_blk_meta_size(struct nd_blk_device *blk_dev)
 {
 	return blk_dev->nsblk->lbasize - blk_dev->sector_size;
@@ -264,7 +262,6 @@ static int nd_blk_attach_disk(struct nd_namespace_common *ndns,
 	}
 
 	disk->driverfs_dev	= &ndns->dev;
-	disk->major		= nd_blk_major;
 	disk->first_minor	= 0;
 	disk->fops		= &nd_blk_fops;
 	disk->private_data	= blk_dev;
@@ -358,25 +355,12 @@ static struct nd_device_driver nd_blk_driver = {
 
 static int __init nd_blk_init(void)
 {
-	int rc;
-
-	rc = register_blkdev(0, "nd_blk");
-	if (rc < 0)
-		return rc;
-
-	nd_blk_major = rc;
-	rc = nd_driver_register(&nd_blk_driver);
-
-	if (rc < 0)
-		unregister_blkdev(nd_blk_major, "nd_blk");
-
-	return rc;
+	return nd_driver_register(&nd_blk_driver);
 }
 
 static void __exit nd_blk_exit(void)
 {
 	driver_unregister(&nd_blk_driver.drv);
-	unregister_blkdev(nd_blk_major, "nd_blk");
 }
 
 MODULE_AUTHOR("Ross Zwisler <ross.zwisler@linux.intel.com>");

Re: [PATCH 1/4] nvdimm/blk: don't allocate unused major device number

[Johannes Thumshirn]

On Thu, Mar 10, 2016 at 08:59:28AM +1100, NeilBrown wrote:
> When alloc_disk(0) is used ->major is completely ignored, all devices
> are allocated with a "major" of BLOCK_EXT_MAJOR.
> 
> So don't allocate nd_blk_major
> 
> Signed-off-by: NeilBrown <neilb@suse.com>

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 3/4] memstick: don't allocate unused major for ms_block

[NeilBrown]

When alloc_disk(0) is used the ->major number is completely ignored.
All devices are allocated with a major of BLOCK_EXT_MAJOR.

So remove registration and deregistration of 'major'.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/memstick/core/ms_block.c |   17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/drivers/memstick/core/ms_block.c b/drivers/memstick/core/ms_block.c
index 84abf9d3c24e..3d086f1d1234 100644
--- a/drivers/memstick/core/ms_block.c
+++ b/drivers/memstick/core/ms_block.c
@@ -2340,23 +2340,11 @@ static struct memstick_driver msb_driver = {
 	.resume   = msb_resume
 };
 
-static int major;
-
 static int __init msb_init(void)
 {
-	int rc = register_blkdev(0, DRIVER_NAME);
-
-	if (rc < 0) {
-		pr_err("failed to register major (error %d)\n", rc);
-		return rc;
-	}
-
-	major = rc;
-	rc = memstick_register_driver(&msb_driver);
-	if (rc) {
-		unregister_blkdev(major, DRIVER_NAME);
+	int rc = memstick_register_driver(&msb_driver);
+	if (rc)
 		pr_err("failed to register memstick driver (error %d)\n", rc);
-	}
 
 	return rc;
 }
@@ -2364,7 +2352,6 @@ static int __init msb_init(void)
 static void __exit msb_exit(void)
 {
 	memstick_unregister_driver(&msb_driver);
-	unregister_blkdev(major, DRIVER_NAME);
 	idr_destroy(&msb_disk_idr);
 }
 

Re: [PATCH 3/4] memstick: don't allocate unused major for ms_block

[Johannes Thumshirn]

On Thu, Mar 10, 2016 at 08:59:28AM +1100, NeilBrown wrote:
> When alloc_disk(0) is used the ->major number is completely ignored.
> All devices are allocated with a major of BLOCK_EXT_MAJOR.
> 
> So remove registration and deregistration of 'major'.
> 
> Signed-off-by: NeilBrown <neilb@suse.com>

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 4/4] NVMe: don't allocate unused nvme_major

[NeilBrown]

When alloc_disk(0) is used, the ->major number is ignored.  All device
numbers are allocated with a major of BLOCK_EXT_MAJOR.

So remove all references to nvme_major.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/nvme/host/core.c |   16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 03c46412fff4..10e2435c84a9 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -33,9 +33,6 @@
 
 #define NVME_MINORS		(1U << MINORBITS)
 
-static int nvme_major;
-module_param(nvme_major, int, 0);
-
 static int nvme_char_major;
 module_param(nvme_char_major, int, 0);
 
@@ -1170,8 +1167,6 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid)
 	blk_queue_logical_block_size(ns->queue, 1 << ns->lba_shift);
 	nvme_set_queue_limits(ctrl, ns->queue);
 
-	disk->major = nvme_major;
-	disk->first_minor = 0;
 	disk->fops = &nvme_fops;
 	disk->private_data = ns;
 	disk->queue = ns->queue;
@@ -1480,16 +1475,10 @@ int __init nvme_core_init(void)
 {
 	int result;
 
-	result = register_blkdev(nvme_major, "nvme");
-	if (result < 0)
-		return result;
-	else if (result > 0)
-		nvme_major = result;
-
 	result = __register_chrdev(nvme_char_major, 0, NVME_MINORS, "nvme",
 							&nvme_dev_fops);
 	if (result < 0)
-		goto unregister_blkdev;
+		return result;
 	else if (result > 0)
 		nvme_char_major = result;
 
@@ -1503,14 +1492,11 @@ int __init nvme_core_init(void)
 
  unregister_chrdev:
 	__unregister_chrdev(nvme_char_major, 0, NVME_MINORS, "nvme");
- unregister_blkdev:
-	unregister_blkdev(nvme_major, "nvme");
 	return result;
 }
 
 void nvme_core_exit(void)
 {
-	unregister_blkdev(nvme_major, "nvme");
 	class_destroy(nvme_class);
 	__unregister_chrdev(nvme_char_major, 0, NVME_MINORS, "nvme");
 }

Re: [PATCH 4/4] NVMe: don't allocate unused nvme_major

[Johannes Thumshirn]

On Thu, Mar 10, 2016 at 08:59:28AM +1100, NeilBrown wrote:
> When alloc_disk(0) is used, the ->major number is ignored.  All device
> numbers are allocated with a major of BLOCK_EXT_MAJOR.
> 
> So remove all references to nvme_major.
> 
> Signed-off-by: NeilBrown <neilb@suse.com>

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 0/4] Remove un-needed 'major' registration when alloc_disk(0) is used.

[Dan Williams]

On Wed, Mar 9, 2016 at 1:59 PM, NeilBrown <neilb@suse.com> wrote:
> When alloc_disk(0) is used, the ->major number is ignored and
> irrelevant.  Yet several drivers register a major number anyway.
>
> This series of patches removes the pointless registrations.  The pmem
> driver also does this, but a patch has already been sent for that
> driver.
>
> Note that I am not in a position to test these beyond simple compile
> testing.
>
> Thanks,
> NeilBrown
>
>
> ---
>
> NeilBrown (4):
>       nvdimm/blk: don't allocate unused major device number
>       nvdimm/btt: don't allocate unused major device number

The libnvdimm unit tests were fine with these, applied.

Btw, because you mentioned it, our unit test infrastructure does not
require real hardware.  See the ndctl readme [1], and if you have
copious amounts of free time the lwn write up on how we're mocking
resources [2].

[1]: https://github.com/pmem/ndctl/blob/master/README.md
[2]: https://lwn.net/Articles/654071/

Re: [PATCH 0/4] Remove un-needed 'major' registration when alloc_disk(0) is used.

[Ross Zwisler]

On Thu, Mar 10, 2016 at 08:59:28AM +1100, NeilBrown wrote:
> When alloc_disk(0) is used, the ->major number is ignored and
> irrelevant.  Yet several drivers register a major number anyway.
> 
> This series of patches removes the pointless registrations.  The pmem
> driver also does this, but a patch has already been sent for that
> driver.
> 
> Note that I am not in a position to test these beyond simple compile
> testing.
> 
> Thanks,
> NeilBrown
> 
> 
> ---
> 
> NeilBrown (4):
>       nvdimm/blk: don't allocate unused major device number
>       nvdimm/btt: don't allocate unused major device number
>       memstick: don't allocate unused major for ms_block
>       NVMe: don't allocate unused nvme_major
> 
> 
>  drivers/memstick/core/ms_block.c |   17 ++---------------
>  drivers/nvdimm/blk.c             |   18 +-----------------
>  drivers/nvdimm/btt.c             |   19 ++-----------------
>  drivers/nvme/host/core.c         |   16 +---------------
>  4 files changed, 6 insertions(+), 64 deletions(-)

There are several other drivers that allocate a major, but then use it for
some small number of minors (1 for null_blk.c and 16 for virtio_blk.c).  They
both have GENHD_FL_EXT_DEVT set, so I think what happens is that after we
exhaust the allocated minors they hop over to having BLOCK_EXT_MAJOR as a
major and a dynamically assigned minor.

It seems like these could easily be converted in the same way so they'd use
BLOCK_EXT_MAJOR for their major and have a bunch of dynamically assigned
minors.

Does this break something I'm not seeing?

Yay for this series, by the way. :) 

Re: [PATCH 0/4] Remove un-needed 'major' registration when alloc_disk(0) is used.

[NeilBrown]

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

On Tue, Mar 15 2016, Ross Zwisler wrote:

> On Thu, Mar 10, 2016 at 08:59:28AM +1100, NeilBrown wrote:
>> When alloc_disk(0) is used, the ->major number is ignored and
>> irrelevant.  Yet several drivers register a major number anyway.
>> 
>> This series of patches removes the pointless registrations.  The pmem
>> driver also does this, but a patch has already been sent for that
>> driver.
>> 
>> Note that I am not in a position to test these beyond simple compile
>> testing.
>> 
>> Thanks,
>> NeilBrown
>> 
>> 
>> ---
>> 
>> NeilBrown (4):
>>       nvdimm/blk: don't allocate unused major device number
>>       nvdimm/btt: don't allocate unused major device number
>>       memstick: don't allocate unused major for ms_block
>>       NVMe: don't allocate unused nvme_major
>> 
>> 
>>  drivers/memstick/core/ms_block.c |   17 ++---------------
>>  drivers/nvdimm/blk.c             |   18 +-----------------
>>  drivers/nvdimm/btt.c             |   19 ++-----------------
>>  drivers/nvme/host/core.c         |   16 +---------------
>>  4 files changed, 6 insertions(+), 64 deletions(-)
>
> There are several other drivers that allocate a major, but then use it for
> some small number of minors (1 for null_blk.c and 16 for virtio_blk.c).  They
> both have GENHD_FL_EXT_DEVT set, so I think what happens is that after we
> exhaust the allocated minors they hop over to having BLOCK_EXT_MAJOR as a
> major and a dynamically assigned minor.

null_blk looks like it would be safe to convert - it is just used for
testing.  Jens Axboe would probably know for sure.

virtio_blk is a much older and there may will be code which has some
sort of expectations about minor numbers.  I think it would not be worth
the risks to change it.

>
> It seems like these could easily be converted in the same way so they'd use
> BLOCK_EXT_MAJOR for their major and have a bunch of dynamically assigned
> minors.
>
> Does this break something I'm not seeing?
>
> Yay for this series, by the way. :) 

Thanks... two are in -next now (thank Dan) - I might poke the other two
in a week or two if nothing happens.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH 0/4] Remove un-needed 'major' registration when alloc_disk(0) is used.

[Jens Axboe]

On 03/15/2016 03:15 PM, NeilBrown wrote:
> On Tue, Mar 15 2016, Ross Zwisler wrote:
>
>> On Thu, Mar 10, 2016 at 08:59:28AM +1100, NeilBrown wrote:
>>> When alloc_disk(0) is used, the ->major number is ignored and
>>> irrelevant.  Yet several drivers register a major number anyway.
>>>
>>> This series of patches removes the pointless registrations.  The pmem
>>> driver also does this, but a patch has already been sent for that
>>> driver.
>>>
>>> Note that I am not in a position to test these beyond simple compile
>>> testing.
>>>
>>> Thanks,
>>> NeilBrown
>>>
>>>
>>> ---
>>>
>>> NeilBrown (4):
>>>        nvdimm/blk: don't allocate unused major device number
>>>        nvdimm/btt: don't allocate unused major device number
>>>        memstick: don't allocate unused major for ms_block
>>>        NVMe: don't allocate unused nvme_major
>>>
>>>
>>>   drivers/memstick/core/ms_block.c |   17 ++---------------
>>>   drivers/nvdimm/blk.c             |   18 +-----------------
>>>   drivers/nvdimm/btt.c             |   19 ++-----------------
>>>   drivers/nvme/host/core.c         |   16 +---------------
>>>   4 files changed, 6 insertions(+), 64 deletions(-)
>>
>> There are several other drivers that allocate a major, but then use it for
>> some small number of minors (1 for null_blk.c and 16 for virtio_blk.c).  They
>> both have GENHD_FL_EXT_DEVT set, so I think what happens is that after we
>> exhaust the allocated minors they hop over to having BLOCK_EXT_MAJOR as a
>> major and a dynamically assigned minor.
>
> null_blk looks like it would be safe to convert - it is just used for
> testing.  Jens Axboe would probably know for sure.
>
> virtio_blk is a much older and there may will be code which has some
> sort of expectations about minor numbers.  I think it would not be worth
> the risks to change it.

Agree on both - null_blk can be trivially converted, and I too would be 
worried about virt_blkio changes breaking existing assumptions.

-- 
Jens Axboe