lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 22 Oct 2021 20:21:26 +1300
From:   Michael Schmitz <schmitzmic@...il.com>
To:     Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        Luis Chamberlain <mcgrof@...nel.org>
Cc:     linux-raid@...r.kernel.org, linux-scsi@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org, axboe@...nel.dk, hch@....de,
        efremov@...ux.com, song@...nel.org, jejb@...ux.ibm.com,
        martin.petersen@...cle.com, viro@...iv.linux.org.uk, hare@...e.de,
        jack@...e.cz, ming.lei@...hat.com, tj@...nel.org
Subject: Re: [PATCH v3 0/3] last batch of add_disk() error handling
 conversions

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;
>>  }
>>
>>

Powered by blists - more mailing lists