Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1354509 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/4] Remove un-needed 'major' registration when  alloc_disk(0) is used. NeilBrown <neilb@suse.com> - 2016-03-09 23:10 +0100
    [PATCH 2/4] nvdimm/btt: don't allocate unused major device number NeilBrown <neilb@suse.com> - 2016-03-09 23:10 +0100
    [PATCH 1/4] nvdimm/blk: don't allocate unused major device number NeilBrown <neilb@suse.com> - 2016-03-09 23:10 +0100
      Re: [PATCH 1/4] nvdimm/blk: don't allocate unused major device number Johannes Thumshirn <jthumshirn@suse.de> - 2016-03-10 10:10 +0100
    Re: [PATCH 0/4] Remove un-needed 'major' registration when  alloc_disk(0) is used. Dan Williams <dan.j.williams@intel.com> - 2016-03-10 00:40 +0100
    Re: [PATCH 0/4] Remove un-needed 'major' registration when  alloc_disk(0) is used. Ross Zwisler <ross.zwisler@linux.intel.com> - 2016-03-15 00:00 +0100
      Re: [PATCH 0/4] Remove un-needed 'major' registration when alloc_disk(0) is used. NeilBrown <neilb@suse.com> - 2016-03-15 23:20 +0100
        Re: [PATCH 0/4] Remove un-needed 'major' registration when  alloc_disk(0) is used. Jens Axboe <axboe@fb.com> - 2016-03-16 01:30 +0100

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

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

[toc] | [next] | [standalone]

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

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);

[toc] | [prev] | [next] | [standalone]

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

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>");

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

#1354625

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/

[toc] | [prev] | [next] | [standalone]

#1357672

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. :) 

[toc] | [prev] | [next] | [standalone]

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

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

#1358451

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web