* cleanup ubd gendisk registration @ 2021-06-14 6:07 ` Christoph Hellwig 0 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2021-06-14 6:07 UTC (permalink / raw) To: Richard Weinberger, Anton Ivanov, Jens Axboe Cc: linux-um, linux-kernel, linux-block Hi all, this series sits on top of Jens' for-5.14/block branch and tries to convert ubd to the new gendisk and request_queue registration helpers. As part of that I found that the ide emulation code currently registers two gendisk for a request_queue which leads to a bunch of problems we've avoided in other drivers (only the mmc subsystem has a similar issue). Given that the legacy IDE driver isn't practically used any more and modern userspace doesn't hard code specific block drivers, so I think we can just drop it. Let me know if this is ok. ^ permalink raw reply [flat|nested] 11+ messages in thread
* cleanup ubd gendisk registration @ 2021-06-14 6:07 ` Christoph Hellwig 0 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2021-06-14 6:07 UTC (permalink / raw) To: Richard Weinberger, Anton Ivanov, Jens Axboe Cc: linux-um, linux-kernel, linux-block Hi all, this series sits on top of Jens' for-5.14/block branch and tries to convert ubd to the new gendisk and request_queue registration helpers. As part of that I found that the ide emulation code currently registers two gendisk for a request_queue which leads to a bunch of problems we've avoided in other drivers (only the mmc subsystem has a similar issue). Given that the legacy IDE driver isn't practically used any more and modern userspace doesn't hard code specific block drivers, so I think we can just drop it. Let me know if this is ok. _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] ubd: remove the code to register as the legacy IDE driver 2021-06-14 6:07 ` Christoph Hellwig @ 2021-06-14 6:07 ` Christoph Hellwig -1 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2021-06-14 6:07 UTC (permalink / raw) To: Richard Weinberger, Anton Ivanov, Jens Axboe Cc: linux-um, linux-kernel, linux-block With the legacy IDE driver long deprecated, and modern userspace being much more flexible about dev_t assignments there is no reason to fake a registration as the legacy IDE driver in ubd. This registeration is a little problematic as it registers the same request_queue for multiple gendisks, so just remove it. Signed-off-by: Christoph Hellwig <hch@lst.de> --- arch/um/drivers/ubd_kern.c | 106 +++++-------------------------------- 1 file changed, 12 insertions(+), 94 deletions(-) diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c index 8e0b43cf089f..f508d45f7a69 100644 --- a/arch/um/drivers/ubd_kern.c +++ b/arch/um/drivers/ubd_kern.c @@ -125,9 +125,7 @@ static const struct block_device_operations ubd_blops = { }; /* Protected by ubd_lock */ -static int fake_major = UBD_MAJOR; static struct gendisk *ubd_gendisk[MAX_DEV]; -static struct gendisk *fake_gendisk[MAX_DEV]; #ifdef CONFIG_BLK_DEV_UBD_SYNC #define OPEN_FLAGS ((struct openflags) { .r = 1, .w = 1, .s = 1, .c = 0, \ @@ -197,54 +195,19 @@ struct ubd { /* Protected by ubd_lock */ static struct ubd ubd_devs[MAX_DEV] = { [0 ... MAX_DEV - 1] = DEFAULT_UBD }; -/* Only changed by fake_ide_setup which is a setup */ -static int fake_ide = 0; -static struct proc_dir_entry *proc_ide_root = NULL; -static struct proc_dir_entry *proc_ide = NULL; - static blk_status_t ubd_queue_rq(struct blk_mq_hw_ctx *hctx, const struct blk_mq_queue_data *bd); -static void make_proc_ide(void) -{ - proc_ide_root = proc_mkdir("ide", NULL); - proc_ide = proc_mkdir("ide0", proc_ide_root); -} - -static int fake_ide_media_proc_show(struct seq_file *m, void *v) -{ - seq_puts(m, "disk\n"); - return 0; -} - -static void make_ide_entries(const char *dev_name) -{ - struct proc_dir_entry *dir, *ent; - char name[64]; - - if(proc_ide_root == NULL) make_proc_ide(); - - dir = proc_mkdir(dev_name, proc_ide); - if(!dir) return; - - ent = proc_create_single("media", S_IRUGO, dir, - fake_ide_media_proc_show); - if(!ent) return; - snprintf(name, sizeof(name), "ide0/%s", dev_name); - proc_symlink(dev_name, proc_ide_root, name); -} - static int fake_ide_setup(char *str) { - fake_ide = 1; + pr_warn("The fake_ide option has been removed\n"); return 1; } - __setup("fake_ide", fake_ide_setup); __uml_help(fake_ide_setup, "fake_ide\n" -" Create ide0 entries that map onto ubd devices.\n\n" +" Obsolete stub.\n\n" ); static int parse_unit(char **ptr) @@ -296,20 +259,8 @@ static int ubd_setup_common(char *str, int *index_out, char **error_out) return err; } - mutex_lock(&ubd_lock); - if (fake_major != UBD_MAJOR) { - *error_out = "Can't assign a fake major twice"; - goto out1; - } - - fake_major = major; - - printk(KERN_INFO "Setting extra ubd major number to %d\n", - major); - err = 0; - out1: - mutex_unlock(&ubd_lock); - return err; + pr_warn("fake major not supported any more\n"); + return 0; } n = parse_unit(&str); @@ -917,7 +868,6 @@ static const struct attribute_group *ubd_attr_groups[] = { static int ubd_disk_register(int major, u64 size, int unit, struct gendisk **disk_out) { - struct device *parent = NULL; struct gendisk *disk; disk = alloc_disk(1 << UBD_SHIFT); @@ -928,24 +878,17 @@ static int ubd_disk_register(int major, u64 size, int unit, disk->first_minor = unit << UBD_SHIFT; disk->fops = &ubd_blops; set_capacity(disk, size / 512); - if (major == UBD_MAJOR) - sprintf(disk->disk_name, "ubd%c", 'a' + unit); - else - sprintf(disk->disk_name, "ubd_fake%d", unit); - - /* sysfs register (not for ide fake devices) */ - if (major == UBD_MAJOR) { - ubd_devs[unit].pdev.id = unit; - ubd_devs[unit].pdev.name = DRIVER_NAME; - ubd_devs[unit].pdev.dev.release = ubd_device_release; - dev_set_drvdata(&ubd_devs[unit].pdev.dev, &ubd_devs[unit]); - platform_device_register(&ubd_devs[unit].pdev); - parent = &ubd_devs[unit].pdev.dev; - } + sprintf(disk->disk_name, "ubd%c", 'a' + unit); + + ubd_devs[unit].pdev.id = unit; + ubd_devs[unit].pdev.name = DRIVER_NAME; + ubd_devs[unit].pdev.dev.release = ubd_device_release; + dev_set_drvdata(&ubd_devs[unit].pdev.dev, &ubd_devs[unit]); + platform_device_register(&ubd_devs[unit].pdev); disk->private_data = &ubd_devs[unit]; disk->queue = ubd_devs[unit].queue; - device_add_disk(parent, disk, ubd_attr_groups); + device_add_disk(&ubd_devs[unit].pdev.dev, disk, ubd_attr_groups); *disk_out = disk; return 0; @@ -1001,17 +944,6 @@ static int ubd_add(int n, char **error_out) goto out_cleanup_tags; } - if (fake_major != UBD_MAJOR) - ubd_disk_register(fake_major, ubd_dev->size, n, - &fake_gendisk[n]); - - /* - * Perhaps this should also be under the "if (fake_major)" above - * using the fake_disk->disk_name - */ - if (fake_ide) - make_ide_entries(ubd_gendisk[n]->disk_name); - err = 0; out: return err; @@ -1126,12 +1058,6 @@ static int ubd_remove(int n, char **error_out) put_disk(disk); } - if(fake_gendisk[n] != NULL){ - del_gendisk(fake_gendisk[n]); - put_disk(fake_gendisk[n]); - fake_gendisk[n] = NULL; - } - err = 0; platform_device_unregister(&ubd_dev->pdev); out: @@ -1188,14 +1114,6 @@ static int __init ubd_init(void) if (register_blkdev(UBD_MAJOR, "ubd")) return -1; - if (fake_major != UBD_MAJOR) { - char name[sizeof("ubd_nnn\0")]; - - snprintf(name, sizeof(name), "ubd_%d", fake_major); - if (register_blkdev(fake_major, "ubd")) - return -1; - } - irq_req_buffer = kmalloc_array(UBD_REQ_BUFFER_SIZE, sizeof(struct io_thread_req *), GFP_KERNEL -- 2.30.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 1/2] ubd: remove the code to register as the legacy IDE driver @ 2021-06-14 6:07 ` Christoph Hellwig 0 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2021-06-14 6:07 UTC (permalink / raw) To: Richard Weinberger, Anton Ivanov, Jens Axboe Cc: linux-um, linux-kernel, linux-block With the legacy IDE driver long deprecated, and modern userspace being much more flexible about dev_t assignments there is no reason to fake a registration as the legacy IDE driver in ubd. This registeration is a little problematic as it registers the same request_queue for multiple gendisks, so just remove it. Signed-off-by: Christoph Hellwig <hch@lst.de> --- arch/um/drivers/ubd_kern.c | 106 +++++-------------------------------- 1 file changed, 12 insertions(+), 94 deletions(-) diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c index 8e0b43cf089f..f508d45f7a69 100644 --- a/arch/um/drivers/ubd_kern.c +++ b/arch/um/drivers/ubd_kern.c @@ -125,9 +125,7 @@ static const struct block_device_operations ubd_blops = { }; /* Protected by ubd_lock */ -static int fake_major = UBD_MAJOR; static struct gendisk *ubd_gendisk[MAX_DEV]; -static struct gendisk *fake_gendisk[MAX_DEV]; #ifdef CONFIG_BLK_DEV_UBD_SYNC #define OPEN_FLAGS ((struct openflags) { .r = 1, .w = 1, .s = 1, .c = 0, \ @@ -197,54 +195,19 @@ struct ubd { /* Protected by ubd_lock */ static struct ubd ubd_devs[MAX_DEV] = { [0 ... MAX_DEV - 1] = DEFAULT_UBD }; -/* Only changed by fake_ide_setup which is a setup */ -static int fake_ide = 0; -static struct proc_dir_entry *proc_ide_root = NULL; -static struct proc_dir_entry *proc_ide = NULL; - static blk_status_t ubd_queue_rq(struct blk_mq_hw_ctx *hctx, const struct blk_mq_queue_data *bd); -static void make_proc_ide(void) -{ - proc_ide_root = proc_mkdir("ide", NULL); - proc_ide = proc_mkdir("ide0", proc_ide_root); -} - -static int fake_ide_media_proc_show(struct seq_file *m, void *v) -{ - seq_puts(m, "disk\n"); - return 0; -} - -static void make_ide_entries(const char *dev_name) -{ - struct proc_dir_entry *dir, *ent; - char name[64]; - - if(proc_ide_root == NULL) make_proc_ide(); - - dir = proc_mkdir(dev_name, proc_ide); - if(!dir) return; - - ent = proc_create_single("media", S_IRUGO, dir, - fake_ide_media_proc_show); - if(!ent) return; - snprintf(name, sizeof(name), "ide0/%s", dev_name); - proc_symlink(dev_name, proc_ide_root, name); -} - static int fake_ide_setup(char *str) { - fake_ide = 1; + pr_warn("The fake_ide option has been removed\n"); return 1; } - __setup("fake_ide", fake_ide_setup); __uml_help(fake_ide_setup, "fake_ide\n" -" Create ide0 entries that map onto ubd devices.\n\n" +" Obsolete stub.\n\n" ); static int parse_unit(char **ptr) @@ -296,20 +259,8 @@ static int ubd_setup_common(char *str, int *index_out, char **error_out) return err; } - mutex_lock(&ubd_lock); - if (fake_major != UBD_MAJOR) { - *error_out = "Can't assign a fake major twice"; - goto out1; - } - - fake_major = major; - - printk(KERN_INFO "Setting extra ubd major number to %d\n", - major); - err = 0; - out1: - mutex_unlock(&ubd_lock); - return err; + pr_warn("fake major not supported any more\n"); + return 0; } n = parse_unit(&str); @@ -917,7 +868,6 @@ static const struct attribute_group *ubd_attr_groups[] = { static int ubd_disk_register(int major, u64 size, int unit, struct gendisk **disk_out) { - struct device *parent = NULL; struct gendisk *disk; disk = alloc_disk(1 << UBD_SHIFT); @@ -928,24 +878,17 @@ static int ubd_disk_register(int major, u64 size, int unit, disk->first_minor = unit << UBD_SHIFT; disk->fops = &ubd_blops; set_capacity(disk, size / 512); - if (major == UBD_MAJOR) - sprintf(disk->disk_name, "ubd%c", 'a' + unit); - else - sprintf(disk->disk_name, "ubd_fake%d", unit); - - /* sysfs register (not for ide fake devices) */ - if (major == UBD_MAJOR) { - ubd_devs[unit].pdev.id = unit; - ubd_devs[unit].pdev.name = DRIVER_NAME; - ubd_devs[unit].pdev.dev.release = ubd_device_release; - dev_set_drvdata(&ubd_devs[unit].pdev.dev, &ubd_devs[unit]); - platform_device_register(&ubd_devs[unit].pdev); - parent = &ubd_devs[unit].pdev.dev; - } + sprintf(disk->disk_name, "ubd%c", 'a' + unit); + + ubd_devs[unit].pdev.id = unit; + ubd_devs[unit].pdev.name = DRIVER_NAME; + ubd_devs[unit].pdev.dev.release = ubd_device_release; + dev_set_drvdata(&ubd_devs[unit].pdev.dev, &ubd_devs[unit]); + platform_device_register(&ubd_devs[unit].pdev); disk->private_data = &ubd_devs[unit]; disk->queue = ubd_devs[unit].queue; - device_add_disk(parent, disk, ubd_attr_groups); + device_add_disk(&ubd_devs[unit].pdev.dev, disk, ubd_attr_groups); *disk_out = disk; return 0; @@ -1001,17 +944,6 @@ static int ubd_add(int n, char **error_out) goto out_cleanup_tags; } - if (fake_major != UBD_MAJOR) - ubd_disk_register(fake_major, ubd_dev->size, n, - &fake_gendisk[n]); - - /* - * Perhaps this should also be under the "if (fake_major)" above - * using the fake_disk->disk_name - */ - if (fake_ide) - make_ide_entries(ubd_gendisk[n]->disk_name); - err = 0; out: return err; @@ -1126,12 +1058,6 @@ static int ubd_remove(int n, char **error_out) put_disk(disk); } - if(fake_gendisk[n] != NULL){ - del_gendisk(fake_gendisk[n]); - put_disk(fake_gendisk[n]); - fake_gendisk[n] = NULL; - } - err = 0; platform_device_unregister(&ubd_dev->pdev); out: @@ -1188,14 +1114,6 @@ static int __init ubd_init(void) if (register_blkdev(UBD_MAJOR, "ubd")) return -1; - if (fake_major != UBD_MAJOR) { - char name[sizeof("ubd_nnn\0")]; - - snprintf(name, sizeof(name), "ubd_%d", fake_major); - if (register_blkdev(fake_major, "ubd")) - return -1; - } - irq_req_buffer = kmalloc_array(UBD_REQ_BUFFER_SIZE, sizeof(struct io_thread_req *), GFP_KERNEL -- 2.30.2 _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] ubd: remove the code to register as the legacy IDE driver 2021-06-14 6:07 ` Christoph Hellwig (?) @ 2021-06-14 8:22 ` Anton Ivanov -1 siblings, 0 replies; 11+ messages in thread From: Anton Ivanov @ 2021-06-14 8:22 UTC (permalink / raw) To: Christoph Hellwig, Richard Weinberger, Jens Axboe Cc: linux-um, linux-kernel, linux-block On 14/06/2021 07:07, Christoph Hellwig wrote: > With the legacy IDE driver long deprecated, and modern userspace being > much more flexible about dev_t assignments there is no reason to fake > a registration as the legacy IDE driver in ubd. This registeration > is a little problematic as it registers the same request_queue for > multiple gendisks, so just remove it. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > arch/um/drivers/ubd_kern.c | 106 +++++-------------------------------- > 1 file changed, 12 insertions(+), 94 deletions(-) > > diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c > index 8e0b43cf089f..f508d45f7a69 100644 > --- a/arch/um/drivers/ubd_kern.c > +++ b/arch/um/drivers/ubd_kern.c > @@ -125,9 +125,7 @@ static const struct block_device_operations ubd_blops = { > }; > > /* Protected by ubd_lock */ > -static int fake_major = UBD_MAJOR; > static struct gendisk *ubd_gendisk[MAX_DEV]; > -static struct gendisk *fake_gendisk[MAX_DEV]; > > #ifdef CONFIG_BLK_DEV_UBD_SYNC > #define OPEN_FLAGS ((struct openflags) { .r = 1, .w = 1, .s = 1, .c = 0, \ > @@ -197,54 +195,19 @@ struct ubd { > /* Protected by ubd_lock */ > static struct ubd ubd_devs[MAX_DEV] = { [0 ... MAX_DEV - 1] = DEFAULT_UBD }; > > -/* Only changed by fake_ide_setup which is a setup */ > -static int fake_ide = 0; > -static struct proc_dir_entry *proc_ide_root = NULL; > -static struct proc_dir_entry *proc_ide = NULL; > - > static blk_status_t ubd_queue_rq(struct blk_mq_hw_ctx *hctx, > const struct blk_mq_queue_data *bd); > > -static void make_proc_ide(void) > -{ > - proc_ide_root = proc_mkdir("ide", NULL); > - proc_ide = proc_mkdir("ide0", proc_ide_root); > -} > - > -static int fake_ide_media_proc_show(struct seq_file *m, void *v) > -{ > - seq_puts(m, "disk\n"); > - return 0; > -} > - > -static void make_ide_entries(const char *dev_name) > -{ > - struct proc_dir_entry *dir, *ent; > - char name[64]; > - > - if(proc_ide_root == NULL) make_proc_ide(); > - > - dir = proc_mkdir(dev_name, proc_ide); > - if(!dir) return; > - > - ent = proc_create_single("media", S_IRUGO, dir, > - fake_ide_media_proc_show); > - if(!ent) return; > - snprintf(name, sizeof(name), "ide0/%s", dev_name); > - proc_symlink(dev_name, proc_ide_root, name); > -} > - > static int fake_ide_setup(char *str) > { > - fake_ide = 1; > + pr_warn("The fake_ide option has been removed\n"); > return 1; > } > - > __setup("fake_ide", fake_ide_setup); > > __uml_help(fake_ide_setup, > "fake_ide\n" > -" Create ide0 entries that map onto ubd devices.\n\n" > +" Obsolete stub.\n\n" > ); > > static int parse_unit(char **ptr) > @@ -296,20 +259,8 @@ static int ubd_setup_common(char *str, int *index_out, char **error_out) > return err; > } > > - mutex_lock(&ubd_lock); > - if (fake_major != UBD_MAJOR) { > - *error_out = "Can't assign a fake major twice"; > - goto out1; > - } > - > - fake_major = major; > - > - printk(KERN_INFO "Setting extra ubd major number to %d\n", > - major); > - err = 0; > - out1: > - mutex_unlock(&ubd_lock); > - return err; > + pr_warn("fake major not supported any more\n"); > + return 0; > } > > n = parse_unit(&str); > @@ -917,7 +868,6 @@ static const struct attribute_group *ubd_attr_groups[] = { > static int ubd_disk_register(int major, u64 size, int unit, > struct gendisk **disk_out) > { > - struct device *parent = NULL; > struct gendisk *disk; > > disk = alloc_disk(1 << UBD_SHIFT); > @@ -928,24 +878,17 @@ static int ubd_disk_register(int major, u64 size, int unit, > disk->first_minor = unit << UBD_SHIFT; > disk->fops = &ubd_blops; > set_capacity(disk, size / 512); > - if (major == UBD_MAJOR) > - sprintf(disk->disk_name, "ubd%c", 'a' + unit); > - else > - sprintf(disk->disk_name, "ubd_fake%d", unit); > - > - /* sysfs register (not for ide fake devices) */ > - if (major == UBD_MAJOR) { > - ubd_devs[unit].pdev.id = unit; > - ubd_devs[unit].pdev.name = DRIVER_NAME; > - ubd_devs[unit].pdev.dev.release = ubd_device_release; > - dev_set_drvdata(&ubd_devs[unit].pdev.dev, &ubd_devs[unit]); > - platform_device_register(&ubd_devs[unit].pdev); > - parent = &ubd_devs[unit].pdev.dev; > - } > + sprintf(disk->disk_name, "ubd%c", 'a' + unit); > + > + ubd_devs[unit].pdev.id = unit; > + ubd_devs[unit].pdev.name = DRIVER_NAME; > + ubd_devs[unit].pdev.dev.release = ubd_device_release; > + dev_set_drvdata(&ubd_devs[unit].pdev.dev, &ubd_devs[unit]); > + platform_device_register(&ubd_devs[unit].pdev); > > disk->private_data = &ubd_devs[unit]; > disk->queue = ubd_devs[unit].queue; > - device_add_disk(parent, disk, ubd_attr_groups); > + device_add_disk(&ubd_devs[unit].pdev.dev, disk, ubd_attr_groups); > > *disk_out = disk; > return 0; > @@ -1001,17 +944,6 @@ static int ubd_add(int n, char **error_out) > goto out_cleanup_tags; > } > > - if (fake_major != UBD_MAJOR) > - ubd_disk_register(fake_major, ubd_dev->size, n, > - &fake_gendisk[n]); > - > - /* > - * Perhaps this should also be under the "if (fake_major)" above > - * using the fake_disk->disk_name > - */ > - if (fake_ide) > - make_ide_entries(ubd_gendisk[n]->disk_name); > - > err = 0; > out: > return err; > @@ -1126,12 +1058,6 @@ static int ubd_remove(int n, char **error_out) > put_disk(disk); > } > > - if(fake_gendisk[n] != NULL){ > - del_gendisk(fake_gendisk[n]); > - put_disk(fake_gendisk[n]); > - fake_gendisk[n] = NULL; > - } > - > err = 0; > platform_device_unregister(&ubd_dev->pdev); > out: > @@ -1188,14 +1114,6 @@ static int __init ubd_init(void) > if (register_blkdev(UBD_MAJOR, "ubd")) > return -1; > > - if (fake_major != UBD_MAJOR) { > - char name[sizeof("ubd_nnn\0")]; > - > - snprintf(name, sizeof(name), "ubd_%d", fake_major); > - if (register_blkdev(fake_major, "ubd")) > - return -1; > - } > - > irq_req_buffer = kmalloc_array(UBD_REQ_BUFFER_SIZE, > sizeof(struct io_thread_req *), > GFP_KERNEL > Acked-By: Anton Ivanov <anton.ivanov@cambridgegreys.com> -- Anton R. Ivanov Cambridgegreys Limited. Registered in England. Company Number 10273661 https://www.cambridgegreys.com/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] ubd: use blk_mq_alloc_disk and blk_cleanup_disk 2021-06-14 6:07 ` Christoph Hellwig @ 2021-06-14 6:07 ` Christoph Hellwig -1 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2021-06-14 6:07 UTC (permalink / raw) To: Richard Weinberger, Anton Ivanov, Jens Axboe Cc: linux-um, linux-kernel, linux-block Use blk_mq_alloc_disk and blk_cleanup_disk to simplify the gendisk and request_queue allocation. Signed-off-by: Christoph Hellwig <hch@lst.de> --- arch/um/drivers/ubd_kern.c | 44 ++++++++++++-------------------------- 1 file changed, 14 insertions(+), 30 deletions(-) diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c index f508d45f7a69..0b86aa1b12f1 100644 --- a/arch/um/drivers/ubd_kern.c +++ b/arch/um/drivers/ubd_kern.c @@ -825,7 +825,6 @@ static void ubd_device_release(struct device *dev) { struct ubd *ubd_dev = dev_get_drvdata(dev); - blk_cleanup_queue(ubd_dev->queue); blk_mq_free_tag_set(&ubd_dev->tag_set); *ubd_dev = ((struct ubd) DEFAULT_UBD); } @@ -865,17 +864,12 @@ static const struct attribute_group *ubd_attr_groups[] = { NULL, }; -static int ubd_disk_register(int major, u64 size, int unit, - struct gendisk **disk_out) +static void ubd_disk_register(int major, u64 size, int unit, + struct gendisk *disk) { - struct gendisk *disk; - - disk = alloc_disk(1 << UBD_SHIFT); - if(disk == NULL) - return -ENOMEM; - disk->major = major; disk->first_minor = unit << UBD_SHIFT; + disk->minors = 1 << UBD_SHIFT; disk->fops = &ubd_blops; set_capacity(disk, size / 512); sprintf(disk->disk_name, "ubd%c", 'a' + unit); @@ -889,9 +883,6 @@ static int ubd_disk_register(int major, u64 size, int unit, disk->private_data = &ubd_devs[unit]; disk->queue = ubd_devs[unit].queue; device_add_disk(&ubd_devs[unit].pdev.dev, disk, ubd_attr_groups); - - *disk_out = disk; - return 0; } #define ROUND_BLOCK(n) ((n + (SECTOR_SIZE - 1)) & (-SECTOR_SIZE)) @@ -903,6 +894,7 @@ static const struct blk_mq_ops ubd_mq_ops = { static int ubd_add(int n, char **error_out) { struct ubd *ubd_dev = &ubd_devs[n]; + struct gendisk *disk; int err = 0; if(ubd_dev->file == NULL) @@ -927,32 +919,24 @@ static int ubd_add(int n, char **error_out) if (err) goto out; - ubd_dev->queue = blk_mq_init_queue(&ubd_dev->tag_set); - if (IS_ERR(ubd_dev->queue)) { - err = PTR_ERR(ubd_dev->queue); + disk = blk_mq_alloc_disk(&ubd_dev->tag_set, ubd_dev); + if (IS_ERR(disk)) { + err = PTR_ERR(disk); goto out_cleanup_tags; } + ubd_dev->queue = disk->queue; - ubd_dev->queue->queuedata = ubd_dev; blk_queue_write_cache(ubd_dev->queue, true, false); - blk_queue_max_segments(ubd_dev->queue, MAX_SG); blk_queue_segment_boundary(ubd_dev->queue, PAGE_SIZE - 1); - err = ubd_disk_register(UBD_MAJOR, ubd_dev->size, n, &ubd_gendisk[n]); - if(err){ - *error_out = "Failed to register device"; - goto out_cleanup_tags; - } - - err = 0; -out: - return err; + ubd_disk_register(UBD_MAJOR, ubd_dev->size, n, disk); + ubd_gendisk[n] = disk; + return 0; out_cleanup_tags: blk_mq_free_tag_set(&ubd_dev->tag_set); - if (!(IS_ERR(ubd_dev->queue))) - blk_cleanup_queue(ubd_dev->queue); - goto out; +out: + return err; } static int ubd_config(char *str, char **error_out) @@ -1055,7 +1039,7 @@ static int ubd_remove(int n, char **error_out) ubd_gendisk[n] = NULL; if(disk != NULL){ del_gendisk(disk); - put_disk(disk); + blk_cleanup_disk(disk); } err = 0; -- 2.30.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] ubd: use blk_mq_alloc_disk and blk_cleanup_disk @ 2021-06-14 6:07 ` Christoph Hellwig 0 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2021-06-14 6:07 UTC (permalink / raw) To: Richard Weinberger, Anton Ivanov, Jens Axboe Cc: linux-um, linux-kernel, linux-block Use blk_mq_alloc_disk and blk_cleanup_disk to simplify the gendisk and request_queue allocation. Signed-off-by: Christoph Hellwig <hch@lst.de> --- arch/um/drivers/ubd_kern.c | 44 ++++++++++++-------------------------- 1 file changed, 14 insertions(+), 30 deletions(-) diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c index f508d45f7a69..0b86aa1b12f1 100644 --- a/arch/um/drivers/ubd_kern.c +++ b/arch/um/drivers/ubd_kern.c @@ -825,7 +825,6 @@ static void ubd_device_release(struct device *dev) { struct ubd *ubd_dev = dev_get_drvdata(dev); - blk_cleanup_queue(ubd_dev->queue); blk_mq_free_tag_set(&ubd_dev->tag_set); *ubd_dev = ((struct ubd) DEFAULT_UBD); } @@ -865,17 +864,12 @@ static const struct attribute_group *ubd_attr_groups[] = { NULL, }; -static int ubd_disk_register(int major, u64 size, int unit, - struct gendisk **disk_out) +static void ubd_disk_register(int major, u64 size, int unit, + struct gendisk *disk) { - struct gendisk *disk; - - disk = alloc_disk(1 << UBD_SHIFT); - if(disk == NULL) - return -ENOMEM; - disk->major = major; disk->first_minor = unit << UBD_SHIFT; + disk->minors = 1 << UBD_SHIFT; disk->fops = &ubd_blops; set_capacity(disk, size / 512); sprintf(disk->disk_name, "ubd%c", 'a' + unit); @@ -889,9 +883,6 @@ static int ubd_disk_register(int major, u64 size, int unit, disk->private_data = &ubd_devs[unit]; disk->queue = ubd_devs[unit].queue; device_add_disk(&ubd_devs[unit].pdev.dev, disk, ubd_attr_groups); - - *disk_out = disk; - return 0; } #define ROUND_BLOCK(n) ((n + (SECTOR_SIZE - 1)) & (-SECTOR_SIZE)) @@ -903,6 +894,7 @@ static const struct blk_mq_ops ubd_mq_ops = { static int ubd_add(int n, char **error_out) { struct ubd *ubd_dev = &ubd_devs[n]; + struct gendisk *disk; int err = 0; if(ubd_dev->file == NULL) @@ -927,32 +919,24 @@ static int ubd_add(int n, char **error_out) if (err) goto out; - ubd_dev->queue = blk_mq_init_queue(&ubd_dev->tag_set); - if (IS_ERR(ubd_dev->queue)) { - err = PTR_ERR(ubd_dev->queue); + disk = blk_mq_alloc_disk(&ubd_dev->tag_set, ubd_dev); + if (IS_ERR(disk)) { + err = PTR_ERR(disk); goto out_cleanup_tags; } + ubd_dev->queue = disk->queue; - ubd_dev->queue->queuedata = ubd_dev; blk_queue_write_cache(ubd_dev->queue, true, false); - blk_queue_max_segments(ubd_dev->queue, MAX_SG); blk_queue_segment_boundary(ubd_dev->queue, PAGE_SIZE - 1); - err = ubd_disk_register(UBD_MAJOR, ubd_dev->size, n, &ubd_gendisk[n]); - if(err){ - *error_out = "Failed to register device"; - goto out_cleanup_tags; - } - - err = 0; -out: - return err; + ubd_disk_register(UBD_MAJOR, ubd_dev->size, n, disk); + ubd_gendisk[n] = disk; + return 0; out_cleanup_tags: blk_mq_free_tag_set(&ubd_dev->tag_set); - if (!(IS_ERR(ubd_dev->queue))) - blk_cleanup_queue(ubd_dev->queue); - goto out; +out: + return err; } static int ubd_config(char *str, char **error_out) @@ -1055,7 +1039,7 @@ static int ubd_remove(int n, char **error_out) ubd_gendisk[n] = NULL; if(disk != NULL){ del_gendisk(disk); - put_disk(disk); + blk_cleanup_disk(disk); } err = 0; -- 2.30.2 _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] ubd: use blk_mq_alloc_disk and blk_cleanup_disk 2021-06-14 6:07 ` Christoph Hellwig (?) @ 2021-06-14 8:26 ` Anton Ivanov 2021-06-14 9:49 ` Christoph Hellwig -1 siblings, 1 reply; 11+ messages in thread From: Anton Ivanov @ 2021-06-14 8:26 UTC (permalink / raw) To: Christoph Hellwig, Richard Weinberger, Jens Axboe Cc: linux-um, linux-kernel, linux-block On 14/06/2021 07:07, Christoph Hellwig wrote: > Use blk_mq_alloc_disk and blk_cleanup_disk to simplify the gendisk and > request_queue allocation. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > arch/um/drivers/ubd_kern.c | 44 ++++++++++++-------------------------- > 1 file changed, 14 insertions(+), 30 deletions(-) > > diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c > index f508d45f7a69..0b86aa1b12f1 100644 > --- a/arch/um/drivers/ubd_kern.c > +++ b/arch/um/drivers/ubd_kern.c > @@ -825,7 +825,6 @@ static void ubd_device_release(struct device *dev) > { > struct ubd *ubd_dev = dev_get_drvdata(dev); > > - blk_cleanup_queue(ubd_dev->queue); > blk_mq_free_tag_set(&ubd_dev->tag_set); > *ubd_dev = ((struct ubd) DEFAULT_UBD); > } > @@ -865,17 +864,12 @@ static const struct attribute_group *ubd_attr_groups[] = { > NULL, > }; > > -static int ubd_disk_register(int major, u64 size, int unit, > - struct gendisk **disk_out) > +static void ubd_disk_register(int major, u64 size, int unit, > + struct gendisk *disk) > { > - struct gendisk *disk; > - > - disk = alloc_disk(1 << UBD_SHIFT); > - if(disk == NULL) > - return -ENOMEM; > - > disk->major = major; > disk->first_minor = unit << UBD_SHIFT; > + disk->minors = 1 << UBD_SHIFT; > disk->fops = &ubd_blops; > set_capacity(disk, size / 512); > sprintf(disk->disk_name, "ubd%c", 'a' + unit); > @@ -889,9 +883,6 @@ static int ubd_disk_register(int major, u64 size, int unit, > disk->private_data = &ubd_devs[unit]; > disk->queue = ubd_devs[unit].queue; > device_add_disk(&ubd_devs[unit].pdev.dev, disk, ubd_attr_groups); > - > - *disk_out = disk; > - return 0; > } > > #define ROUND_BLOCK(n) ((n + (SECTOR_SIZE - 1)) & (-SECTOR_SIZE)) > @@ -903,6 +894,7 @@ static const struct blk_mq_ops ubd_mq_ops = { > static int ubd_add(int n, char **error_out) > { > struct ubd *ubd_dev = &ubd_devs[n]; > + struct gendisk *disk; > int err = 0; > > if(ubd_dev->file == NULL) > @@ -927,32 +919,24 @@ static int ubd_add(int n, char **error_out) > if (err) > goto out; > > - ubd_dev->queue = blk_mq_init_queue(&ubd_dev->tag_set); > - if (IS_ERR(ubd_dev->queue)) { > - err = PTR_ERR(ubd_dev->queue); > + disk = blk_mq_alloc_disk(&ubd_dev->tag_set, ubd_dev); > + if (IS_ERR(disk)) { > + err = PTR_ERR(disk); > goto out_cleanup_tags; > } > + ubd_dev->queue = disk->queue; > > - ubd_dev->queue->queuedata = ubd_dev; > blk_queue_write_cache(ubd_dev->queue, true, false); > - > blk_queue_max_segments(ubd_dev->queue, MAX_SG); > blk_queue_segment_boundary(ubd_dev->queue, PAGE_SIZE - 1); > - err = ubd_disk_register(UBD_MAJOR, ubd_dev->size, n, &ubd_gendisk[n]); > - if(err){ > - *error_out = "Failed to register device"; > - goto out_cleanup_tags; > - } > - > - err = 0; > -out: > - return err; > + ubd_disk_register(UBD_MAJOR, ubd_dev->size, n, disk); > + ubd_gendisk[n] = disk; > + return 0; > > out_cleanup_tags: > blk_mq_free_tag_set(&ubd_dev->tag_set); > - if (!(IS_ERR(ubd_dev->queue))) > - blk_cleanup_queue(ubd_dev->queue); > - goto out; > +out: > + return err; > } > > static int ubd_config(char *str, char **error_out) > @@ -1055,7 +1039,7 @@ static int ubd_remove(int n, char **error_out) > ubd_gendisk[n] = NULL; > if(disk != NULL){ > del_gendisk(disk); > - put_disk(disk); > + blk_cleanup_disk(disk); > } > > err = 0; > This does not build for me when applied to master: arch/um/drivers/ubd_kern.c: In function ‘ubd_add’: arch/um/drivers/ubd_kern.c:922:9: error: implicit declaration of function ‘blk_mq_alloc_disk’; did you mean ‘blk_mq_alloc_request’? [-Werror=implicit-function-declaration] disk = blk_mq_alloc_disk(&ubd_dev->tag_set, ubd_dev); ^~~~~~~~~~~~~~~~~ blk_mq_alloc_request arch/um/drivers/ubd_kern.c:922:7: warning: assignment to ‘struct gendisk *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion] disk = blk_mq_alloc_disk(&ubd_dev->tag_set, ubd_dev); ^ arch/um/drivers/ubd_kern.c: In function ‘ubd_remove’: arch/um/drivers/ubd_kern.c:1042:3: error: implicit declaration of function ‘blk_cleanup_disk’; did you mean ‘blk_cleanup_queue’? [-Werror=implicit-function-declaration] blk_cleanup_disk(disk); -- Anton R. Ivanov Cambridgegreys Limited. Registered in England. Company Number 10273661 https://www.cambridgegreys.com/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] ubd: use blk_mq_alloc_disk and blk_cleanup_disk 2021-06-14 8:26 ` Anton Ivanov @ 2021-06-14 9:49 ` Christoph Hellwig 0 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2021-06-14 9:49 UTC (permalink / raw) To: Anton Ivanov Cc: Christoph Hellwig, Richard Weinberger, Jens Axboe, linux-um, linux-kernel, linux-block On Mon, Jun 14, 2021 at 09:26:58AM +0100, Anton Ivanov wrote: > This does not build for me when applied to master: Yes, as mentioned in the cover letter it needs for-5.14/block branch from here: git://git.kernel.dk/linux-block for-5.14/block Gitweb: https://git.kernel.dk/cgit/linux-block/log/?h=for-5.14/block ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] ubd: use blk_mq_alloc_disk and blk_cleanup_disk @ 2021-06-14 9:49 ` Christoph Hellwig 0 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2021-06-14 9:49 UTC (permalink / raw) To: Anton Ivanov Cc: Christoph Hellwig, Richard Weinberger, Jens Axboe, linux-um, linux-kernel, linux-block On Mon, Jun 14, 2021 at 09:26:58AM +0100, Anton Ivanov wrote: > This does not build for me when applied to master: Yes, as mentioned in the cover letter it needs for-5.14/block branch from here: git://git.kernel.dk/linux-block for-5.14/block Gitweb: https://git.kernel.dk/cgit/linux-block/log/?h=for-5.14/block _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: cleanup ubd gendisk registration 2021-06-14 6:07 ` Christoph Hellwig ` (2 preceding siblings ...) (?) @ 2021-06-15 21:57 ` Jens Axboe -1 siblings, 0 replies; 11+ messages in thread From: Jens Axboe @ 2021-06-15 21:57 UTC (permalink / raw) To: Christoph Hellwig, Richard Weinberger, Anton Ivanov Cc: linux-um, linux-kernel, linux-block On 6/14/21 12:07 AM, Christoph Hellwig wrote: > Hi all, > > this series sits on top of Jens' for-5.14/block branch and tries to > convert ubd to the new gendisk and request_queue registration helpers. > As part of that I found that the ide emulation code currently registers > two gendisk for a request_queue which leads to a bunch of problems we've > avoided in other drivers (only the mmc subsystem has a similar issue). > Given that the legacy IDE driver isn't practically used any more and > modern userspace doesn't hard code specific block drivers, so I think > we can just drop it. Let me know if this is ok. Applied, thanks. -- Jens Axboe ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-06-15 21:57 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-14 6:07 cleanup ubd gendisk registration Christoph Hellwig 2021-06-14 6:07 ` Christoph Hellwig 2021-06-14 6:07 ` [PATCH 1/2] ubd: remove the code to register as the legacy IDE driver Christoph Hellwig 2021-06-14 6:07 ` Christoph Hellwig 2021-06-14 8:22 ` Anton Ivanov 2021-06-14 6:07 ` [PATCH 2/2] ubd: use blk_mq_alloc_disk and blk_cleanup_disk Christoph Hellwig 2021-06-14 6:07 ` Christoph Hellwig 2021-06-14 8:26 ` Anton Ivanov 2021-06-14 9:49 ` Christoph Hellwig 2021-06-14 9:49 ` Christoph Hellwig 2021-06-15 21:57 ` cleanup ubd gendisk registration Jens Axboe
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.