Re: [for-4.16 PATCH v4 2/4] block: use queue_lock when clearing QUEUE_FLAG_REGISTERED in blk_unregister_queue

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

 



On Thu, Jan 11 2018 at  7:28pm -0500,
Bart Van Assche <Bart.VanAssche@xxxxxxx> wrote:

> On Thu, 2018-01-11 at 15:14 -0500, Mike Snitzer wrote:
> > blk_unregister_queue() must protect against any modifications of
> > q->queue_flags (not just those performed in blk-sysfs.c).  Therefore
> > q->queue_lock needs to be used rather than q->sysfs_lock.
> > 
> > Fixes: e9a823fb34a8b ("block: fix warning when I/O elevator is changed as request_queue is being removed")
> > Cc: stable@xxxxxxxxxxxxxxx # 4.14+
> > Reported-by: Bart Van Assche <Bart.VanAssche@xxxxxxx>
> > Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx>
> > ---
> >  block/blk-sysfs.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
> > index 870484eaed1f..52f57539f1c7 100644
> > --- a/block/blk-sysfs.c
> > +++ b/block/blk-sysfs.c
> > @@ -929,9 +929,9 @@ void blk_unregister_queue(struct gendisk *disk)
> >  	if (WARN_ON(!q))
> >  		return;
> >  
> > -	mutex_lock(&q->sysfs_lock);
> > +	spin_lock_irq(q->queue_lock);
> >  	queue_flag_clear_unlocked(QUEUE_FLAG_REGISTERED, q);
> > -	mutex_unlock(&q->sysfs_lock);
> > +	spin_unlock_irq(q->queue_lock);
> 
> Hello Mike,
> 
> The function name queue_flag_clear_unlocked() means "clear a queue flag
> without holding the queue lock". So at least to me the above code is confusing.
> Please consider to change queue_flag_clear_unlocked() into queue_flag_clear().

If Jens would like to change it when applying the patch to his tree
that is fine by me.

But as you know, it doesn't matter:
queue_flag_clear() just has extra queue_lockdep_assert_held(q);

So I see no reason to respin this patch for this.  Especially when you
consider patch 3 replaces it with queue_flag_test_and_clear() -- and no
it isn't a problem for stable@ to carry on using
queue_flag_clear_unlocked

Mike



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux