2021-08-30 21:31:33

by Luis Chamberlain

[permalink] [raw]
Subject: [PATCH v3 0/8] block: first batch of add_disk() error handling conversions

Jens,

I think this first set is ready, but pending review of just two patches:

* mmc/core/block
* dm

All other patches have a respective Reviewed-by tag. The above two
patches were integrated back into the series once I understood
Christoph's concerns, and adjusted the patch as such.

This goes rebased onto your for-next as of today. If anyone wants to
explore the pending full set this is up on my linux-next branch
20210830-for-axboe-add-disk-error-handling-next [0].

[0] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20210830-for-axboe-add-disk-error-handling-next

Luis Chamberlain (8):
scsi/sd: add error handling support for add_disk()
scsi/sr: add error handling support for add_disk()
nvme: add error handling support for add_disk()
mmc/core/block: add error handling support for add_disk()
md: add error handling support for add_disk()
dm: add add_disk() error handling
loop: add error handling support for add_disk()
nbd: add error handling support for add_disk()

drivers/block/loop.c | 9 ++++++++-
drivers/block/nbd.c | 6 +++++-
drivers/md/dm.c | 4 +++-
drivers/md/md.c | 7 ++++++-
drivers/mmc/core/block.c | 7 ++++++-
drivers/nvme/host/core.c | 9 ++++++++-
drivers/scsi/sd.c | 6 +++++-
drivers/scsi/sr.c | 5 ++++-
8 files changed, 45 insertions(+), 8 deletions(-)

--
2.30.2


2021-08-30 21:31:44

by Luis Chamberlain

[permalink] [raw]
Subject: [PATCH v3 2/8] scsi/sr: 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.

Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Luis Chamberlain <[email protected]>
---
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 2942a4ec9bdd..72fd21844367 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -779,7 +779,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.30.2

2021-09-06 06:15:02

by Hannes Reinecke

[permalink] [raw]
Subject: Re: [PATCH v3 2/8] scsi/sr: add error handling support for add_disk()

On 8/30/21 11:25 PM, Luis Chamberlain wrote:
> We never checked for errors on add_disk() as this function
> returned void. Now that this is fixed, use the shiny new
> error handling.
>
> Reviewed-by: Christoph Hellwig <[email protected]>
> Signed-off-by: Luis Chamberlain <[email protected]>
> ---
> drivers/scsi/sr.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
Reviewed-by: Hannes Reinecke <[email protected]>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
[email protected] +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

2021-09-07 01:40:28

by Ming Lei

[permalink] [raw]
Subject: Re: [PATCH v3 2/8] scsi/sr: add error handling support for add_disk()

On Mon, Aug 30, 2021 at 02:25:32PM -0700, Luis Chamberlain wrote:
> We never checked for errors on add_disk() as this function
> returned void. Now that this is fixed, use the shiny new
> error handling.
>
> Reviewed-by: Christoph Hellwig <[email protected]>
> Signed-off-by: Luis Chamberlain <[email protected]>
> ---
> 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 2942a4ec9bdd..72fd21844367 100644
> --- a/drivers/scsi/sr.c
> +++ b/drivers/scsi/sr.c
> @@ -779,7 +779,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;

You don't undo register_cdrom(), maybe you can use kref_put(&cd->kref, sr_kref_release);
to simplify the error handling.


Thanks,
Ming

2021-09-13 17:29:07

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCH v3 2/8] scsi/sr: add error handling support for add_disk()

On Tue, Sep 07, 2021 at 09:37:05AM +0800, Ming Lei wrote:
> On Mon, Aug 30, 2021 at 02:25:32PM -0700, Luis Chamberlain wrote:
> > We never checked for errors on add_disk() as this function
> > returned void. Now that this is fixed, use the shiny new
> > error handling.
> >
> > Reviewed-by: Christoph Hellwig <[email protected]>
> > Signed-off-by: Luis Chamberlain <[email protected]>
> > ---
> > 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 2942a4ec9bdd..72fd21844367 100644
> > --- a/drivers/scsi/sr.c
> > +++ b/drivers/scsi/sr.c
> > @@ -779,7 +779,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;
>
> You don't undo register_cdrom(), maybe you can use kref_put(&cd->kref, sr_kref_release);
> to simplify the error handling.

Works with me, thanks!

Luis