block: third batch of add_disk() error handling conversions
Subsystems (6)
v2 cover block: third batch of add_disk() error handling conversions
This is the 2nd version of the third batch of driver conversions for the add_disk() error handling. This and the entire 7th series of driver conversions can be found on my 20210927-for-axboe-add-disk-error-handling branch [0].
On this v2 series the following changes have been made since the v1 series of this patch set:
- rebased onto linux-next tag 20210927
- z2ram: fixed compile warning reported by 0day and Geert Uytterhoe
- um/drivers/ubd_kern: added the reviewed-by tag by Gabriel Krisman Bertazi
Christoph Hellwig (4):
pcd: move the identify buffer into pcd_identify
pcd: cleanup initialization
pf: cleanup initialization
pd: cleanup initialization
Luis Chamberlain (11):
z2ram: add error handling support for add_disk()
aoe: add error handling support for add_disk() m68k/emu/nfblock: add error handling support for add_disk()
drbd: add error handling support for add_disk() um/drivers/ubd_kern: add error handling support for add_disk() xtensa/platforms/iss/simdisk: add error handling support for
add_disk()
n64cart: add error handling support for add_disk()
pcd: add error handling support for add_disk()
pcd: fix ordering of unregister_cdrom()
pcd: capture errors on cdrom_register()
pd: add error handling support for add_disk()
arch/m68k/emu/nfblock.c | 9 +- arch/um/drivers/ubd_kern.c | 13 +- arch/xtensa/platforms/iss/simdisk.c | 13 +- drivers/block/aoe/aoeblk.c | 6 +- drivers/block/drbd/drbd_main.c | 6 +- drivers/block/n64cart.c | 12 +- drivers/block/paride/pcd.c | 304 +++++++++++++--------------- drivers/block/paride/pd.c | 144 ++++++------- drivers/block/paride/pf.c | 223 +++++++++----------- drivers/block/z2ram.c | 7 +- 10 files changed, 364 insertions(+), 373 deletions(-)
Signature · 1 line
2.30.2
PATCH v2 1/15 z2ram: add error handling support for add_disk()
We never checked for errors on add_disk() as this function returned void. Now that this is fixed, use the shiny new error handling. Only the disk is cleaned up inside z2ram_register_disk() as the caller deals with the rest.
--- drivers/block/z2ram.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/block/z2ram.c +5 -2
@@ -318,6 +318,7 @@ static const struct blk_mq_ops z2_mq_ops = {static int z2ram_register_disk (int minor) {struct gendisk *disk; +int err; disk =blk_mq_alloc_disk (&tag_set,NULL );if (IS_ERR (disk))
@@ -333,8 +334,10 @@ static int z2ram_register_disk(int minor)sprintf (disk->disk_name,"z2ram" ); z2ram_gendisk[minor] = disk; -add_disk (disk); -return 0 ; + err =add_disk (disk); +if (err) +blk_cleanup_disk (disk); +return err; }static int __init z2_init (void )
Signature · 1 line
2.30.2
PATCH v2 2/15 aoe: add error handling support for add_disk()
In reply to Luis ChamberlainWe never checked for errors on add_disk() as this function returned void. Now that this is fixed, use the shiny new error handling.
--- drivers/block/aoe/aoeblk.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/block/aoe/aoeblk.c +5 -1
@@ -417,7 +417,9 @@ aoeblk_gdalloc(void *vp)spin_unlock_irqrestore (&d->lock, flags); -device_add_disk (NULL , gd, aoe_attr_groups); + err =device_add_disk (NULL , gd, aoe_attr_groups); +if (err) + goto out_disk_cleanup;aoedisk_add_debugfs (d);spin_lock_irqsave (&d->lock, flags);
@@ -426,6 +428,8 @@ aoeblk_gdalloc(void *vp)spin_unlock_irqrestore (&d->lock, flags);return ; +out_disk_cleanup: +blk_cleanup_disk (gd); err_tagset:blk_mq_free_tag_set (set); err_mempool:
Signature · 1 line
2.30.2
PATCH v2 3/15 m68k/emu/nfblock: add error handling support for add_disk()
In reply to Luis ChamberlainWe never checked for errors on add_disk() as this function returned void. Now that this is fixed, use the shiny new error handling.
--- arch/m68k/emu/nfblock.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
arch/m68k/emu/nfblock.c +7 -2
@@ -100,6 +100,7 @@ static int __init nfhd_init_one(int id, u32 blocks, u32 bsize) {struct nfhd_device *dev;int dev_id = id -NFHD_DEV_OFFSET ; +int err = -ENOMEM ;pr_info ("nfhd%u: found device with %u blocks (%u bytes)\n" , dev_id, blocks, bsize);
@@ -130,16 +131,20 @@ static int __init nfhd_init_one(int id, u32 blocks, u32 bsize)sprintf (dev->disk->disk_name,"nfhd%u" , dev_id);set_capacity (dev->disk, (sector_t )blocks * (bsize /512 ));blk_queue_logical_block_size (dev->disk->queue, bsize); -add_disk (dev->disk); + err =add_disk (dev->disk); +if (err) + goto out_cleanup_disk;list_add_tail (&dev->list, &nfhd_list);return 0 ; +out_cleanup_disk: +blk_cleanup_disk (dev->disk); free_dev:kfree (dev); out: -return -ENOMEM ; +return err; }static int __init nfhd_init (void )
Signature · 1 line
2.30.2
PATCH v2 4/15 drbd: add error handling support for add_disk()
In reply to Luis ChamberlainWe never checked for errors on add_disk() as this function returned void. Now that this is fixed, use the shiny new error handling.
--- drivers/block/drbd/drbd_main.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/block/drbd/drbd_main.c +5 -1
@@ -2794,7 +2794,9 @@ enum drbd_ret_code drbd_create_device(struct drbd_config_context *adm_ctx, unsig goto out_idr_remove_vol; } -add_disk (disk); + err =add_disk (disk); +if (err) + goto out_cleanup_disk;/* inherit the connection state */ device->state.conn =first_connection (resource)->cstate;
@@ -2808,6 +2810,8 @@ enum drbd_ret_code drbd_create_device(struct drbd_config_context *adm_ctx, unsigdrbd_debugfs_device_add (device);return NO_ERROR ; +out_cleanup_disk: +blk_cleanup_disk (disk); out_idr_remove_vol:idr_remove (&connection->peer_devices, vnr); out_idr_remove_from_resource:
Signature · 1 line
2.30.2
PATCH v2 5/15 um/drivers/ubd_kern: add error handling support for add_disk()
In reply to Luis ChamberlainWe never checked for errors on add_disk() as this function returned void. Now that this is fixed, use the shiny new error handling.
ubd_disk_register() never returned an error, so just fix that now and let the caller handle the error condition.
--- arch/um/drivers/ubd_kern.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
arch/um/drivers/ubd_kern.c +9 -4
@@ -854,8 +854,8 @@ static const struct attribute_group *ubd_attr_groups[] = { NULL, }; -static void ubd_disk_register (int major,u64 size,int unit, -struct gendisk *disk) +static int ubd_disk_register (int major,u64 size,int unit, +struct gendisk *disk) { disk->major = major; disk->first_minor = unit <<UBD_SHIFT ;
@@ -872,7 +872,7 @@ static void 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); +return device_add_disk (&ubd_devs[unit].pdev.dev, disk, ubd_attr_groups); }#define ROUND_BLOCK (n) ((n + (SECTOR_SIZE - 1)) & (-SECTOR_SIZE))
@@ -919,10 +919,15 @@ static int ubd_add(int n, char **error_out)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 ); -ubd_disk_register (UBD_MAJOR , ubd_dev->size, n, disk); + err =ubd_disk_register (UBD_MAJOR , ubd_dev->size, n, disk); +if (err) + goto out_cleanup_disk; + ubd_gendisk[n] = disk;return 0 ; +out_cleanup_disk: +blk_cleanup_disk (disk); out_cleanup_tags:blk_mq_free_tag_set (&ubd_dev->tag_set); out:
Signature · 1 line
2.30.2
PATCH v2 6/15 xtensa/platforms/iss/simdisk: add error handling support for add_disk()
In reply to Luis ChamberlainWe never checked for errors on add_disk() as this function returned void. Now that this is fixed, use the shiny new error handling.
--- arch/xtensa/platforms/iss/simdisk.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
arch/xtensa/platforms/iss/simdisk.c +11 -2
@@ -259,6 +259,7 @@ static int __init simdisk_setup(struct simdisk *dev, int which, struct proc_dir_entry *procdir) {char tmp[2 ] = {'0' + which,0 }; +int err = -ENOMEM ; dev->fd =-1 ; dev->filename =NULL ;
@@ -267,7 +268,7 @@ static int __init simdisk_setup(struct simdisk *dev, int which, dev->gd =blk_alloc_disk (NUMA_NO_NODE );if (!dev->gd) -return -ENOMEM ; + goto out; dev->gd->major = simdisk_major; dev->gd->first_minor = which; dev->gd->minors =SIMDISK_MINORS ;
@@ -275,10 +276,18 @@ static int __init simdisk_setup(struct simdisk *dev, int which, dev->gd->private_data = dev;snprintf (dev->gd->disk_name,32 ,"simdisk%d" , which);set_capacity (dev->gd,0 ); -add_disk (dev->gd); + err =add_disk (dev->gd); +if (err) + goto out_cleanup_disk; dev->procfile =proc_create_data (tmp,0644 , procdir, &simdisk_proc_ops, dev); +return 0 ; + +out_cleanup_disk: +blk_cleanup_disk (dev->gd); +out: +return err; }static int __init simdisk_init (void )
Signature · 1 line
2.30.2
Re: [PATCH v2 06/15] xtensa/platforms/iss/simdisk: add error handling support for add_disk()
Quoted 9 lines · Luis Chamberlain
We never checked for errors on
add_disk()as this function returned void. Now that this is fixed, use the shiny new error handling.Signed-off-by Luis Chamberlain <[email protected]>--- arch/xtensa/platforms/iss/simdisk.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
Signature · 2 lines
Thanks. -- Max
Re: [PATCH v2 06/15] xtensa/platforms/iss/simdisk: add error handling support for add_disk()
Quoted 6 lines · Max Filippov
Quoted 5 lines · Luis Chamberlain
We never checked for errors on
add_disk()as this function returned void. Now that this is fixed, use the shiny new error handling.
Quoted 6 lines
Quoted 4 lines
Signed-off-by Luis Chamberlain <[email protected]>--- arch/xtensa/platforms/iss/simdisk.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)Acked-by Max Filippov <[email protected]>
Applied, thanks.
Signature · 1 line
Jens Axboe
PATCH v2 7/15 n64cart: add error handling support for add_disk()
In reply to Luis ChamberlainWe never checked for errors on add_disk() as this function returned void. Now that this is fixed, use the shiny new error handling.
--- drivers/block/n64cart.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
drivers/block/n64cart.c +10 -2
@@ -117,6 +117,7 @@ static const struct block_device_operations n64cart_fops = {static int __init n64cart_probe (struct platform_device *pdev) {struct gendisk *disk; +int err = -ENOMEM ;if (!start || !size) {pr_err ("start or size not specified\n" );
@@ -134,7 +135,7 @@ static int __init n64cart_probe(struct platform_device *pdev) disk =blk_alloc_disk (NUMA_NO_NODE );if (!disk) -return -ENOMEM ; + goto out; disk->first_minor =0 ; disk->flags =GENHD_FL_NO_PART_SCAN ;
@@ -149,11 +150,18 @@ static int __init n64cart_probe(struct platform_device *pdev)blk_queue_physical_block_size (disk->queue,4096 );blk_queue_logical_block_size (disk->queue,4096 ); -add_disk (disk); + err =add_disk (disk); +if (err) + goto out_cleanup_disk;pr_info ("n64cart: %u kb disk\n" , size /1024 );return 0 ; + +out_cleanup_disk: +blk_cleanup_disk (disk); +out: +return err; }static struct platform_driver n64cart_driver = {
Signature · 1 line
2.30.2
PATCH v2 8/15 pcd: move the identify buffer into pcd_identify
In reply to Luis ChamberlainFrom: Christoph Hellwig <[email protected]>
No need to pass it through a bunch of functions.
--- drivers/block/paride/pcd.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/block/paride/pcd.c +8 -8
@@ -630,10 +630,11 @@ static int pcd_drive_status(struct cdrom_device_info *cdi, int slot_nr)return CDS_DISC_OK ; } -static int pcd_identify (struct pcd_unit *cd,char *id) +static int pcd_identify (struct pcd_unit *cd) { -int k, s;char id_cmd[12 ] = {0x12 ,0 ,0 ,0 ,36 ,0 ,0 ,0 ,0 ,0 ,0 ,0 }; +char id[18 ]; +int k, s; pcd_bufblk =-1 ;
@@ -664,15 +665,15 @@ static int pcd_identify(struct pcd_unit *cd, char *id)* returns 0, with id set if drive is detected * -1, if drive detection failed */ -static int pcd_probe (struct pcd_unit *cd,int ms,char *id) +static int pcd_probe (struct pcd_unit *cd,int ms) {if (ms ==-1 ) {for (cd->drive =0 ; cd->drive <=1 ; cd->drive++) -if (!pcd_reset (cd) && !pcd_identify (cd, id)) +if (!pcd_reset (cd) && !pcd_identify (cd))return 0 ; }else { cd->drive = ms; -if (!pcd_reset (cd) && !pcd_identify (cd, id)) +if (!pcd_reset (cd) && !pcd_identify (cd))return 0 ; }return -1 ;
@@ -709,7 +710,6 @@ static void pcd_probe_capabilities(void)static int pcd_detect (void ) { -char id[18 ];int k, unit;struct pcd_unit *cd;
@@ -727,7 +727,7 @@ static int pcd_detect(void) cd = pcd;if (cd->disk &&pi_init (cd->pi,1 ,-1 ,-1 ,-1 ,-1 ,-1 , pcd_buffer,PI_PCD , verbose, cd->name)) { -if (!pcd_probe (cd,-1 , id)) { +if (!pcd_probe (cd,-1 )) { cd->present =1 ; k++; }else
@@ -744,7 +744,7 @@ static int pcd_detect(void) conf[D_UNI ], conf[D_PRO ], conf[D_DLY ], pcd_buffer, PI_PCD, verbose, cd->name)) continue; -if (!pcd_probe (cd, conf[D_SLV ], id)) { +if (!pcd_probe (cd, conf[D_SLV ])) { cd->present =1 ; k++; }else
Signature · 1 line
2.30.2
PATCH v2 9/15 pcd: cleanup initialization
In reply to Luis ChamberlainFrom: Christoph Hellwig <[email protected]>
Refactor the pcd initialization to have a dedicated helper to initialize a single disk.
--- drivers/block/paride/pcd.c | 286 ++++++++++++++++--------------------- 1 file changed, 127 insertions(+), 159 deletions(-)
drivers/block/paride/pcd.c +127 -159
@@ -183,8 +183,6 @@ static int pcd_audio_ioctl(struct cdrom_device_info *cdi,static int pcd_packet (struct cdrom_device_info *cdi,struct packet_command *cgc); -static int pcd_detect (void ); -static void pcd_probe_capabilities (void );static void do_pcd_read_drq (void );static blk_status_t pcd_queue_rq (struct blk_mq_hw_ctx *hctx,const struct blk_mq_queue_data *bd);
@@ -302,53 +300,6 @@ static const struct blk_mq_ops pcd_mq_ops = { .queue_rq = pcd_queue_rq, }; -static void pcd_init_units (void ) -{ -struct pcd_unit *cd; -int unit; - - pcd_drive_count =0 ; -for (unit =0 , cd = pcd; unit <PCD_UNITS ; unit++, cd++) { -struct gendisk *disk; - -if (blk_mq_alloc_sq_tag_set (&cd->tag_set, &pcd_mq_ops,1 , -BLK_MQ_F_SHOULD_MERGE )) -continue ; - - disk =blk_mq_alloc_disk (&cd->tag_set, cd); -if (IS_ERR (disk)) { -blk_mq_free_tag_set (&cd->tag_set); -continue ; - } - -INIT_LIST_HEAD (&cd->rq_list); -blk_queue_bounce_limit (disk->queue,BLK_BOUNCE_HIGH ); - cd->disk = disk; - cd->pi = &cd->pia; - cd->present =0 ; - cd->last_sense =0 ; - cd->changed =1 ; - cd->drive = (*drives[unit])[D_SLV ]; -if ((*drives[unit])[D_PRT ]) - pcd_drive_count++; - - cd->name = &cd->info.name[0 ]; -snprintf (cd->name,sizeof (cd->info.name),"%s%d" , name, unit); - cd->info.ops = &pcd_dops; - cd->info.handle = cd; - cd->info.speed =0 ; - cd->info.capacity =1 ; - cd->info.mask =0 ; - disk->major = major; - disk->first_minor = unit; - disk->minors =1 ; -strcpy (disk->disk_name, cd->name);/* umm... */ - disk->fops = &pcd_bdops; - disk->flags =GENHD_FL_BLOCK_EVENTS_ON_EXCL_WRITE ; - disk->events =DISK_EVENT_MEDIA_CHANGE ; - } -} -static int pcd_open (struct cdrom_device_info *cdi,int purpose) {struct pcd_unit *cd = cdi->handle;
@@ -679,90 +630,31 @@ static int pcd_probe(struct pcd_unit *cd, int ms)return -1 ; } -static void pcd_probe_capabilities (void ) +static int pcd_probe_capabilities (struct pcd_unit *cd) { -int unit, r; -char buffer[32 ];char cmd[12 ] = {0x5a ,1 <<3 ,0x2a ,0 ,0 ,0 ,0 ,18 ,0 ,0 ,0 ,0 }; -struct pcd_unit *cd; - -for (unit =0 , cd = pcd; unit <PCD_UNITS ; unit++, cd++) { -if (!cd->present) -continue ; - r =pcd_atapi (cd, cmd,18 , buffer,"mode sense capabilities" ); -if (r) -continue ; -/* we should now have the cap page */ -if ((buffer[11 ] &1 ) ==0 ) - cd->info.mask |=CDC_CD_R ; -if ((buffer[11 ] &2 ) ==0 ) - cd->info.mask |=CDC_CD_RW ; -if ((buffer[12 ] &1 ) ==0 ) - cd->info.mask |=CDC_PLAY_AUDIO ; -if ((buffer[14 ] &1 ) ==0 ) - cd->info.mask |=CDC_LOCK ; -if ((buffer[14 ] &8 ) ==0 ) - cd->info.mask |=CDC_OPEN_TRAY ; -if ((buffer[14 ] >>6 ) ==0 ) - cd->info.mask |=CDC_CLOSE_TRAY ; - } -} - -static int pcd_detect (void ) -{ -int k, unit; -struct pcd_unit *cd; - -printk ("%s: %s version %s, major %d, nice %d\n" , - name, name,PCD_VERSION , major, nice); +char buffer[32 ]; +int ret; - par_drv =pi_register_driver (name); -if (!par_drv) { -pr_err ("failed to register %s driver\n" , name); -return -1 ; - } + ret =pcd_atapi (cd, cmd,18 , buffer,"mode sense capabilities" ); +if (ret) +return ret; + +/* we should now have the cap page */ +if ((buffer[11 ] &1 ) ==0 ) + cd->info.mask |=CDC_CD_R ; +if ((buffer[11 ] &2 ) ==0 ) + cd->info.mask |=CDC_CD_RW ; +if ((buffer[12 ] &1 ) ==0 ) + cd->info.mask |=CDC_PLAY_AUDIO ; +if ((buffer[14 ] &1 ) ==0 ) + cd->info.mask |=CDC_LOCK ; +if ((buffer[14 ] &8 ) ==0 ) + cd->info.mask |=CDC_OPEN_TRAY ; +if ((buffer[14 ] >>6 ) ==0 ) + cd->info.mask |=CDC_CLOSE_TRAY ; - k =0 ; -if (pcd_drive_count ==0 ) {/* nothing spec'd - so autoprobe for 1 */ - cd = pcd; -if (cd->disk &&pi_init (cd->pi,1 ,-1 ,-1 ,-1 ,-1 ,-1 , - pcd_buffer,PI_PCD , verbose, cd->name)) { -if (!pcd_probe (cd,-1 )) { - cd->present =1 ; - k++; - }else -pi_release (cd->pi); - } - }else { -for (unit =0 , cd = pcd; unit <PCD_UNITS ; unit++, cd++) { -int *conf = *drives[unit]; -if (!conf[D_PRT ]) -continue ; -if (!cd->disk) -continue ; -if (!pi_init (cd->pi,0 , conf[D_PRT ], conf[D_MOD ], - conf[D_UNI ], conf[D_PRO ], conf[D_DLY ], - pcd_buffer,PI_PCD , verbose, cd->name)) -continue ; -if (!pcd_probe (cd, conf[D_SLV ])) { - cd->present =1 ; - k++; - }else -pi_release (cd->pi); - } - } -if (k) -return 0 ; - -printk ("%s: No CD-ROM drive found\n" , name); -for (unit =0 , cd = pcd; unit <PCD_UNITS ; unit++, cd++) { -if (!cd->disk) -continue ; -blk_cleanup_disk (cd->disk); -blk_mq_free_tag_set (&cd->tag_set); - } -pi_unregister_driver (par_drv); -return -1 ; +return 0 ; }/* I/O request processing */
@@ -999,43 +891,121 @@ static int pcd_get_mcn(struct cdrom_device_info *cdi, struct cdrom_mcn *mcn)return 0 ; } +static int pcd_init_unit (struct pcd_unit *cd,bool autoprobe,int port, +int mode,int unit,int protocol,int delay,int ms) +{ +struct gendisk *disk; +int ret; + + ret =blk_mq_alloc_sq_tag_set (&cd->tag_set, &pcd_mq_ops,1 , +BLK_MQ_F_SHOULD_MERGE ); +if (ret) +return ret; + + disk =blk_mq_alloc_disk (&cd->tag_set, cd); +if (IS_ERR (disk)) { + ret =PTR_ERR (disk); + goto out_free_tag_set; + } + +INIT_LIST_HEAD (&cd->rq_list); +blk_queue_bounce_limit (disk->queue,BLK_BOUNCE_HIGH ); + cd->disk = disk; + cd->pi = &cd->pia; + cd->present =0 ; + cd->last_sense =0 ; + cd->changed =1 ; + cd->drive = (*drives[cd - pcd])[D_SLV ]; + + cd->name = &cd->info.name[0 ]; +snprintf (cd->name,sizeof (cd->info.name),"%s%d" , name, unit); + cd->info.ops = &pcd_dops; + cd->info.handle = cd; + cd->info.speed =0 ; + cd->info.capacity =1 ; + cd->info.mask =0 ; + disk->major = major; + disk->first_minor = unit; + disk->minors =1 ; +strcpy (disk->disk_name, cd->name);/* umm... */ + disk->fops = &pcd_bdops; + disk->flags =GENHD_FL_BLOCK_EVENTS_ON_EXCL_WRITE ; + disk->events =DISK_EVENT_MEDIA_CHANGE ; + +if (!pi_init (cd->pi, autoprobe, port, mode, unit, protocol, delay, + pcd_buffer,PI_PCD , verbose, cd->name)) + goto out_free_disk; +if (pcd_probe (cd, ms)) + goto out_pi_release; + + cd->present =1 ; +pcd_probe_capabilities (cd); +register_cdrom (cd->disk, &cd->info); +add_disk (cd->disk); +return 0 ; + +out_pi_release: +pi_release (cd->pi); +out_free_disk: +blk_cleanup_disk (cd->disk); +out_free_tag_set: +blk_mq_free_tag_set (&cd->tag_set); +return ret; +} +static int __init pcd_init (void ) { -struct pcd_unit *cd; -int unit; +int found =0 , unit;if (disable)return -EINVAL ; -pcd_init_units (); +if (register_blkdev (major, name)) +return -EBUSY ; -if (pcd_detect ()) -return -ENODEV ; +pr_info ("%s: %s version %s, major %d, nice %d\n" , + name, name,PCD_VERSION , major, nice); -/* get the atapi capabilities page */ -pcd_probe_capabilities (); + par_drv =pi_register_driver (name); +if (!par_drv) { +pr_err ("failed to register %s driver\n" , name); + goto out_unregister_blkdev; + } -if (register_blkdev (major, name)) { -for (unit =0 , cd = pcd; unit <PCD_UNITS ; unit++, cd++) { -if (!cd->disk) -continue ; +for (unit =0 ; unit <PCD_UNITS ; unit++) { +if ((*drives[unit])[D_PRT ]) + pcd_drive_count++; + } -blk_cleanup_queue (cd->disk->queue); -blk_mq_free_tag_set (&cd->tag_set); -put_disk (cd->disk); +if (pcd_drive_count ==0 ) {/* nothing spec'd - so autoprobe for 1 */ +if (!pcd_init_unit (pcd,1 ,-1 ,-1 ,-1 ,-1 ,-1 ,-1 )) + found++; + }else { +for (unit =0 ; unit <PCD_UNITS ; unit++) { +struct pcd_unit *cd = &pcd[unit]; +int *conf = *drives[unit]; + +if (!conf[D_PRT ]) +continue ; +if (!pcd_init_unit (cd,0 , conf[D_PRT ], conf[D_MOD ], + conf[D_UNI ], conf[D_PRO ], conf[D_DLY ], + conf[D_SLV ])) + found++; } -return -EBUSY ; } -for (unit =0 , cd = pcd; unit <PCD_UNITS ; unit++, cd++) { -if (cd->present) { -register_cdrom (cd->disk, &cd->info); - cd->disk->private_data = cd; -add_disk (cd->disk); - } +if (!found) { +pr_info ("%s: No CD-ROM drive found\n" , name); + goto out_unregister_pi_driver; }return 0 ; + +out_unregister_pi_driver: +pi_unregister_driver (par_drv); +out_unregister_blkdev: +unregister_blkdev (major, name); +return -ENODEV ; } static void __exit pcd_exit(void)
@@ -1044,20 +1014,18 @@ static void __exit pcd_exit(void)int unit;for (unit =0 , cd = pcd; unit <PCD_UNITS ; unit++, cd++) { -if (!cd->disk) +if (!cd->present)continue ; -if (cd->present) { -del_gendisk (cd->disk); -pi_release (cd->pi); -unregister_cdrom (&cd->info); - } -blk_cleanup_queue (cd->disk->queue); +del_gendisk (cd->disk); +pi_release (cd->pi); +unregister_cdrom (&cd->info); +blk_cleanup_disk (cd->disk); +blk_mq_free_tag_set (&cd->tag_set); -put_disk (cd->disk); } -unregister_blkdev (major, name);pi_unregister_driver (par_drv); +unregister_blkdev (major, name); }MODULE_LICENSE ("GPL" );
Signature · 1 line
2.30.2
PATCH v2 10/15 pf: cleanup initialization
In reply to Luis ChamberlainFrom: Christoph Hellwig <[email protected]>
Refactor the pf initialization to have a dedicated helper to initialize a single disk.
--- drivers/block/paride/pf.c | 223 +++++++++++++++++--------------------- 1 file changed, 99 insertions(+), 124 deletions(-)
drivers/block/paride/pf.c +99 -124
@@ -214,7 +214,6 @@ static int pf_getgeo(struct block_device *bdev, struct hd_geometry *geo);static void pf_release (struct gendisk *disk,fmode_t mode); -static int pf_detect (void );static void do_pf_read (void );static void do_pf_read_start (void );static void do_pf_write (void );
@@ -285,45 +284,6 @@ static const struct blk_mq_ops pf_mq_ops = { .queue_rq = pf_queue_rq, }; -static void __init pf_init_units (void ) -{ -struct pf_unit *pf; -int unit; - - pf_drive_count =0 ; -for (unit =0 , pf = units; unit <PF_UNITS ; unit++, pf++) { -struct gendisk *disk; - -if (blk_mq_alloc_sq_tag_set (&pf->tag_set, &pf_mq_ops,1 , -BLK_MQ_F_SHOULD_MERGE )) -continue ; - - disk =blk_mq_alloc_disk (&pf->tag_set, pf); -if (IS_ERR (disk)) { -blk_mq_free_tag_set (&pf->tag_set); -continue ; - } - -INIT_LIST_HEAD (&pf->rq_list); -blk_queue_max_segments (disk->queue, cluster); -blk_queue_bounce_limit (disk->queue,BLK_BOUNCE_HIGH ); - pf->disk = disk; - pf->pi = &pf->pia; - pf->media_status =PF_NM ; - pf->drive = (*drives[unit])[D_SLV ]; - pf->lun = (*drives[unit])[D_LUN ]; -snprintf (pf->name,PF_NAMELEN ,"%s%d" , name, unit); - disk->major = major; - disk->first_minor = unit; - disk->minors =1 ; -strcpy (disk->disk_name, pf->name); - disk->fops = &pf_fops; - disk->events =DISK_EVENT_MEDIA_CHANGE ; -if (!(*drives[unit])[D_PRT ]) - pf_drive_count++; - } -} -static int pf_open (struct block_device *bdev,fmode_t mode) {struct pf_unit *pf = bdev->bd_disk->private_data;
@@ -718,59 +678,6 @@ static int pf_probe(struct pf_unit *pf)return -1 ; } -static int pf_detect (void ) -{ -struct pf_unit *pf = units; -int k, unit; - -printk ("%s: %s version %s, major %d, cluster %d, nice %d\n" , - name, name,PF_VERSION , major, cluster, nice); - - par_drv =pi_register_driver (name); -if (!par_drv) { -pr_err ("failed to register %s driver\n" , name); -return -1 ; - } - k =0 ; -if (pf_drive_count ==0 ) { -if (pi_init (pf->pi,1 ,-1 ,-1 ,-1 ,-1 ,-1 , pf_scratch,PI_PF , - verbose, pf->name)) { -if (!pf_probe (pf) && pf->disk) { - pf->present =1 ; - k++; - }else -pi_release (pf->pi); - } - - }else -for (unit =0 ; unit <PF_UNITS ; unit++, pf++) { -int *conf = *drives[unit]; -if (!conf[D_PRT ]) -continue ; -if (pi_init (pf->pi,0 , conf[D_PRT ], conf[D_MOD ], - conf[D_UNI ], conf[D_PRO ], conf[D_DLY ], - pf_scratch,PI_PF , verbose, pf->name)) { -if (pf->disk && !pf_probe (pf)) { - pf->present =1 ; - k++; - }else -pi_release (pf->pi); - } - } -if (k) -return 0 ; - -printk ("%s: No ATAPI disk detected\n" , name); -for (pf = units, unit =0 ; unit <PF_UNITS ; pf++, unit++) { -if (!pf->disk) -continue ; -blk_cleanup_disk (pf->disk); -blk_mq_free_tag_set (&pf->tag_set); - } -pi_unregister_driver (par_drv); -return -1 ; -} -/* The i/o request engine */ static int pf_start (struct pf_unit *pf,int cmd,int b,int c)
@@ -1014,61 +921,129 @@ static void do_pf_write_done(void)next_request (0 ); } +static int __init pf_init_unit (struct pf_unit *pf,bool autoprobe,int port, +int mode,int unit,int protocol,int delay,int ms) +{ +struct gendisk *disk; +int ret; + + ret =blk_mq_alloc_sq_tag_set (&pf->tag_set, &pf_mq_ops,1 , +BLK_MQ_F_SHOULD_MERGE ); +if (ret) +return ret; + + disk =blk_mq_alloc_disk (&pf->tag_set, pf); +if (IS_ERR (disk)) { + ret =PTR_ERR (disk); + goto out_free_tag_set; + } + disk->major = major; + disk->first_minor = pf - units; + disk->minors =1 ; +strcpy (disk->disk_name, pf->name); + disk->fops = &pf_fops; + disk->events =DISK_EVENT_MEDIA_CHANGE ; + disk->private_data = pf; + +blk_queue_max_segments (disk->queue, cluster); +blk_queue_bounce_limit (disk->queue,BLK_BOUNCE_HIGH ); + +INIT_LIST_HEAD (&pf->rq_list); + pf->disk = disk; + pf->pi = &pf->pia; + pf->media_status =PF_NM ; + pf->drive = (*drives[disk->first_minor])[D_SLV ]; + pf->lun = (*drives[disk->first_minor])[D_LUN ]; +snprintf (pf->name,PF_NAMELEN ,"%s%d" , name, disk->first_minor); + +if (!pi_init (pf->pi, autoprobe, port, mode, unit, protocol, delay, + pf_scratch,PI_PF , verbose, pf->name)) + goto out_free_disk; +if (pf_probe (pf)) + goto out_pi_release; + +add_disk (disk); + pf->present =1 ; +return 0 ; + +out_pi_release: +pi_release (pf->pi); +out_free_disk: +blk_cleanup_disk (pf->disk); +out_free_tag_set: +blk_mq_free_tag_set (&pf->tag_set); +return ret; +} +static int __init pf_init (void ) {/* preliminary initialisation */ struct pf_unit *pf; -int unit; +int found =0 , unit;if (disable)return -EINVAL ; -pf_init_units (); +if (register_blkdev (major, name)) +return -EBUSY ; -if (pf_detect ()) -return -ENODEV ; - pf_busy =0 ; +printk ("%s: %s version %s, major %d, cluster %d, nice %d\n" , + name, name,PF_VERSION , major, cluster, nice); -if (register_blkdev (major, name)) { -for (pf = units, unit =0 ; unit <PF_UNITS ; pf++, unit++) { -if (!pf->disk) -continue ; -blk_cleanup_queue (pf->disk->queue); -blk_mq_free_tag_set (&pf->tag_set); -put_disk (pf->disk); - } -return -EBUSY ; + par_drv =pi_register_driver (name); +if (!par_drv) { +pr_err ("failed to register %s driver\n" , name); + goto out_unregister_blkdev; } -for (pf = units, unit =0 ; unit <PF_UNITS ; pf++, unit++) { -struct gendisk *disk = pf->disk; +for (unit =0 ; unit <PF_UNITS ; unit++) { +if (!(*drives[unit])[D_PRT ]) + pf_drive_count++; + } -if (!pf->present) -continue ; - disk->private_data = pf; -add_disk (disk); + pf = units; +if (pf_drive_count ==0 ) { +if (pf_init_unit (pf,1 ,-1 ,-1 ,-1 ,-1 ,-1 , verbose)) + found++; + }else { +for (unit =0 ; unit <PF_UNITS ; unit++, pf++) { +int *conf = *drives[unit]; +if (!conf[D_PRT ]) +continue ; +if (pf_init_unit (pf,0 , conf[D_PRT ], conf[D_MOD ], + conf[D_UNI ], conf[D_PRO ], conf[D_DLY ], + verbose)) + found++; + } + } +if (!found) { +printk ("%s: No ATAPI disk detected\n" , name); + goto out_unregister_pi_driver; } + pf_busy =0 ;return 0 ; + +out_unregister_pi_driver: +pi_unregister_driver (par_drv); +out_unregister_blkdev: +unregister_blkdev (major, name); +return -ENODEV ; } static void __exit pf_exit(void) {struct pf_unit *pf;int unit; -unregister_blkdev (major, name); +for (pf = units, unit =0 ; unit <PF_UNITS ; pf++, unit++) { -if (!pf->disk) +if (!pf->present)continue ; - -if (pf->present) -del_gendisk (pf->disk); - -blk_cleanup_queue (pf->disk->queue); +del_gendisk (pf->disk); +blk_cleanup_disk (pf->disk);blk_mq_free_tag_set (&pf->tag_set); -put_disk (pf->disk); - -if (pf->present) -pi_release (pf->pi); +pi_release (pf->pi); } + +unregister_blkdev (major, name); }MODULE_LICENSE ("GPL" );
Signature · 1 line
2.30.2
PATCH v2 11/15 pd: cleanup initialization
In reply to Luis ChamberlainFrom: Christoph Hellwig <[email protected]>
Refactor the pf initialization to have a dedicated helper to initialize a single disk.
--- drivers/block/paride/pd.c | 140 +++++++++++++++++++------------------- 1 file changed, 70 insertions(+), 70 deletions(-)
drivers/block/paride/pd.c +70 -70
@@ -875,9 +875,27 @@ static const struct blk_mq_ops pd_mq_ops = { .queue_rq = pd_queue_rq, }; -static void pd_probe_drive (struct pd_unit *disk) +static int pd_probe_drive (struct pd_unit *disk,int autoprobe,int port, +int mode,int unit,int protocol,int delay) { +int index = disk - pd; +int *parm = *drives[index];struct gendisk *p; +int ret; + + disk->pi = &disk->pia; + disk->access =0 ; + disk->changed =1 ; + disk->capacity =0 ; + disk->drive = parm[D_SLV ]; +snprintf (disk->name,PD_NAMELEN ,"%s%c" , name,'a' + index); + disk->alt_geom = parm[D_GEO ]; + disk->standby = parm[D_SBY ]; +INIT_LIST_HEAD (&disk->rq_list); + +if (!pi_init (disk->pi, autoprobe, port, mode, unit, protocol, delay, + pd_scratch,PI_PD , verbose, disk->name)) +return -ENXIO ;memset (&disk->tag_set,0 ,sizeof (disk->tag_set)); disk->tag_set.ops = &pd_mq_ops;
@@ -887,14 +905,14 @@ static void pd_probe_drive(struct pd_unit *disk) disk->tag_set.queue_depth =2 ; disk->tag_set.numa_node =NUMA_NO_NODE ; disk->tag_set.flags =BLK_MQ_F_SHOULD_MERGE |BLK_MQ_F_BLOCKING ; - -if (blk_mq_alloc_tag_set (&disk->tag_set)) -return ; + ret =blk_mq_alloc_tag_set (&disk->tag_set); +if (ret) + goto pi_release; p =blk_mq_alloc_disk (&disk->tag_set, disk);if (IS_ERR (p)) { -blk_mq_free_tag_set (&disk->tag_set); -return ; + ret =PTR_ERR (p); + goto free_tag_set; } disk->gd = p;
@@ -905,102 +923,84 @@ static void pd_probe_drive(struct pd_unit *disk) p->minors =1 <<PD_BITS ; p->events =DISK_EVENT_MEDIA_CHANGE ; p->private_data = disk; -blk_queue_max_hw_sectors (p->queue, cluster);blk_queue_bounce_limit (p->queue,BLK_BOUNCE_HIGH );if (disk->drive ==-1 ) { -for (disk->drive =0 ; disk->drive <=1 ; disk->drive++) -if (pd_special_command (disk, pd_identify) ==0 ) -return ; - }else if (pd_special_command (disk, pd_identify) ==0 ) -return ; - disk->gd =NULL ; +for (disk->drive =0 ; disk->drive <=1 ; disk->drive++) { + ret =pd_special_command (disk, pd_identify); +if (ret ==0 ) +break ; + } + }else { + ret =pd_special_command (disk, pd_identify); + } +if (ret) + goto put_disk; +set_capacity (disk->gd, disk->capacity); +add_disk (disk->gd); +return 0 ; +put_disk:put_disk (p); + disk->gd =NULL ; +free_tag_set: +blk_mq_free_tag_set (&disk->tag_set); +pi_release: +pi_release (disk->pi); +return ret; } -static int pd_detect (void ) +static int __init pd_init (void ) {int found =0 , unit, pd_drive_count =0 ;struct pd_unit *disk; -for (unit =0 ; unit <PD_UNITS ; unit++) { -int *parm = *drives[unit]; -struct pd_unit *disk = pd + unit; - disk->pi = &disk->pia; - disk->access =0 ; - disk->changed =1 ; - disk->capacity =0 ; - disk->drive = parm[D_SLV ]; -snprintf (disk->name,PD_NAMELEN ,"%s%c" , name,'a' +unit); - disk->alt_geom = parm[D_GEO ]; - disk->standby = parm[D_SBY ]; -if (parm[D_PRT ]) - pd_drive_count++; -INIT_LIST_HEAD (&disk->rq_list); - } +if (disable) +return -ENODEV ; + +if (register_blkdev (major, name)) +return -ENODEV ; + +printk ("%s: %s version %s, major %d, cluster %d, nice %d\n" , + name, name,PD_VERSION , major, cluster, nice); par_drv =pi_register_driver (name);if (!par_drv) {pr_err ("failed to register %s driver\n" , name); -return -1 ; + goto out_unregister_blkdev; } -if (pd_drive_count ==0 ) {/* nothing spec'd - so autoprobe for 1 */ - disk = pd; -if (pi_init (disk->pi,1 ,-1 ,-1 ,-1 ,-1 ,-1 , pd_scratch, -PI_PD , verbose, disk->name)) { -pd_probe_drive (disk); -if (!disk->gd) -pi_release (disk->pi); - } +for (unit =0 ; unit <PD_UNITS ; unit++) { +int *parm = *drives[unit]; +if (parm[D_PRT ]) + pd_drive_count++; + } + +if (pd_drive_count ==0 ) {/* nothing spec'd - so autoprobe for 1 */ +if (!pd_probe_drive (pd,1 ,-1 ,-1 ,-1 ,-1 ,-1 )) + found++; }else {for (unit =0 , disk = pd; unit <PD_UNITS ; unit++, disk++) {int *parm = *drives[unit];if (!parm[D_PRT ])continue ; -if (pi_init (disk->pi,0 , parm[D_PRT ], parm[D_MOD ], - parm[D_UNI ], parm[D_PRO ], parm[D_DLY ], - pd_scratch,PI_PD , verbose, disk->name)) { -pd_probe_drive (disk); -if (!disk->gd) -pi_release (disk->pi); - } - } - } -for (unit =0 , disk = pd; unit <PD_UNITS ; unit++, disk++) { -if (disk->gd) { -set_capacity (disk->gd, disk->capacity); -add_disk (disk->gd); - found =1 ; +if (!pd_probe_drive (disk,0 , parm[D_PRT ], parm[D_MOD ], + parm[D_UNI ], parm[D_PRO ], parm[D_DLY ])) + found++; } }if (!found) {printk ("%s: no valid drive found\n" , name); -pi_unregister_driver (par_drv); + goto out_pi_unregister_driver; } -return found; -} - -static int __init pd_init (void ) -{ -if (disable) - goto out1; - -if (register_blkdev (major, name)) - goto out1; - -printk ("%s: %s version %s, major %d, cluster %d, nice %d\n" , - name, name,PD_VERSION , major, cluster, nice); -if (!pd_detect ()) - goto out2;return 0 ; -out2: +out_pi_unregister_driver: +pi_unregister_driver (par_drv); +out_unregister_blkdev:unregister_blkdev (major, name); -out1:return -ENODEV ; }
Signature · 1 line
2.30.2
PATCH v2 12/15 pcd: add error handling support for add_disk()
In reply to Luis ChamberlainWe never checked for errors on add_disk() as this function returned void. Now that this is fixed, use the shiny new error handling.
--- drivers/block/paride/pcd.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/block/paride/pcd.c +5 -1
@@ -941,9 +941,13 @@ static int pcd_init_unit(struct pcd_unit *cd, bool autoprobe, int port, cd->present =1 ;pcd_probe_capabilities (cd);register_cdrom (cd->disk, &cd->info); -add_disk (cd->disk); + ret =add_disk (cd->disk); +if (ret) + goto out_unreg_cdrom;return 0 ; +out_unreg_cdrom: +unregister_cdrom (&cd->info); out_pi_release:pi_release (cd->pi); out_free_disk:
Signature · 1 line
2.30.2
PATCH v2 13/15 pcd: fix ordering of unregister_cdrom()
In reply to Luis ChamberlainWe first register cdrom and then we add_disk() and so we we should likewise unregister the cdrom first and then del_gendisk().
--- drivers/block/paride/pcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/block/paride/pcd.c +1 -1
@@ -1021,9 +1021,9 @@ static void __exit pcd_exit(void)if (!cd->present)continue ; +unregister_cdrom (&cd->info);del_gendisk (cd->disk);pi_release (cd->pi); -unregister_cdrom (&cd->info);blk_cleanup_disk (cd->disk);blk_mq_free_tag_set (&cd->tag_set);
Signature · 1 line
2.30.2
PATCH v2 14/15 pcd: capture errors on cdrom_register()
In reply to Luis ChamberlainNo errors were being captured wehen cdrom_register() fails, capture the error and return the error.
--- drivers/block/paride/pcd.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/block/paride/pcd.c +3 -1
@@ -940,7 +940,9 @@ static int pcd_init_unit(struct pcd_unit *cd, bool autoprobe, int port, cd->present =1 ;pcd_probe_capabilities (cd); -register_cdrom (cd->disk, &cd->info); + ret =register_cdrom (cd->disk, &cd->info); +if (ret) + goto out_pi_release; ret =add_disk (cd->disk);if (ret) goto out_unreg_cdrom;
Signature · 1 line
2.30.2
PATCH v2 15/15 pd: add error handling support for add_disk()
In reply to Luis ChamberlainWe never checked for errors on add_disk() as this function returned void. Now that this is fixed, use the shiny new error handling.
--- drivers/block/paride/pd.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/block/paride/pd.c +5 -1
@@ -938,8 +938,12 @@ static int pd_probe_drive(struct pd_unit *disk, int autoprobe, int port,if (ret) goto put_disk;set_capacity (disk->gd, disk->capacity); -add_disk (disk->gd); + ret =add_disk (disk->gd); +if (ret) + goto cleanup_disk;return 0 ; +cleanup_disk: +blk_cleanup_disk (disk->gd); put_disk:put_disk (p); disk->gd =NULL ;
Signature · 1 line
2.30.2
Jens Axboe
maintainerQuoted 4 lines · Luis Chamberlain
This is the 2nd version of the third batch of driver conversions for the
add_disk()error handling. This and the entire 7th series of driver conversions can be found on my 20210927-for-axboe-add-disk-error-handling branch [0].
Applied 2, 4, 7, 8-15.
Signature · 1 line
Jens Axboe