Re: [for-4.16 PATCH v3 2/3] block: allow gendisk's request_queue registration to be deferred

[Mike Snitzer <snitzer@redhat.com>]

On Thu, Jan 11 2018 at 12:47pm -0500,
Bart Van Assche <Bart.VanAssche@wdc.com> wrote:

> On Thu, 2018-01-11 at 12:29 -0500, Mike Snitzer wrote:
> > On Thu, Jan 11 2018 at 12:18pm -0500,
> > Bart Van Assche <Bart.VanAssche@wdc.com> wrote:
> > 
> > > On Thu, 2018-01-11 at 12:04 -0500, Mike Snitzer wrote:
> > > > So my v4, that I'll send out shortly, won't be using test_and_clear_bit()
> > > 
> > > Please use queue_flag_set(), queue_flag_clear(), queue_flag_test_and_clear() and/or
> > > queue_flag_test_and_set() to manipulate queue flags.
> > 
> > Can you please expand on this?  My patch is only using test_bit().
> 
> Hello Mike,
> 
> I was referring to the following code, which apparenly is existing code:
> 
>         mutex_lock(&q->sysfs_lock);
>         queue_flag_clear_unlocked(QUEUE_FLAG_REGISTERED, q);
>         mutex_unlock(&q->sysfs_lock);
> 
> The above code is wrong. Other code that changes the queue flags protects
> these changes with the the queue lock. The above code should be changed into
> the following:
> 
> 	spin_lock_irq(q->queue_lock);
> 	queue_flag_clear(QUEUE_FLAG_REGISTERED, q);
> 	spin_unlock_irq(q->queue_lock);
> 
> The only functions from which it is safe to call queue_flag_(set|clear)_unlocked()
> without holding the queue lock are blk_alloc_queue_node() and
> __blk_release_queue() because for these functions it is guaranteed that no queue
> flag changes can happen from another context, e.g. through a blk_set_queue_dying()
> call.

Yes, I noticed the use of sysfs_lock also.  I've fixed it up earlier in
my v4 patchset and build on it.  I'll add your Reported-by too.

Mike