linux-raid.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Luis Chamberlain <mcgrof@kernel.org>
To: axboe@kernel.dk, hch@lst.de, penguin-kernel@i-love.sakura.ne.jp,
	dan.j.williams@intel.com, vishal.l.verma@intel.com,
	dave.jiang@intel.com, ira.weiny@intel.com, richard@nod.at,
	miquel.raynal@bootlin.com, vigneshr@ti.com, efremov@linux.com,
	song@kernel.org, martin.petersen@oracle.com, hare@suse.de,
	jack@suse.cz, ming.lei@redhat.com, tj@kernel.org,
	mcgrof@kernel.org
Cc: linux-mtd@lists.infradead.org, linux-scsi@vger.kernel.org,
	linux-raid@vger.kernel.org, linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH v3 12/13] block: fix __register_blkdev() probe add_disk() failures
Date: Wed,  3 Nov 2021 10:45:20 -0700	[thread overview]
Message-ID: <20211103174521.1426407-13-mcgrof@kernel.org> (raw)
In-Reply-To: <20211103174521.1426407-1-mcgrof@kernel.org>

__register_blkdev() is used to register a probe callback, and
that callback is typically used to call add_disk(). Now that
we are able to capture errors for add_disk(), we need to fix
those probe calls where add_disk() fails and clean up resources.

We don't extend the probe call to return the error given:

1) we'd have to always special-case the case where the disk
   was already present, as otherwise concurrent requests to
   open an existing block device would fail, and this would be
   a userspace visible change
2) the error from ilookup() on blkdev_get_no_open() is sufficient
3) The only thing the probe call is used for is to support
   pre-devtmpfs, pre-udev semantics that want to create disks when
   their pre-created device node is accessed, and so we don't care
   for failures on probe there.

We expand documentation for the probe callback to ensure users
cleanup resources if add_disk() is used and to clarify this interface
may be removed in the future.

This fixes the ataflop and floppy driver uses of the probe call,
which lacked proper error handling for the add_disk() calls.

Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 block/genhd.c           |  5 ++++-
 drivers/block/ataflop.c | 12 +++++++++---
 drivers/block/floppy.c  | 11 +++++++++--
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 4ed87f25276a..8837a89242a2 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -213,7 +213,10 @@ 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: pre-devtmpfs / pre-udev callback used to create disks when their
+ *	   their pre-created device node is accessed. When a probe call
+ *	   uses add_disk() and it fails the driver must cleanup resources.
+ *	   This interface may soon be removed.
  *
  * The @name must be unique within the system.
  *
diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index 170dd193cef6..cce6615fe10a 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -2012,6 +2012,7 @@ static void ataflop_probe(dev_t dev)
 {
 	int drive = MINOR(dev) & 3;
 	int type  = MINOR(dev) >> 2;
+	int err = 0;
 
 	if (type)
 		type--;
@@ -2019,9 +2020,14 @@ static void ataflop_probe(dev_t dev)
 	if (drive >= FD_MAX_UNITS || type >= NUM_DISK_MINORS)
 		return;
 	if (!unit[drive].disk[type]) {
-		if (ataflop_alloc_disk(drive, type) == 0) {
-			add_disk(unit[drive].disk[type]);
-			unit[drive].registered[type] = true;
+		err = ataflop_alloc_disk(drive, type);
+		if (err == 0) {
+			err = add_disk(unit[drive].disk[type]);
+			if (err) {
+				blk_cleanup_disk(unit[drive].disk[type]);
+				unit[drive].disk[type] = NULL;
+			} else
+				unit[drive].registered[type] = true;
 		}
 	}
 }
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 3873e789478e..cebc11b74b1a 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4522,6 +4522,7 @@ static void floppy_probe(dev_t dev)
 {
 	unsigned int drive = (MINOR(dev) & 3) | ((MINOR(dev) & 0x80) >> 5);
 	unsigned int type = (MINOR(dev) >> 2) & 0x1f;
+	int err = 0;
 
 	if (drive >= N_DRIVE || !floppy_available(drive) ||
 	    type >= ARRAY_SIZE(floppy_type))
@@ -4529,8 +4530,14 @@ static void floppy_probe(dev_t dev)
 
 	mutex_lock(&floppy_probe_lock);
 	if (!disks[drive][type]) {
-		if (floppy_alloc_disk(drive, type) == 0)
-			add_disk(disks[drive][type]);
+		err = floppy_alloc_disk(drive, type);
+		if (err == 0) {
+			err = add_disk(disks[drive][type]);
+			if (err) {
+				blk_cleanup_disk(disks[drive][type]);
+				disks[drive][type] = NULL;
+			}
+		}
 	}
 	mutex_unlock(&floppy_probe_lock);
 }
-- 
2.33.0


  parent reply	other threads:[~2021-11-03 17:45 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-03 17:45 [PATCH v3 00/13] block: add_disk() error handling stragglers Luis Chamberlain
2021-11-03 17:45 ` [PATCH v3 01/13] nvdimm/btt: do not call del_gendisk() if not needed Luis Chamberlain
2021-11-03 17:45 ` [PATCH v3 02/13] nvdimm/btt: use goto error labels on btt_blk_init() Luis Chamberlain
2021-11-03 17:45 ` [PATCH v3 03/13] nvdimm/btt: add error handling support for add_disk() Luis Chamberlain
2021-11-03 17:45 ` [PATCH v3 04/13] nvdimm/blk: avoid calling del_gendisk() on early failures Luis Chamberlain
2021-11-03 17:45 ` [PATCH v3 05/13] nvdimm/blk: add error handling support for add_disk() Luis Chamberlain
2021-11-03 17:45 ` [PATCH v3 06/13] nvdimm/pmem: cleanup the disk if pmem_release_disk() is yet assigned Luis Chamberlain
2021-11-03 17:45 ` [PATCH v3 07/13] nvdimm/pmem: use add_disk() error handling Luis Chamberlain
2021-11-03 17:45 ` [PATCH v3 08/13] z2ram: add error handling support for add_disk() Luis Chamberlain
2021-11-03 17:45 ` [PATCH v3 09/13] block/sunvdc: " Luis Chamberlain
2021-11-03 17:45 ` [PATCH v3 10/13] mtd/ubi/block: " Luis Chamberlain
2021-11-03 17:45 ` [PATCH v3 11/13] ataflop: remove ataflop_probe_lock mutex Luis Chamberlain
2021-11-03 17:45 ` Luis Chamberlain [this message]
2021-11-03 17:48   ` [PATCH v3 12/13] block: fix __register_blkdev() probe add_disk() failures Christoph Hellwig
2021-11-03 17:45 ` [PATCH v3 13/13] block: add __must_check for *add_disk*() callers Luis Chamberlain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211103174521.1426407-13-mcgrof@kernel.org \
    --to=mcgrof@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=dan.j.williams@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=efremov@linux.com \
    --cc=hare@suse.de \
    --cc=hch@lst.de \
    --cc=ira.weiny@intel.com \
    --cc=jack@suse.cz \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-raid@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=ming.lei@redhat.com \
    --cc=miquel.raynal@bootlin.com \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=richard@nod.at \
    --cc=song@kernel.org \
    --cc=tj@kernel.org \
    --cc=vigneshr@ti.com \
    --cc=vishal.l.verma@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).