On Wed, Aug 23, 2017 at 11:34 AM, Ming Lei <tom.leiming@xxxxxxxxx> wrote: > On Wed, Aug 9, 2017 at 9:44 AM, Ming Lei <tom.leiming@xxxxxxxxx> wrote: >> Hi David, >> >> On Wed, Aug 9, 2017 at 2:13 AM, David Jeffery <djeffery@xxxxxxxxxx> wrote: >>> On 08/07/2017 07:53 PM, Ming Lei wrote: >>>> On Tue, Aug 8, 2017 at 3:38 AM, David Jeffery <djeffery@xxxxxxxxxx> wrote: >>> >>>>> >>>>> Signed-off-by: David Jeffery <djeffery@xxxxxxxxxx> >>>>> --- >>>>> block/blk-sysfs.c | 2 ++ >>>>> block/elevator.c | 4 ++++ >>>>> 2 files changed, 6 insertions(+) >>>>> >>>>> >>>>> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c >>>>> index 27aceab..b8362c0 100644 >>>>> --- a/block/blk-sysfs.c >>>>> +++ b/block/blk-sysfs.c >>>>> @@ -931,7 +931,9 @@ void blk_unregister_queue(struct gendisk *disk) >>>>> if (WARN_ON(!q)) >>>>> return; >>>>> >>>>> + mutex_lock(&q->sysfs_lock); >>>>> queue_flag_clear_unlocked(QUEUE_FLAG_REGISTERED, q); >>>>> + mutex_unlock(&q->sysfs_lock); >>>> >>>> Could you share why the lock of 'q->sysfs_lock' is needed here? >>> >>> As the elevator change is initiated through a sysfs attr file in the >>> queue directory, the task doing the elevator change already acquires the >>> q->sysfs_lock before it can try and change the elevator. Adding the >> >> It is e->sysfs_lock which is held in elv_attr_store(), instead of q->sysfs_lock. > > Looks I was wrong, and the store is from queue_attr_store() instead of > elv_attr_store(). > > I can reproduce the issue and this patch can fix the issue in my test > on scsi_debug, > so: > > Tested-by: Ming Lei <ming.lei@xxxxxxxxxx> > > And it is a typical race between removing queue kobj and adding children of > this queue kobj, we can't acquire q->sysfs_lock in blk_unregister_queue() > because of sysfs's limit(may cause deadlock), so one state has to be used > for this purpose, just like what register/unregister hctx kobjs does, > and it should > be fine to use QUEUE_FLAG_REGISTERED here. More changes are required if > we use e->registered, so this patch looks fine: > > Reviewed-by: Ming Lei <ming.lei@xxxxxxxxxx> Hi Jens, Could you consider this patch for v4.13 or v4.14? Thanks, Ming Lei
