Re: [PATCH 1/4] block: check for GENHD_FL_UP in del_gendisk()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2017-12-13 at 08:36 +0100, Hannes Reinecke wrote:
> On 12/12/2017 05:57 PM, Bart Van Assche wrote:
> > On Tue, 2017-12-12 at 09:57 +0100, Hannes Reinecke wrote:
> > > From: Hannes Reinecke <hare@xxxxxxxx>
> > > 
> > > When a device is probed asynchronously del_gendisk() might be called
> > > before the async probing was run, causing del_gendisk() to crash
> > > due to uninitialized sysfs objects.
> > > 
> > > Signed-off-by: Hannes Reinecke <hare@xxxxxxxx>
> > > ---
> > >  block/genhd.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/block/genhd.c b/block/genhd.c
> > > index c2223f1..cc40d95 100644
> > > --- a/block/genhd.c
> > > +++ b/block/genhd.c
> > > @@ -697,6 +697,9 @@ void del_gendisk(struct gendisk *disk)
> > >  	struct disk_part_iter piter;
> > >  	struct hd_struct *part;
> > >  
> > > +	if (!(disk->flags & GENHD_FL_UP))
> > > +		return;
> > > +
> > >  	blk_integrity_del(disk);
> > >  	disk_del_events(disk);
> > 
> > Hello Hannes,
> > 
> > Thank you for having published your approach for increasing disk probing
> > concurrency. Your approach looks interesting to me. However, I don't think
> > that patches 1/4..3/4 are sufficient to avoid races between e.g.
> > device_add_disk() and del_gendisk(). As far as I know no locks are held
> > around the device_add_disk() and del_gendisk() calls. Does that mean that
> > del_gendisk() can call e.g. blk_integrity_del() before device_add_disk() has
> > called blk_integrity_add()?
> 
> In principle, yes. However, the overall idea here is that device_add_disk()
> and del_gendisk() are enclosed within upper layer procedures, which themselves
> provide additional locking. In our case the sd driver provided synchronisation
> guarantees ensuring that device_add_disk() and del_gendisk() doesn't run
> concurrently.
> 
> if one is really concerned we could convert disk->flags to a bitmask, and use
> atomic bitmask modification; that should avoid any concurrency issues.

Hello Hannes,

Regarding the scenario explained in a previous e-mail: what guarantees that the
device_add_disk() call in sd_probe_async() does not happen concurrently with the
device_unregister() call from __scsi_remove_device()?

Thanks,

Bart.




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux