| Message ID | 20211021163856.2000993-1-mcgrof@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | last batch of add_disk() error handling conversions | expand |
On 2021/10/22 1:38, Luis Chamberlain wrote: > I rebased Tetsuo Handa's patch onto the latest linux-next as this > series depends on it, and so I am sending it part of this series as > without it, this won't apply. Tetsuo, does the rebase of your patch > look OK? OK, though I wanted my fix to be sent to upstream and stable before this series. > > If it is not too much trouble, I'd like to ask for testing for the > ataflop changes from Michael Schmitz, if possible, that is he'd just > have to merge Tetsuo's rebased patch and the 2nd patch in this series. > This is all rebased on linux-next tag 20211020. Yes, please. After this series, I guess we can remove "bool registered[NUM_DISK_MINORS];" like below due to (unit[drive].disk[type] != NULL) == (unit[drive].registered[type] == true). Regarding this series, setting unit[drive].registered[type] = true in ataflop_probe() is pointless because atari_floppy_cleanup() checks unit[i].disk[type] != NULL for calling del_gendisk(). And we need to fix __register_blkdev() in driver/block/floppy.c because floppy_probe_lock is pointless. drivers/block/ataflop.c | 75 +++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 47 deletions(-) diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c index c58750dcc685..7fedf8506335 100644 --- a/drivers/block/ataflop.c +++ b/drivers/block/ataflop.c @@ -299,7 +299,6 @@ static struct atari_floppy_struct { disk change detection) */ int flags; /* flags */ struct gendisk *disk[NUM_DISK_MINORS]; - bool registered[NUM_DISK_MINORS]; int ref; int type; struct blk_mq_tag_set tag_set; @@ -1988,41 +1987,20 @@ static int ataflop_probe(dev_t dev) if (drive >= FD_MAX_UNITS || type >= NUM_DISK_MINORS) return -EINVAL; - if (!unit[drive].disk[type]) { - 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; + if (unit[drive].disk[type]) + return 0; + 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; } } return err; } -static void atari_floppy_cleanup(void) -{ - int i; - int 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_queue(unit[i].disk[type]->queue); - put_disk(unit[i].disk[type]); - } - blk_mq_free_tag_set(&unit[i].tag_set); - } - - del_timer_sync(&fd_timer); - atari_stram_free(DMABuffer); -} - static void atari_cleanup_floppy_disk(struct atari_floppy_struct *fs) { int type; @@ -2030,13 +2008,24 @@ static void atari_cleanup_floppy_disk(struct atari_floppy_struct *fs) for (type = 0; type < NUM_DISK_MINORS; type++) { if (!fs->disk[type]) continue; - if (fs->registered[type]) - del_gendisk(fs->disk[type]); + del_gendisk(fs->disk[type]); blk_cleanup_disk(fs->disk[type]); } blk_mq_free_tag_set(&fs->tag_set); } +static void atari_floppy_cleanup(void) +{ + int i; + + for (i = 0; i < FD_MAX_UNITS; i++) + atari_cleanup_floppy_disk(&unit[i]); + + del_timer_sync(&fd_timer); + if (DMABuffer) + atari_stram_free(DMABuffer); +} + static int __init atari_floppy_init (void) { int i; @@ -2055,13 +2044,10 @@ static int __init atari_floppy_init (void) unit[i].tag_set.numa_node = NUMA_NO_NODE; unit[i].tag_set.flags = BLK_MQ_F_SHOULD_MERGE; ret = blk_mq_alloc_tag_set(&unit[i].tag_set); - if (ret) - goto err; - - ret = ataflop_alloc_disk(i, 0); if (ret) { - blk_mq_free_tag_set(&unit[i].tag_set); - goto err; + while (--i >= 0) + blk_mq_free_tag_set(&unit[i].tag_set); + return ret; } } @@ -2090,10 +2076,9 @@ static int __init atari_floppy_init (void) for (i = 0; i < FD_MAX_UNITS; i++) { unit[i].track = -1; unit[i].flags = 0; - ret = add_disk(unit[i].disk[0]); - if (ret) - goto err_out_dma; - unit[i].registered[0] = true; + ret = ataflop_probe(MKDEV(0, 1 << 2)); + if (err) + goto err; } printk(KERN_INFO "Atari floppy driver: max. %cD, %strack buffering\n", @@ -2108,12 +2093,8 @@ static int __init atari_floppy_init (void) } return ret; -err_out_dma: - atari_stram_free(DMABuffer); err: - while (--i >= 0) - atari_cleanup_floppy_disk(&unit[i]); - + atari_floppy_cleanup(); return ret; }
Luis, Tetsuo, I'll try to test this - still running 5.13 (need the old IDE driver for now), so not sure this will all apply cleanly. Cheers, Michael On 22/10/21 14:06, Tetsuo Handa wrote: > On 2021/10/22 1:38, Luis Chamberlain wrote: >> I rebased Tetsuo Handa's patch onto the latest linux-next as this >> series depends on it, and so I am sending it part of this series as >> without it, this won't apply. Tetsuo, does the rebase of your patch >> look OK? > > OK, though I wanted my fix to be sent to upstream and stable before this series. > >> >> If it is not too much trouble, I'd like to ask for testing for the >> ataflop changes from Michael Schmitz, if possible, that is he'd just >> have to merge Tetsuo's rebased patch and the 2nd patch in this series. >> This is all rebased on linux-next tag 20211020. > > Yes, please. > > After this series, I guess we can remove "bool registered[NUM_DISK_MINORS];" like below > due to (unit[drive].disk[type] != NULL) == (unit[drive].registered[type] == true). > Regarding this series, setting unit[drive].registered[type] = true in ataflop_probe() is > pointless because atari_floppy_cleanup() checks unit[i].disk[type] != NULL for calling > del_gendisk(). And we need to fix __register_blkdev() in driver/block/floppy.c because > floppy_probe_lock is pointless. > > drivers/block/ataflop.c | 75 +++++++++++++++-------------------------- > 1 file changed, 28 insertions(+), 47 deletions(-) > > diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c > index c58750dcc685..7fedf8506335 100644 > --- a/drivers/block/ataflop.c > +++ b/drivers/block/ataflop.c > @@ -299,7 +299,6 @@ static struct atari_floppy_struct { > disk change detection) */ > int flags; /* flags */ > struct gendisk *disk[NUM_DISK_MINORS]; > - bool registered[NUM_DISK_MINORS]; > int ref; > int type; > struct blk_mq_tag_set tag_set; > @@ -1988,41 +1987,20 @@ static int ataflop_probe(dev_t dev) > if (drive >= FD_MAX_UNITS || type >= NUM_DISK_MINORS) > return -EINVAL; > > - if (!unit[drive].disk[type]) { > - 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; > + if (unit[drive].disk[type]) > + return 0; > + 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; > } > } > > return err; > } > > -static void atari_floppy_cleanup(void) > -{ > - int i; > - int 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_queue(unit[i].disk[type]->queue); > - put_disk(unit[i].disk[type]); > - } > - blk_mq_free_tag_set(&unit[i].tag_set); > - } > - > - del_timer_sync(&fd_timer); > - atari_stram_free(DMABuffer); > -} > - > static void atari_cleanup_floppy_disk(struct atari_floppy_struct *fs) > { > int type; > @@ -2030,13 +2008,24 @@ static void atari_cleanup_floppy_disk(struct atari_floppy_struct *fs) > for (type = 0; type < NUM_DISK_MINORS; type++) { > if (!fs->disk[type]) > continue; > - if (fs->registered[type]) > - del_gendisk(fs->disk[type]); > + del_gendisk(fs->disk[type]); > blk_cleanup_disk(fs->disk[type]); > } > blk_mq_free_tag_set(&fs->tag_set); > } > > +static void atari_floppy_cleanup(void) > +{ > + int i; > + > + for (i = 0; i < FD_MAX_UNITS; i++) > + atari_cleanup_floppy_disk(&unit[i]); > + > + del_timer_sync(&fd_timer); > + if (DMABuffer) > + atari_stram_free(DMABuffer); > +} > + > static int __init atari_floppy_init (void) > { > int i; > @@ -2055,13 +2044,10 @@ static int __init atari_floppy_init (void) > unit[i].tag_set.numa_node = NUMA_NO_NODE; > unit[i].tag_set.flags = BLK_MQ_F_SHOULD_MERGE; > ret = blk_mq_alloc_tag_set(&unit[i].tag_set); > - if (ret) > - goto err; > - > - ret = ataflop_alloc_disk(i, 0); > if (ret) { > - blk_mq_free_tag_set(&unit[i].tag_set); > - goto err; > + while (--i >= 0) > + blk_mq_free_tag_set(&unit[i].tag_set); > + return ret; > } > } > > @@ -2090,10 +2076,9 @@ static int __init atari_floppy_init (void) > for (i = 0; i < FD_MAX_UNITS; i++) { > unit[i].track = -1; > unit[i].flags = 0; > - ret = add_disk(unit[i].disk[0]); > - if (ret) > - goto err_out_dma; > - unit[i].registered[0] = true; > + ret = ataflop_probe(MKDEV(0, 1 << 2)); > + if (err) > + goto err; > } > > printk(KERN_INFO "Atari floppy driver: max. %cD, %strack buffering\n", > @@ -2108,12 +2093,8 @@ static int __init atari_floppy_init (void) > } > return ret; > > -err_out_dma: > - atari_stram_free(DMABuffer); > err: > - while (--i >= 0) > - atari_cleanup_floppy_disk(&unit[i]); > - > + atari_floppy_cleanup(); > return ret; > } > >
Turns out both patches don't apply cleanly to 5.13 (probably no surprise to you). I'll update my system to work with the pata IDE driver and update to the latest version in Geert's m68k tree (meant to do that anyway...), might take a few days. Cheers, Michael On 22/10/21 15:33, Michael Schmitz wrote: > Luis, Tetsuo, > > I'll try to test this - still running 5.13 (need the old IDE driver for > now), so not sure this will all apply cleanly. > > Cheers, > > Michael > > > On 22/10/21 14:06, Tetsuo Handa wrote: >> On 2021/10/22 1:38, Luis Chamberlain wrote: >>> I rebased Tetsuo Handa's patch onto the latest linux-next as this >>> series depends on it, and so I am sending it part of this series as >>> without it, this won't apply. Tetsuo, does the rebase of your patch >>> look OK? >> >> OK, though I wanted my fix to be sent to upstream and stable before >> this series. >> >>> >>> If it is not too much trouble, I'd like to ask for testing for the >>> ataflop changes from Michael Schmitz, if possible, that is he'd just >>> have to merge Tetsuo's rebased patch and the 2nd patch in this series. >>> This is all rebased on linux-next tag 20211020. >> >> Yes, please. >> >> After this series, I guess we can remove "bool >> registered[NUM_DISK_MINORS];" like below >> due to (unit[drive].disk[type] != NULL) == >> (unit[drive].registered[type] == true). >> Regarding this series, setting unit[drive].registered[type] = true in >> ataflop_probe() is >> pointless because atari_floppy_cleanup() checks unit[i].disk[type] != >> NULL for calling >> del_gendisk(). And we need to fix __register_blkdev() in >> driver/block/floppy.c because >> floppy_probe_lock is pointless. >> >> drivers/block/ataflop.c | 75 +++++++++++++++-------------------------- >> 1 file changed, 28 insertions(+), 47 deletions(-) >> >> diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c >> index c58750dcc685..7fedf8506335 100644 >> --- a/drivers/block/ataflop.c >> +++ b/drivers/block/ataflop.c >> @@ -299,7 +299,6 @@ static struct atari_floppy_struct { >> disk change detection) */ >> int flags; /* flags */ >> struct gendisk *disk[NUM_DISK_MINORS]; >> - bool registered[NUM_DISK_MINORS]; >> int ref; >> int type; >> struct blk_mq_tag_set tag_set; >> @@ -1988,41 +1987,20 @@ static int ataflop_probe(dev_t dev) >> if (drive >= FD_MAX_UNITS || type >= NUM_DISK_MINORS) >> return -EINVAL; >> >> - if (!unit[drive].disk[type]) { >> - 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; >> + if (unit[drive].disk[type]) >> + return 0; >> + 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; >> } >> } >> >> return err; >> } >> >> -static void atari_floppy_cleanup(void) >> -{ >> - int i; >> - int 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_queue(unit[i].disk[type]->queue); >> - put_disk(unit[i].disk[type]); >> - } >> - blk_mq_free_tag_set(&unit[i].tag_set); >> - } >> - >> - del_timer_sync(&fd_timer); >> - atari_stram_free(DMABuffer); >> -} >> - >> static void atari_cleanup_floppy_disk(struct atari_floppy_struct *fs) >> { >> int type; >> @@ -2030,13 +2008,24 @@ static void atari_cleanup_floppy_disk(struct >> atari_floppy_struct *fs) >> for (type = 0; type < NUM_DISK_MINORS; type++) { >> if (!fs->disk[type]) >> continue; >> - if (fs->registered[type]) >> - del_gendisk(fs->disk[type]); >> + del_gendisk(fs->disk[type]); >> blk_cleanup_disk(fs->disk[type]); >> } >> blk_mq_free_tag_set(&fs->tag_set); >> } >> >> +static void atari_floppy_cleanup(void) >> +{ >> + int i; >> + >> + for (i = 0; i < FD_MAX_UNITS; i++) >> + atari_cleanup_floppy_disk(&unit[i]); >> + >> + del_timer_sync(&fd_timer); >> + if (DMABuffer) >> + atari_stram_free(DMABuffer); >> +} >> + >> static int __init atari_floppy_init (void) >> { >> int i; >> @@ -2055,13 +2044,10 @@ static int __init atari_floppy_init (void) >> unit[i].tag_set.numa_node = NUMA_NO_NODE; >> unit[i].tag_set.flags = BLK_MQ_F_SHOULD_MERGE; >> ret = blk_mq_alloc_tag_set(&unit[i].tag_set); >> - if (ret) >> - goto err; >> - >> - ret = ataflop_alloc_disk(i, 0); >> if (ret) { >> - blk_mq_free_tag_set(&unit[i].tag_set); >> - goto err; >> + while (--i >= 0) >> + blk_mq_free_tag_set(&unit[i].tag_set); >> + return ret; >> } >> } >> >> @@ -2090,10 +2076,9 @@ static int __init atari_floppy_init (void) >> for (i = 0; i < FD_MAX_UNITS; i++) { >> unit[i].track = -1; >> unit[i].flags = 0; >> - ret = add_disk(unit[i].disk[0]); >> - if (ret) >> - goto err_out_dma; >> - unit[i].registered[0] = true; >> + ret = ataflop_probe(MKDEV(0, 1 << 2)); >> + if (err) >> + goto err; >> } >> >> printk(KERN_INFO "Atari floppy driver: max. %cD, %strack >> buffering\n", >> @@ -2108,12 +2093,8 @@ static int __init atari_floppy_init (void) >> } >> return ret; >> >> -err_out_dma: >> - atari_stram_free(DMABuffer); >> err: >> - while (--i >= 0) >> - atari_cleanup_floppy_disk(&unit[i]); >> - >> + atari_floppy_cleanup(); >> return ret; >> } >> >>
On Fri, Oct 22, 2021 at 10:06:07AM +0900, Tetsuo Handa wrote: > On 2021/10/22 1:38, Luis Chamberlain wrote: > > I rebased Tetsuo Handa's patch onto the latest linux-next as this > > series depends on it, and so I am sending it part of this series as > > without it, this won't apply. Tetsuo, does the rebase of your patch > > look OK? > > OK, though I wanted my fix to be sent to upstream and stable before this series. Sure, absolutely, your patch is certainly separate and is needed as a fix downstream to stable it would seem. > > If it is not too much trouble, I'd like to ask for testing for the > > ataflop changes from Michael Schmitz, if possible, that is he'd just > > have to merge Tetsuo's rebased patch and the 2nd patch in this series. > > This is all rebased on linux-next tag 20211020. > > Yes, please. > > After this series, I guess we can remove "bool registered[NUM_DISK_MINORS];" like below > due to (unit[drive].disk[type] != NULL) == (unit[drive].registered[type] == true). Sounds good. > Regarding this series, setting unit[drive].registered[type] = true in ataflop_probe() is > pointless because atari_floppy_cleanup() checks unit[i].disk[type] != NULL for calling > del_gendisk(). I see, will fix. > And we need to fix __register_blkdev() in driver/block/floppy.c because > floppy_probe_lock is pointless. Sure, I take it you're going to work on a patch? So much excitement with the floppy world, who would have thought? :) Luis
On Fri, Oct 22, 2021 at 10:08:39AM -0700, Luis Chamberlain wrote: > On Fri, Oct 22, 2021 at 10:06:07AM +0900, Tetsuo Handa wrote: > > On 2021/10/22 1:38, Luis Chamberlain wrote: > > > I rebased Tetsuo Handa's patch onto the latest linux-next as this > > > series depends on it, and so I am sending it part of this series as > > > without it, this won't apply. Tetsuo, does the rebase of your patch > > > look OK? > > > > OK, though I wanted my fix to be sent to upstream and stable before this series. > > Sure, absolutely, your patch is certainly separate and is needed as a > fix downstream to stable it would seem. > > > > If it is not too much trouble, I'd like to ask for testing for the > > > ataflop changes from Michael Schmitz, if possible, that is he'd just > > > have to merge Tetsuo's rebased patch and the 2nd patch in this series. > > > This is all rebased on linux-next tag 20211020. > > > > Yes, please. > > > > After this series, I guess we can remove "bool registered[NUM_DISK_MINORS];" like below > > due to (unit[drive].disk[type] != NULL) == (unit[drive].registered[type] == true). > > Sounds good. > > > Regarding this series, setting unit[drive].registered[type] = true in ataflop_probe() is > > pointless because atari_floppy_cleanup() checks unit[i].disk[type] != NULL for calling > > del_gendisk(). > > I see, will fix. Actually just not doing it for that case seems odd, so I would prefer the removal of the bool registered to be a separate patch to make it clearer. Luis
Hi Tetsuo, On 22/10/21 14:06, Tetsuo Handa wrote: > On 2021/10/22 1:38, Luis Chamberlain wrote: >> I rebased Tetsuo Handa's patch onto the latest linux-next as this >> series depends on it, and so I am sending it part of this series as >> without it, this won't apply. Tetsuo, does the rebase of your patch >> look OK? > > OK, though I wanted my fix to be sent to upstream and stable before this series. > >> >> If it is not too much trouble, I'd like to ask for testing for the >> ataflop changes from Michael Schmitz, if possible, that is he'd just >> have to merge Tetsuo's rebased patch and the 2nd patch in this series. >> This is all rebased on linux-next tag 20211020. > > Yes, please. Took a little convincing (patch 2 didn't apply cleanly by 'git am' on yesterday's top of linux-next), but works just fine, thanks. I'll submit another patch with ataflop fixes that were used in my tests, but nothing in that interacts with your patches at all. Tested-by: Michael Schmitz <schmitzmic@gmail.com> > After this series, I guess we can remove "bool registered[NUM_DISK_MINORS];" like below > due to (unit[drive].disk[type] != NULL) == (unit[drive].registered[type] == true). > Regarding this series, setting unit[drive].registered[type] = true in ataflop_probe() is > pointless because atari_floppy_cleanup() checks unit[i].disk[type] != NULL for calling > del_gendisk(). And we need to fix __register_blkdev() in driver/block/floppy.c because > floppy_probe_lock is pointless. > > drivers/block/ataflop.c | 75 +++++++++++++++-------------------------- > 1 file changed, 28 insertions(+), 47 deletions(-) > > diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c > index c58750dcc685..7fedf8506335 100644 > --- a/drivers/block/ataflop.c > +++ b/drivers/block/ataflop.c > @@ -299,7 +299,6 @@ static struct atari_floppy_struct { > disk change detection) */ > int flags; /* flags */ > struct gendisk *disk[NUM_DISK_MINORS]; > - bool registered[NUM_DISK_MINORS]; > int ref; > int type; > struct blk_mq_tag_set tag_set; > @@ -1988,41 +1987,20 @@ static int ataflop_probe(dev_t dev) > if (drive >= FD_MAX_UNITS || type >= NUM_DISK_MINORS) > return -EINVAL; > > - if (!unit[drive].disk[type]) { > - 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; > + if (unit[drive].disk[type]) > + return 0; > + 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; > } > } > > return err; > } > > -static void atari_floppy_cleanup(void) > -{ > - int i; > - int 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_queue(unit[i].disk[type]->queue); > - put_disk(unit[i].disk[type]); > - } > - blk_mq_free_tag_set(&unit[i].tag_set); > - } > - > - del_timer_sync(&fd_timer); > - atari_stram_free(DMABuffer); > -} > - > static void atari_cleanup_floppy_disk(struct atari_floppy_struct *fs) > { > int type; > @@ -2030,13 +2008,24 @@ static void atari_cleanup_floppy_disk(struct atari_floppy_struct *fs) > for (type = 0; type < NUM_DISK_MINORS; type++) { > if (!fs->disk[type]) > continue; > - if (fs->registered[type]) > - del_gendisk(fs->disk[type]); > + del_gendisk(fs->disk[type]); > blk_cleanup_disk(fs->disk[type]); > } > blk_mq_free_tag_set(&fs->tag_set); > } > > +static void atari_floppy_cleanup(void) > +{ > + int i; > + > + for (i = 0; i < FD_MAX_UNITS; i++) > + atari_cleanup_floppy_disk(&unit[i]); > + > + del_timer_sync(&fd_timer); > + if (DMABuffer) > + atari_stram_free(DMABuffer); > +} > + > static int __init atari_floppy_init (void) > { > int i; > @@ -2055,13 +2044,10 @@ static int __init atari_floppy_init (void) > unit[i].tag_set.numa_node = NUMA_NO_NODE; > unit[i].tag_set.flags = BLK_MQ_F_SHOULD_MERGE; > ret = blk_mq_alloc_tag_set(&unit[i].tag_set); > - if (ret) > - goto err; > - > - ret = ataflop_alloc_disk(i, 0); > if (ret) { > - blk_mq_free_tag_set(&unit[i].tag_set); > - goto err; > + while (--i >= 0) > + blk_mq_free_tag_set(&unit[i].tag_set); > + return ret; > } > } > > @@ -2090,10 +2076,9 @@ static int __init atari_floppy_init (void) > for (i = 0; i < FD_MAX_UNITS; i++) { > unit[i].track = -1; > unit[i].flags = 0; > - ret = add_disk(unit[i].disk[0]); > - if (ret) > - goto err_out_dma; > - unit[i].registered[0] = true; > + ret = ataflop_probe(MKDEV(0, 1 << 2)); > + if (err) > + goto err; > } > > printk(KERN_INFO "Atari floppy driver: max. %cD, %strack buffering\n", > @@ -2108,12 +2093,8 @@ static int __init atari_floppy_init (void) > } > return ret; > > -err_out_dma: > - atari_stram_free(DMABuffer); > err: > - while (--i >= 0) > - atari_cleanup_floppy_disk(&unit[i]); > - > + atari_floppy_cleanup(); > return ret; > } > >
This is v3 series of the last patch set which should be considered once nvdimm/blk driver is removed, as Dan Williams noted it would be gone and once *all* add_disk() error handling patches have been merged. I rebased Tetsuo Handa's patch onto the latest linux-next as this series depends on it, and so I am sending it part of this series as without it, this won't apply. Tetsuo, does the rebase of your patch look OK? If it is not too much trouble, I'd like to ask for testing for the ataflop changes from Michael Schmitz, if possible, that is he'd just have to merge Tetsuo's rebased patch and the 2nd patch in this series. This is all rebased on linux-next tag 20211020. Changes in this v3: - we don't set ataflop registered to true when we fail, an issue spotted during review by Tetsuo - rebased to take into consideration Tetsuo's changes, both his old and the latest patch carried in this series - sets the floppy to null on failure from add_disk(), an issue spotted by Tetsuo during patch review - removes out label from ataflop as suggested by Finn Thain - we return the failure from floppy_alloc_disk as suggested by Finn Thain Luis Chamberlain (2): block: make __register_blkdev() return an error block: add __must_check for *add_disk*() callers Tetsuo Handa (1): ataflop: remove ataflop_probe_lock mutex block/bdev.c | 5 +++- block/genhd.c | 27 +++++++++++------ drivers/block/ataflop.c | 66 +++++++++++++++++++++++++---------------- drivers/block/brd.c | 7 +++-- drivers/block/floppy.c | 17 ++++++++--- drivers/block/loop.c | 11 +++++-- drivers/md/md.c | 12 ++++++-- drivers/scsi/sd.c | 3 +- include/linux/genhd.h | 10 +++---- 9 files changed, 105 insertions(+), 53 deletions(-)