[RFC 00/12] block: *add_disk*() driver conversions __register_blkdev()

[RFC 00/12] block: *add_disk*() driver conversions __register_blkdev()

[Luis Chamberlain]

This is the 4th group of driver conversion examples. This set is
complete, as there are only a few drivers which use __register_blkdev().
The __register_blkdev() call uses an optional probe call, and if set
will be used when blk_request_module() is called for the block device.
Since these probe calls can fail, now that we added *add_disk*() error
handling, take advatage of this and extend the probe call to capture
the errors.

There are only a few drivers which benefit from this. The meat of the
work is the last patch, which includes all driver conversions. The rest
of the patches are adding add_disk() error handling in the other
places of the drivers.

Luis Chamberlain (12):
  floppy: fix add_disk() assumption on exit
  floppy: use blk_cleanup_disk()
  floppy: add error handling support for add_disk()
  scsi/sd: use blk_cleanup_queue() insted of put_disk()
  scsi/sd: add error handling support for add_disk()
  scsi/sr: use blk_cleanup_disk() instead of put_disk()
  scsi/sr: add error handling support for add_disk()
  block/ataflop: use the blk_cleanup_disk() helper
  block/ataflop: add a helper for removing disks
  block/ataflop add error handling support for add_disk()
  block/brd: add error handling support for add_disk()
  block: make probe in blk_request_module() return an error

 block/genhd.c           | 15 ++++++----
 drivers/block/ataflop.c | 66 ++++++++++++++++++++++++-----------------
 drivers/block/brd.c     | 17 ++++++++---
 drivers/block/floppy.c  | 48 ++++++++++++++----------------
 drivers/block/loop.c    |  6 ++--
 drivers/scsi/sd.c       | 11 +++++--
 drivers/scsi/sr.c       |  7 +++--
 fs/block_dev.c          |  5 +++-
 include/linux/genhd.h   |  4 +--
 9 files changed, 105 insertions(+), 74 deletions(-)

-- 
2.27.0

[RFC 01/12] floppy: fix add_disk() assumption on exit

[Luis Chamberlain]

Even if the queue was cleaned up on exit,
putting the disk *is* still required, and
likewise, blk_cleanup_queue() on a null queue
should be a no-op now, after the patch titled
"block: skip queue if NULL on blk_cleanup_queue()"
so it is safe to use even if the queue is NULL.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/block/floppy.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 87460e0e5c72..b68f4b6bf737 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4953,19 +4953,6 @@ static void __exit floppy_module_exit(void)
 				blk_cleanup_queue(disks[drive][i]->queue);
 		}
 		blk_mq_free_tag_set(&tag_sets[drive]);
-
-		/*
-		 * These disks have not called add_disk().  Don't put down
-		 * queue reference in put_disk().
-		 */
-		if (!(allowed_drive_mask & (1 << drive)) ||
-		    fdc_state[FDC(drive)].version == FDC_NONE) {
-			for (i = 0; i < ARRAY_SIZE(floppy_type); i++) {
-				if (disks[drive][i])
-					disks[drive][i]->queue = NULL;
-			}
-		}
-
 		for (i = 0; i < ARRAY_SIZE(floppy_type); i++) {
 			if (disks[drive][i])
 				put_disk(disks[drive][i]);
-- 
2.27.0

[RFC 02/12] floppy: use blk_cleanup_disk()

[Luis Chamberlain]

Use the blk_cleanup_queue() followed by put_disk() can be
replaced with blk_cleanup_disk(). No need for two separate
loops.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/block/floppy.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index b68f4b6bf737..ba690affc751 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4950,13 +4950,9 @@ static void __exit floppy_module_exit(void)
 		}
 		for (i = 0; i < ARRAY_SIZE(floppy_type); i++) {
 			if (disks[drive][i])
-				blk_cleanup_queue(disks[drive][i]->queue);
+				blk_cleanup_disk(disks[drive][i]);
 		}
 		blk_mq_free_tag_set(&tag_sets[drive]);
-		for (i = 0; i < ARRAY_SIZE(floppy_type); i++) {
-			if (disks[drive][i])
-				put_disk(disks[drive][i]);
-		}
 	}
 
 	cancel_delayed_work_sync(&fd_timeout);
-- 
2.27.0

[RFC 03/12] floppy: add error handling support for add_disk()

[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.

We take advantage of the new ability of del_gendisk() ...

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/block/floppy.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index ba690affc751..cad17b49e700 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4478,6 +4478,7 @@ static const struct blk_mq_ops floppy_mq_ops = {
 };
 
 static struct platform_device floppy_device[N_DRIVE];
+static bool registered[N_DRIVE];
 
 static bool floppy_available(int drive)
 {
@@ -4693,8 +4694,12 @@ static int __init do_floppy_init(void)
 		if (err)
 			goto out_remove_drives;
 
-		device_add_disk(&floppy_device[drive].dev, disks[drive][0],
-				NULL);
+		registered[drive] = true;
+
+		err = device_add_disk(&floppy_device[drive].dev,
+				      disks[drive][0], NULL);
+		if (err)
+			goto out_remove_drives;
 	}
 
 	return 0;
@@ -4703,7 +4708,8 @@ static int __init do_floppy_init(void)
 	while (drive--) {
 		if (floppy_available(drive)) {
 			del_gendisk(disks[drive][0]);
-			platform_device_unregister(&floppy_device[drive]);
+			if (registered[drive])
+				platform_device_unregister(&floppy_device[drive]);
 		}
 	}
 out_release_dma:
@@ -4946,7 +4952,8 @@ static void __exit floppy_module_exit(void)
 				if (disks[drive][i])
 					del_gendisk(disks[drive][i]);
 			}
-			platform_device_unregister(&floppy_device[drive]);
+			if (registered[drive])
+				platform_device_unregister(&floppy_device[drive]);
 		}
 		for (i = 0; i < ARRAY_SIZE(floppy_type); i++) {
 			if (disks[drive][i])
-- 
2.27.0

[RFC 04/12] scsi/sd: use blk_cleanup_queue() insted of put_disk()

[Luis Chamberlain]

The single put_disk() is useful if you know you're not doing
a cleanup after add_disk(), but since we want to add support
for that, just use the normal form of blk_cleanup_disk() to
cleanup the queue and put the disk.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/scsi/sd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 6d2d63629a90..6d0a82da7131 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3510,7 +3510,7 @@ static int sd_probe(struct device *dev)
  out_free_index:
 	ida_free(&sd_index_ida, index);
  out_put:
-	put_disk(gd);
+	blk_cleanup_disk(gd);
  out_free:
 	sd_zbc_release_disk(sdkp);
 	kfree(sdkp);
-- 
2.27.0

[RFC 05/12] scsi/sd: add error handling support for add_disk()

[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 <mcgrof@kernel.org>
---
 drivers/scsi/sd.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 6d0a82da7131..01af61a38e4e 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3489,7 +3489,11 @@ static int sd_probe(struct device *dev)
 		pm_runtime_set_autosuspend_delay(dev,
 			sdp->host->hostt->rpm_autosuspend_delay);
 	}
-	device_add_disk(dev, gd, NULL);
+
+	error = device_add_disk(dev, gd, NULL);
+	if (error)
+		goto out_free_index;
+
 	if (sdkp->capacity)
 		sd_dif_config_host(sdkp);
 
-- 
2.27.0

Re: [RFC 05/12] scsi/sd: add error handling support for add_disk()

[kernel test robot]

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

Hi Luis,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on block/for-next]
[also build test ERROR on mkp-scsi/for-next scsi/for-next v5.14-rc1 next-20210715]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Luis-Chamberlain/block-add_disk-driver-conversions-__register_blkdev/20210716-050317
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: nds32-randconfig-r036-20210715 (attached as .config)
compiler: nds32le-linux-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/c1a87f26cc4c93ee87a100d4ed4103489f9e8e2d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Luis-Chamberlain/block-add_disk-driver-conversions-__register_blkdev/20210716-050317
        git checkout c1a87f26cc4c93ee87a100d4ed4103489f9e8e2d
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=nds32 SHELL=/bin/bash drivers/md/ drivers/scsi/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/scsi/sd.c: In function 'sd_probe':
>> drivers/scsi/sd.c:3489:8: error: void value not ignored as it ought to be
    3489 |  error = device_add_disk(dev, gd, NULL);
         |        ^


vim +3489 drivers/scsi/sd.c

  3362	
  3363	/**
  3364	 *	sd_probe - called during driver initialization and whenever a
  3365	 *	new scsi device is attached to the system. It is called once
  3366	 *	for each scsi device (not just disks) present.
  3367	 *	@dev: pointer to device object
  3368	 *
  3369	 *	Returns 0 if successful (or not interested in this scsi device 
  3370	 *	(e.g. scanner)); 1 when there is an error.
  3371	 *
  3372	 *	Note: this function is invoked from the scsi mid-level.
  3373	 *	This function sets up the mapping between a given 
  3374	 *	<host,channel,id,lun> (found in sdp) and new device name 
  3375	 *	(e.g. /dev/sda). More precisely it is the block device major 
  3376	 *	and minor number that is chosen here.
  3377	 *
  3378	 *	Assume sd_probe is not re-entrant (for time being)
  3379	 *	Also think about sd_probe() and sd_remove() running coincidentally.
  3380	 **/
  3381	static int sd_probe(struct device *dev)
  3382	{
  3383		struct scsi_device *sdp = to_scsi_device(dev);
  3384		struct scsi_disk *sdkp;
  3385		struct gendisk *gd;
  3386		int index;
  3387		int error;
  3388	
  3389		scsi_autopm_get_device(sdp);
  3390		error = -ENODEV;
  3391		if (sdp->type != TYPE_DISK &&
  3392		    sdp->type != TYPE_ZBC &&
  3393		    sdp->type != TYPE_MOD &&
  3394		    sdp->type != TYPE_RBC)
  3395			goto out;
  3396	
  3397		if (!IS_ENABLED(CONFIG_BLK_DEV_ZONED) && sdp->type == TYPE_ZBC) {
  3398			sdev_printk(KERN_WARNING, sdp,
  3399				    "Unsupported ZBC host-managed device.\n");
  3400			goto out;
  3401		}
  3402	
  3403		SCSI_LOG_HLQUEUE(3, sdev_printk(KERN_INFO, sdp,
  3404						"sd_probe\n"));
  3405	
  3406		error = -ENOMEM;
  3407		sdkp = kzalloc(sizeof(*sdkp), GFP_KERNEL);
  3408		if (!sdkp)
  3409			goto out;
  3410	
  3411		gd = alloc_disk(SD_MINORS);
  3412		if (!gd)
  3413			goto out_free;
  3414	
  3415		index = ida_alloc(&sd_index_ida, GFP_KERNEL);
  3416		if (index < 0) {
  3417			sdev_printk(KERN_WARNING, sdp, "sd_probe: memory exhausted.\n");
  3418			goto out_put;
  3419		}
  3420	
  3421		error = sd_format_disk_name("sd", index, gd->disk_name, DISK_NAME_LEN);
  3422		if (error) {
  3423			sdev_printk(KERN_WARNING, sdp, "SCSI disk (sd) name length exceeded.\n");
  3424			goto out_free_index;
  3425		}
  3426	
  3427		sdkp->device = sdp;
  3428		sdkp->driver = &sd_template;
  3429		sdkp->disk = gd;
  3430		sdkp->index = index;
  3431		sdkp->max_retries = SD_MAX_RETRIES;
  3432		atomic_set(&sdkp->openers, 0);
  3433		atomic_set(&sdkp->device->ioerr_cnt, 0);
  3434	
  3435		if (!sdp->request_queue->rq_timeout) {
  3436			if (sdp->type != TYPE_MOD)
  3437				blk_queue_rq_timeout(sdp->request_queue, SD_TIMEOUT);
  3438			else
  3439				blk_queue_rq_timeout(sdp->request_queue,
  3440						     SD_MOD_TIMEOUT);
  3441		}
  3442	
  3443		device_initialize(&sdkp->dev);
  3444		sdkp->dev.parent = dev;
  3445		sdkp->dev.class = &sd_disk_class;
  3446		dev_set_name(&sdkp->dev, "%s", dev_name(dev));
  3447	
  3448		error = device_add(&sdkp->dev);
  3449		if (error)
  3450			goto out_free_index;
  3451	
  3452		get_device(dev);
  3453		dev_set_drvdata(dev, sdkp);
  3454	
  3455		gd->major = sd_major((index & 0xf0) >> 4);
  3456		gd->first_minor = ((index & 0xf) << 4) | (index & 0xfff00);
  3457	
  3458		gd->fops = &sd_fops;
  3459		gd->private_data = &sdkp->driver;
  3460		gd->queue = sdkp->device->request_queue;
  3461	
  3462		/* defaults, until the device tells us otherwise */
  3463		sdp->sector_size = 512;
  3464		sdkp->capacity = 0;
  3465		sdkp->media_present = 1;
  3466		sdkp->write_prot = 0;
  3467		sdkp->cache_override = 0;
  3468		sdkp->WCE = 0;
  3469		sdkp->RCD = 0;
  3470		sdkp->ATO = 0;
  3471		sdkp->first_scan = 1;
  3472		sdkp->max_medium_access_timeouts = SD_MAX_MEDIUM_TIMEOUTS;
  3473	
  3474		sd_revalidate_disk(gd);
  3475	
  3476		gd->flags = GENHD_FL_EXT_DEVT;
  3477		if (sdp->removable) {
  3478			gd->flags |= GENHD_FL_REMOVABLE;
  3479			gd->events |= DISK_EVENT_MEDIA_CHANGE;
  3480			gd->event_flags = DISK_EVENT_FLAG_POLL | DISK_EVENT_FLAG_UEVENT;
  3481		}
  3482	
  3483		blk_pm_runtime_init(sdp->request_queue, dev);
  3484		if (sdp->rpm_autosuspend) {
  3485			pm_runtime_set_autosuspend_delay(dev,
  3486				sdp->host->hostt->rpm_autosuspend_delay);
  3487		}
  3488	
> 3489		error = device_add_disk(dev, gd, NULL);
  3490		if (error)
  3491			goto out_free_index;
  3492	
  3493		if (sdkp->capacity)
  3494			sd_dif_config_host(sdkp);
  3495	
  3496		sd_revalidate_disk(gd);
  3497	
  3498		if (sdkp->security) {
  3499			sdkp->opal_dev = init_opal_dev(sdkp, &sd_sec_submit);
  3500			if (sdkp->opal_dev)
  3501				sd_printk(KERN_NOTICE, sdkp, "supports TCG Opal\n");
  3502		}
  3503	
  3504		sd_printk(KERN_NOTICE, sdkp, "Attached SCSI %sdisk\n",
  3505			  sdp->removable ? "removable " : "");
  3506		scsi_autopm_put_device(sdp);
  3507	
  3508		return 0;
  3509	
  3510	 out_free_index:
  3511		ida_free(&sd_index_ida, index);
  3512	 out_put:
  3513		blk_cleanup_disk(gd);
  3514	 out_free:
  3515		sd_zbc_release_disk(sdkp);
  3516		kfree(sdkp);
  3517	 out:
  3518		scsi_autopm_put_device(sdp);
  3519		return error;
  3520	}
  3521	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 17442 bytes --]

[RFC 06/12] scsi/sr: use blk_cleanup_disk() instead of put_disk()

[Luis Chamberlain]

The single put_disk() is useful if you know you're not doing
a cleanup after add_disk(), but since we want to add support
for that, just use the normal form of blk_cleanup_disk() to
cleanup the queue and put the disk.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/scsi/sr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 94c254e9012e..362f04a3761a 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -789,7 +789,7 @@ static int sr_probe(struct device *dev)
 	clear_bit(minor, sr_index_bits);
 	spin_unlock(&sr_index_lock);
 fail_put:
-	put_disk(disk);
+	blk_cleanup_disk(disk);
 	mutex_destroy(&cd->lock);
 fail_free:
 	kfree(cd);
-- 
2.27.0

[RFC 07/12] scsi/sr: add error handling support for add_disk()

[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 <mcgrof@kernel.org>
---
 drivers/scsi/sr.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 362f04a3761a..ed097b69821e 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -776,7 +776,10 @@ static int sr_probe(struct device *dev)
 	dev_set_drvdata(dev, cd);
 	disk->flags |= GENHD_FL_REMOVABLE;
 	sr_revalidate_disk(cd);
-	device_add_disk(&sdev->sdev_gendev, disk, NULL);
+
+	error = device_add_disk(&sdev->sdev_gendev, disk, NULL);
+	if (error)
+		goto fail_minor;
 
 	sdev_printk(KERN_DEBUG, sdev,
 		    "Attached scsi CD-ROM %s\n", cd->cdi.name);
-- 
2.27.0

Re: [RFC 07/12] scsi/sr: add error handling support for add_disk()

[kernel test robot]

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

Hi Luis,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on block/for-next]
[also build test ERROR on mkp-scsi/for-next scsi/for-next v5.14-rc1 next-20210716]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Luis-Chamberlain/block-add_disk-driver-conversions-__register_blkdev/20210716-050317
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: x86_64-randconfig-a001-20210715 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 0e49c54a8cbd3e779e5526a5888c683c01cc3c50)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/1b194339e9e7b904476b4fdfa4aa3cb801915e6f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Luis-Chamberlain/block-add_disk-driver-conversions-__register_blkdev/20210716-050317
        git checkout 1b194339e9e7b904476b4fdfa4aa3cb801915e6f
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/scsi/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/scsi/sr.c:780:8: error: assigning to 'int' from incompatible type 'void'
           error = device_add_disk(&sdev->sdev_gendev, disk, NULL);
                 ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 error generated.


vim +780 drivers/scsi/sr.c

   695	
   696	static int sr_probe(struct device *dev)
   697	{
   698		struct scsi_device *sdev = to_scsi_device(dev);
   699		struct gendisk *disk;
   700		struct scsi_cd *cd;
   701		int minor, error;
   702	
   703		scsi_autopm_get_device(sdev);
   704		error = -ENODEV;
   705		if (sdev->type != TYPE_ROM && sdev->type != TYPE_WORM)
   706			goto fail;
   707	
   708		error = -ENOMEM;
   709		cd = kzalloc(sizeof(*cd), GFP_KERNEL);
   710		if (!cd)
   711			goto fail;
   712	
   713		kref_init(&cd->kref);
   714	
   715		disk = alloc_disk(1);
   716		if (!disk)
   717			goto fail_free;
   718		mutex_init(&cd->lock);
   719	
   720		spin_lock(&sr_index_lock);
   721		minor = find_first_zero_bit(sr_index_bits, SR_DISKS);
   722		if (minor == SR_DISKS) {
   723			spin_unlock(&sr_index_lock);
   724			error = -EBUSY;
   725			goto fail_put;
   726		}
   727		__set_bit(minor, sr_index_bits);
   728		spin_unlock(&sr_index_lock);
   729	
   730		disk->major = SCSI_CDROM_MAJOR;
   731		disk->first_minor = minor;
   732		sprintf(disk->disk_name, "sr%d", minor);
   733		disk->fops = &sr_bdops;
   734		disk->flags = GENHD_FL_CD | GENHD_FL_BLOCK_EVENTS_ON_EXCL_WRITE;
   735		disk->events = DISK_EVENT_MEDIA_CHANGE | DISK_EVENT_EJECT_REQUEST;
   736		disk->event_flags = DISK_EVENT_FLAG_POLL | DISK_EVENT_FLAG_UEVENT;
   737	
   738		blk_queue_rq_timeout(sdev->request_queue, SR_TIMEOUT);
   739	
   740		cd->device = sdev;
   741		cd->disk = disk;
   742		cd->driver = &sr_template;
   743		cd->disk = disk;
   744		cd->capacity = 0x1fffff;
   745		cd->device->changed = 1;	/* force recheck CD type */
   746		cd->media_present = 1;
   747		cd->use = 1;
   748		cd->readcd_known = 0;
   749		cd->readcd_cdda = 0;
   750	
   751		cd->cdi.ops = &sr_dops;
   752		cd->cdi.handle = cd;
   753		cd->cdi.mask = 0;
   754		cd->cdi.capacity = 1;
   755		sprintf(cd->cdi.name, "sr%d", minor);
   756	
   757		sdev->sector_size = 2048;	/* A guess, just in case */
   758	
   759		/* FIXME: need to handle a get_capabilities failure properly ?? */
   760		get_capabilities(cd);
   761		sr_vendor_init(cd);
   762	
   763		set_capacity(disk, cd->capacity);
   764		disk->private_data = &cd->driver;
   765		disk->queue = sdev->request_queue;
   766	
   767		if (register_cdrom(disk, &cd->cdi))
   768			goto fail_minor;
   769	
   770		/*
   771		 * Initialize block layer runtime PM stuffs before the
   772		 * periodic event checking request gets started in add_disk.
   773		 */
   774		blk_pm_runtime_init(sdev->request_queue, dev);
   775	
   776		dev_set_drvdata(dev, cd);
   777		disk->flags |= GENHD_FL_REMOVABLE;
   778		sr_revalidate_disk(cd);
   779	
 > 780		error = device_add_disk(&sdev->sdev_gendev, disk, NULL);
   781		if (error)
   782			goto fail_minor;
   783	
   784		sdev_printk(KERN_DEBUG, sdev,
   785			    "Attached scsi CD-ROM %s\n", cd->cdi.name);
   786		scsi_autopm_put_device(cd->device);
   787	
   788		return 0;
   789	
   790	fail_minor:
   791		spin_lock(&sr_index_lock);
   792		clear_bit(minor, sr_index_bits);
   793		spin_unlock(&sr_index_lock);
   794	fail_put:
   795		blk_cleanup_disk(disk);
   796		mutex_destroy(&cd->lock);
   797	fail_free:
   798		kfree(cd);
   799	fail:
   800		scsi_autopm_put_device(sdev);
   801		return error;
   802	}
   803	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 47746 bytes --]

[RFC 08/12] block/ataflop: use the blk_cleanup_disk() helper

[Luis Chamberlain]

Use the helper to replace two lines with one.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/block/ataflop.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index a093644ac39f..abb6fde0bd81 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -2076,8 +2076,7 @@ static int __init atari_floppy_init (void)
 
 err:
 	while (--i >= 0) {
-		blk_cleanup_queue(unit[i].disk[0]->queue);
-		put_disk(unit[i].disk[0]);
+		blk_cleanup_disk(unit[i].disk[0]);
 		blk_mq_free_tag_set(&unit[i].tag_set);
 	}
 
