| Message ID | 1426539315-17392-1-git-send-email-dehrenberg@chromium.org |
|---|---|
| State | Changes Requested |
| Headers | show |
(Adding UBI maintainers) On 03/16/2015 05:55 PM, Dan Ehrenberg wrote: > This patch adds a new config option, MTD_UBI_BLOCK_DYNAMIC_MINOR, which > makes minor numbers dynamically allocated for ubiblock devices, starting > at 0. > > From the new Kconfig: > This option makes ubiblock devices have minor numbers beginning from > 0, allocated dynamically independently of the ubi device/volume > number. For users who don't refer to an ubiblock device directly by > the major:minor number (e.g., with udev and/or devtmpfs), this option > is useful if the device number is rather high (>1) and when executing > on a 32-bit platform without LFS enabled in all userspace binaries. > Hm, I'm wondering if there are any users relying on a specific minor. I'm not fond of the new option, and I'm not fond of the ifdefs you bring in. Can't we just change minors to be dynamic?
dynamic in nature. Nick On March 16, 2015 7:12:34 PM EDT, Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote: >(Adding UBI maintainers) > >On 03/16/2015 05:55 PM, Dan Ehrenberg wrote: >> This patch adds a new config option, MTD_UBI_BLOCK_DYNAMIC_MINOR, >which >> makes minor numbers dynamically allocated for ubiblock devices, >starting >> at 0. >> >> From the new Kconfig: >> This option makes ubiblock devices have minor numbers beginning >from >> 0, allocated dynamically independently of the ubi device/volume >> number. For users who don't refer to an ubiblock device directly >by >> the major:minor number (e.g., with udev and/or devtmpfs), this >option >> is useful if the device number is rather high (>1) and when >executing >> on a 32-bit platform without LFS enabled in all userspace >binaries. >> > >Hm, I'm wondering if there are any users relying on a specific minor. > >I'm not fond of the new option, and I'm not fond of the ifdefs you >bring >in. Can't we just change minors to be dynamic? Richard, I have to agree and was going to point out the ugliness of adding these macros. Furthermore I second the vote for just making minor device numbers given to Uni fs
I'd be happy to go that way. Given that the major is dynamic, it'd be a little surprising for someone to depend on a specific minor (yet). Dan On Mon, Mar 16, 2015 at 4:12 PM, Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote: > (Adding UBI maintainers) > > On 03/16/2015 05:55 PM, Dan Ehrenberg wrote: >> This patch adds a new config option, MTD_UBI_BLOCK_DYNAMIC_MINOR, which >> makes minor numbers dynamically allocated for ubiblock devices, starting >> at 0. >> >> From the new Kconfig: >> This option makes ubiblock devices have minor numbers beginning from >> 0, allocated dynamically independently of the ubi device/volume >> number. For users who don't refer to an ubiblock device directly by >> the major:minor number (e.g., with udev and/or devtmpfs), this option >> is useful if the device number is rather high (>1) and when executing >> on a 32-bit platform without LFS enabled in all userspace binaries. >> > > Hm, I'm wondering if there are any users relying on a specific minor. > > I'm not fond of the new option, and I'm not fond of the ifdefs you bring > in. Can't we just change minors to be dynamic? > -- > Ezequiel Garcia, VanguardiaSur > www.vanguardiasur.com.ar
diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig index f0855ce..c0e9a73 100644 --- a/drivers/mtd/ubi/Kconfig +++ b/drivers/mtd/ubi/Kconfig @@ -103,4 +103,15 @@ config MTD_UBI_BLOCK If in doubt, say "N". +config MTD_UBI_BLOCK_DYNAMIC_MINOR + bool "Dynamically allocate minor numbers for ubiblock" + default n + help + This option makes ubiblock devices have minor numbers beginning from + 0, allocated dynamically independently of the ubi device/volume + number. For users who don't refer to an ubiblock device directly by + the major:minor number (e.g., with udev and/or devtmpfs), this option + is useful if the device number is rather high (>1) and when executing + on a 32-bit platform without LFS. + endif # MTD_UBI diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c index db2c05b..bcd164d 100644 --- a/drivers/mtd/ubi/block.c +++ b/drivers/mtd/ubi/block.c @@ -48,6 +48,7 @@ #include <linux/blk-mq.h> #include <linux/hdreg.h> #include <linux/scatterlist.h> +#include <linux/idr.h> #include <asm/div64.h> #include "ubi-media.h" @@ -351,6 +352,20 @@ static struct blk_mq_ops ubiblock_mq_ops = { .map_queue = blk_mq_map_queue, }; +#ifdef CONFIG_MTD_UBI_BLOCK_DYNAMIC_MINOR +static DEFINE_IDR(ubiblock_minor_idr); + +static int ubiblock_alloc_minor(struct ubiblock *dev) +{ + return idr_alloc(&ubiblock_minor_idr, dev, 0, 0, GFP_KERNEL); +} + +static void ubiblock_remove_minor(int minor) +{ + idr_remove(&ubiblock_minor_idr, minor); +} +#endif + int ubiblock_create(struct ubi_volume_info *vi) { struct ubiblock *dev; @@ -388,7 +403,17 @@ int ubiblock_create(struct ubi_volume_info *vi) gd->fops = &ubiblock_ops; gd->major = ubiblock_major; +#ifdef CONFIG_MTD_UBI_BLOCK_DYNAMIC_MINOR + gd->first_minor = ubiblock_alloc_minor(dev); + if (gd->first_minor < 0) { + dev_err(disk_to_dev(gd), + "block: dynamic minor allocation failed"); + ret = -ENODEV; + goto out_put_disk; + } +#else gd->first_minor = dev->ubi_num * UBI_MAX_VOLUMES + dev->vol_id; +#endif gd->private_data = dev; sprintf(gd->disk_name, "ubiblock%d_%d", dev->ubi_num, dev->vol_id); set_capacity(gd, disk_capacity); @@ -405,7 +430,7 @@ int ubiblock_create(struct ubi_volume_info *vi) ret = blk_mq_alloc_tag_set(&dev->tag_set); if (ret) { dev_err(disk_to_dev(dev->gd), "blk_mq_alloc_tag_set failed"); - goto out_put_disk; + goto out_remove_minor; } dev->rq = blk_mq_init_queue(&dev->tag_set); @@ -443,6 +468,10 @@ out_free_queue: blk_cleanup_queue(dev->rq); out_free_tags: blk_mq_free_tag_set(&dev->tag_set); +out_remove_minor: +#ifdef CONFIG_MTD_UBI_BLOCK_DYNAMIC_MINOR + ubiblock_remove_minor(gd->first_minor); +#endif out_put_disk: put_disk(dev->gd); out_free_dev: @@ -461,6 +490,9 @@ static void ubiblock_cleanup(struct ubiblock *dev) blk_cleanup_queue(dev->rq); blk_mq_free_tag_set(&dev->tag_set); dev_info(disk_to_dev(dev->gd), "released"); +#ifdef CONFIG_MTD_UBI_BLOCK_DYNAMIC_MINOR + ubiblock_remove_minor(dev->gd->first_minor); +#endif put_disk(dev->gd); }
This patch adds a new config option, MTD_UBI_BLOCK_DYNAMIC_MINOR, which makes minor numbers dynamically allocated for ubiblock devices, starting at 0. From the new Kconfig: This option makes ubiblock devices have minor numbers beginning from 0, allocated dynamically independently of the ubi device/volume number. For users who don't refer to an ubiblock device directly by the major:minor number (e.g., with udev and/or devtmpfs), this option is useful if the device number is rather high (>1) and when executing on a 32-bit platform without LFS enabled in all userspace binaries. While enabling LFS is clearly a nicer solution, it's often difficult to turn on in practice globally as many widely distributed packages don't work with LFS on. While dev_t is 32-bit even without LFS, stat will return -EOVERFLOW with a minor number which doesn't fit in 8 bits for ABI compatability reasons. Other storage systems have their own workarounds, with SCSI making multiple device majors and MMC having a config option for the number of partitions per device. A completely dynamic minor numbering is simpler than these and suitable for the majority of ubiblock users who don't depend on a minor numbering (the major is dynamic anyway). Signed-off-by: Dan Ehrenberg <dehrenberg@chromium.org> --- drivers/mtd/ubi/Kconfig | 11 +++++++++++ drivers/mtd/ubi/block.c | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 1 deletion(-)