@@ -2135,8 +2134,7 @@ static void __exit atari_floppy_exit(void)
 			if (!unit[i].disk[type])
 				continue;
 			del_gendisk(unit[i].disk[type]);
-			blk_cleanup_queue(unit[i].disk[type]->queue);
-			put_disk(unit[i].disk[type]);
+			blk_cleanup_disk(unit[i].disk[0]);
 		}
 		blk_mq_free_tag_set(&unit[i].tag_set);
 	}
-- 
2.27.0

[RFC 09/12] block/ataflop: add a helper for removing disks

[Luis Chamberlain]

The del_gendisk() can be used now safely even if add_disk()
did not complete, and so using a helper allows other code to
use this later.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/block/ataflop.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index abb6fde0bd81..73eb80b58888 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -1988,6 +1988,23 @@ static int ataflop_alloc_disk(unsigned int drive, unsigned int type)
 
 static DEFINE_MUTEX(ataflop_probe_lock);
 
+static void atari_floppy_disk_remove(void)
+{
+	int i, type;
+
+	for (i = 0; i < FD_MAX_UNITS; i++) {
+		for (type = 0; type < NUM_DISK_MINORS; type++) {
+			if (!unit[i].disk[type])
+				continue;
+			del_gendisk(unit[i].disk[type]);
+			blk_cleanup_disk(unit[i].disk[0]);
+		}
+		blk_mq_free_tag_set(&unit[i].tag_set);
+	}
+	unregister_blkdev(FLOPPY_MAJOR, "fd");
+
+}
+
 static void ataflop_probe(dev_t dev)
 {
 	int drive = MINOR(dev) & 3;
@@ -2127,19 +2144,7 @@ __setup("floppy=", atari_floppy_setup);
 
 static void __exit atari_floppy_exit(void)
 {
-	int i, type;
-
-	for (i = 0; i < FD_MAX_UNITS; i++) {
-		for (type = 0; type < NUM_DISK_MINORS; type++) {
-			if (!unit[i].disk[type])
-				continue;
-			del_gendisk(unit[i].disk[type]);
-			blk_cleanup_disk(unit[i].disk[0]);
-		}
-		blk_mq_free_tag_set(&unit[i].tag_set);
-	}
-	unregister_blkdev(FLOPPY_MAJOR, "fd");
-
+	atari_floppy_disk_remove();
 	del_timer_sync(&fd_timer);
 	atari_stram_free( DMABuffer );
 }
-- 
2.27.0

[RFC 10/12] block/ataflop add error handling support for add_disk()

[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.

We reuse the atari_floppy_disk_remove() call.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/block/ataflop.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index 73eb80b58888..4b3f1158fa04 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -2081,7 +2081,9 @@ static int __init atari_floppy_init (void)
 	for (i = 0; i < FD_MAX_UNITS; i++) {
 		unit[i].track = -1;
 		unit[i].flags = 0;
-		add_disk(unit[i].disk[0]);
+		ret = add_disk(unit[i].disk[0]);
+		if (ret)
+			goto err_out_dma;
 	}
 
 	printk(KERN_INFO "Atari floppy driver: max. %cD, %strack buffering\n",
@@ -2091,13 +2093,10 @@ static int __init atari_floppy_init (void)
 
 	return 0;
 
+err_out_dma:
+	atari_stram_free(DMABuffer);
 err:
-	while (--i >= 0) {
-		blk_cleanup_disk(unit[i].disk[0]);
-		blk_mq_free_tag_set(&unit[i].tag_set);
-	}
-
-	unregister_blkdev(FLOPPY_MAJOR, "fd");
+	atari_floppy_disk_remove();
 out_unlock:
 	mutex_unlock(&ataflop_probe_lock);
 	return ret;
-- 
2.27.0

[RFC 11/12] block/brd: add error handling support for add_disk()

[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 <mcgrof@kernel.org>
---
 drivers/block/brd.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 95694113e38e..ca017ca315c5 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -375,6 +375,7 @@ static int brd_alloc(int i)
 	struct brd_device *brd;
 	struct gendisk *disk;
 	char buf[DISK_NAME_LEN];
+	int err = -ENOMEM;
 
 	brd = kzalloc(sizeof(*brd), GFP_KERNEL);
 	if (!brd)
@@ -413,14 +414,19 @@ static int brd_alloc(int i)
 	/* Tell the block layer that this is not a rotational device */
 	blk_queue_flag_set(QUEUE_FLAG_NONROT, disk->queue);
 	blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, disk->queue);
-	add_disk(disk);
+	err = add_disk(disk);
+	if (err)
+		goto out_cleanup_disk;
+
 	list_add_tail(&brd->brd_list, &brd_devices);
 
 	return 0;
 
+out_cleanup_disk:
+	blk_cleanup_disk(disk);
 out_free_dev:
 	kfree(brd);
-	return -ENOMEM;
+	return err;
 }
 
 static void brd_probe(dev_t dev)
-- 
2.27.0

[RFC 12/12] block: make probe in blk_request_module() return an error

[Luis Chamberlain]

This makes the probe callback use din blk_request_module() return
an error. We do this as add_disk() now has error handling, and
so we can bail earlier than before. If a probe is not implemented
then its not used.

This is mostly useful for the following drivers:

  * ataflop
  * brd
  * floppy
  * loop
  * scsi/sd

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 block/genhd.c           | 15 +++++++++------
 drivers/block/ataflop.c | 20 +++++++++++++++-----
 drivers/block/brd.c     |  7 +++++--
 drivers/block/floppy.c  | 14 ++++++++++----
 drivers/block/loop.c    |  6 +++---
 drivers/scsi/sd.c       |  3 ++-
 fs/block_dev.c          |  5 ++++-
 include/linux/genhd.h   |  4 ++--
 8 files changed, 50 insertions(+), 24 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 72703d243b44..ca6393df09ad 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -161,7 +161,7 @@ static struct blk_major_name {
 	struct blk_major_name *next;
 	int major;
 	char name[16];
-	void (*probe)(dev_t devt);
+	int (*probe)(dev_t devt);
 } *major_names[BLKDEV_MAJOR_HASH_SIZE];
 static DEFINE_MUTEX(major_names_lock);
 
@@ -190,7 +190,7 @@ void blkdev_show(struct seq_file *seqf, off_t offset)
  * @major: the requested major device number [1..BLKDEV_MAJOR_MAX-1]. If
  *         @major = 0, try to allocate any unused major number.
  * @name: the name of the new block device as a zero terminated string
- * @probe: allback that is called on access to any minor number of @major
+ * @probe: callback that is called on access to any minor number of @major
  *
  * The @name must be unique within the system.
  *
@@ -208,7 +208,7 @@ void blkdev_show(struct seq_file *seqf, off_t offset)
  * Use register_blkdev instead for any new code.
  */
 int __register_blkdev(unsigned int major, const char *name,
-		void (*probe)(dev_t devt))
+		int (*probe)(dev_t devt))
 {
 	struct blk_major_name **n, *p;
 	int index, ret = 0;
@@ -728,17 +728,18 @@ static ssize_t disk_badblocks_store(struct device *dev,
 	return badblocks_store(disk->bb, page, len, 0);
 }
 
-void blk_request_module(dev_t devt)
+int blk_request_module(dev_t devt)
 {
 	unsigned int major = MAJOR(devt);
 	struct blk_major_name **n;
+	int err;
 
 	mutex_lock(&major_names_lock);
 	for (n = &major_names[major_to_index(major)]; *n; n = &(*n)->next) {
 		if ((*n)->major == major && (*n)->probe) {
-			(*n)->probe(devt);
+			err = (*n)->probe(devt);
 			mutex_unlock(&major_names_lock);
-			return;
+			return err;
 		}
 	}
 	mutex_unlock(&major_names_lock);
@@ -746,6 +747,8 @@ void blk_request_module(dev_t devt)
 	if (request_module("block-major-%d-%d", MAJOR(devt), MINOR(devt)) > 0)
 		/* Make old-style 2.4 aliases work */
 		request_module("block-major-%d", MAJOR(devt));
+
+	return 0;
 }
 
 /*
diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index 4b3f1158fa04..ce08b8e60163 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -2005,22 +2005,32 @@ static void atari_floppy_disk_remove(void)
 
 }
 
-static void ataflop_probe(dev_t dev)
+static int ataflop_probe(dev_t dev)
 {
 	int drive = MINOR(dev) & 3;
 	int type  = MINOR(dev) >> 2;
+	int err = -ENODEV;
 
 	if (type)
 		type--;
 
-	if (drive >= FD_MAX_UNITS || type >= NUM_DISK_MINORS)
-		return;
+	if (drive >= FD_MAX_UNITS || type >= NUM_DISK_MINORS) {
+		err = -EINVAL;
+		goto out;
+	}
+
 	mutex_lock(&ataflop_probe_lock);
 	if (!unit[drive].disk[type]) {
-		if (ataflop_alloc_disk(drive, type) == 0)
-			add_disk(unit[drive].disk[type]);
+		if (ataflop_alloc_disk(drive, type) == 0) {
+			err = add_disk(unit[drive].disk[type]);
+			if (err)
+				blk_cleanup_disk(unit[drive].disk[type]);
+		}
 	}
 	mutex_unlock(&ataflop_probe_lock);
+
+out:
+	return err;
 }
 
 static int __init atari_floppy_init (void)
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index ca017ca315c5..104adf70a15a 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -429,10 +429,11 @@ static int brd_alloc(int i)
 	return err;
 }
 
-static void brd_probe(dev_t dev)
+static int brd_probe(dev_t dev)
 {
 	int i = MINOR(dev) / max_part;
 	struct brd_device *brd;
+	int err = 0;
 
 	mutex_lock(&brd_devices_mutex);
 	list_for_each_entry(brd, &brd_devices, brd_list) {
@@ -440,9 +441,11 @@ static void brd_probe(dev_t dev)
 			goto out_unlock;
 	}
 
-	brd_alloc(i);
+	err = brd_alloc(i);
 out_unlock:
 	mutex_unlock(&brd_devices_mutex);
+
+	return err;
 }
 
 static void brd_del_one(struct brd_device *brd)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index cad17b49e700..52bc645f3060 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4517,21 +4517,27 @@ static int floppy_alloc_disk(unsigned int drive, unsigned int type)
 
 static DEFINE_MUTEX(floppy_probe_lock);
 
-static void floppy_probe(dev_t dev)
+static int floppy_probe(dev_t dev)
 {
 	unsigned int drive = (MINOR(dev) & 3) | ((MINOR(dev) & 0x80) >> 5);
 	unsigned int type = (MINOR(dev) >> 2) & 0x1f;
+	int err;
 
 	if (drive >= N_DRIVE || !floppy_available(drive) ||
 	    type >= ARRAY_SIZE(floppy_type))
-		return;
+		return -EINVAL;
 
 	mutex_lock(&floppy_probe_lock);
 	if (!disks[drive][type]) {
-		if (floppy_alloc_disk(drive, type) == 0)
-			add_disk(disks[drive][type]);
+		if (floppy_alloc_disk(drive, type) == 0) {
+			err = add_disk(disks[drive][type]);
+			if (err)
+				blk_cleanup_disk(disks[drive][type]);
+		}
 	}
 	mutex_unlock(&floppy_probe_lock);
+
+	return err;
 }
 
 static int __init do_floppy_init(void)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index efbd8e29aca7..081cccd39bc9 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -2358,13 +2358,13 @@ static void loop_remove(struct loop_device *lo)
 	kfree(lo);
 }
 
-static void loop_probe(dev_t dev)
+static int loop_probe(dev_t dev)
 {
 	int idx = MINOR(dev) >> part_shift;
 
 	if (max_loop && idx >= max_loop)
-		return;
-	loop_add(idx);
+		return -EINVAL;
+	return loop_add(idx);
 }
 
 static int loop_control_remove(int idx)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 01af61a38e4e..800dfd5984bd 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -633,8 +633,9 @@ static struct scsi_driver sd_template = {
  * Don't request a new module, as that could deadlock in multipath
  * environment.
  */
-static void sd_default_probe(dev_t devt)
+static int sd_default_probe(dev_t devt)
 {
+	return 0;
 }
 
 /*
diff --git a/fs/block_dev.c b/fs/block_dev.c
index c41d0e550d39..531feb097cb2 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1326,10 +1326,13 @@ struct block_device *blkdev_get_no_open(dev_t dev)
 {
 	struct block_device *bdev;
 	struct gendisk *disk;
+	int err;
 
 	bdev = bdget(dev);
 	if (!bdev) {
-		blk_request_module(dev);
+		err = blk_request_module(dev);
+		if (err)
+			return NULL;
 		bdev = bdget(dev);
 		if (!bdev)
 			return NULL;
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 73024416d2d5..9db56d4562ae 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -312,7 +312,7 @@ struct gendisk *__blk_alloc_disk(int node);
 void blk_cleanup_disk(struct gendisk *disk);
 
 int __register_blkdev(unsigned int major, const char *name,
-		void (*probe)(dev_t devt));
+		int (*probe)(dev_t devt));
 #define register_blkdev(major, name) \
 	__register_blkdev(major, name, NULL)
 void unregister_blkdev(unsigned int major, const char *name);
@@ -342,7 +342,7 @@ static inline void bd_unlink_disk_holder(struct block_device *bdev,
 
 dev_t part_devt(struct gendisk *disk, u8 partno);
 dev_t blk_lookup_devt(const char *name, int partno);
-void blk_request_module(dev_t devt);
+int blk_request_module(dev_t devt);
 #ifdef CONFIG_BLOCK
 void printk_all_partitions(void);
 #else /* CONFIG_BLOCK */
-- 
2.27.0