On Thu, Sep 01 2016 at 10:23am -0400,
Bart Van Assche <Bart.VanAssche@xxxxxxxxxxx> wrote:
> On 08/31/16 20:14, Mike Snitzer wrote:
> > On Wed, Aug 31 2016 at 6:18pm -0400,
> > Bart Van Assche <bart.vanassche@xxxxxxxxxxx> wrote:
> >
> >> Ensure that all ongoing dm_mq_queue_rq() and dm_mq_requeue_request()
> >> calls have stopped before setting the "queue stopped" flag. This
> >> allows to remove the "queue stopped" test from dm_mq_queue_rq() and
> >> dm_mq_requeue_request(). Use BLK_MQ_S_STOPPED instead of
> >> QUEUE_FLAG_STOPPED.
> >
> > At first glance, at a minimum this patch needs a better header. It
> > seems you're doing 2 things:
> >
> > 1) using blk_mq_{freeze,unfreeze}_queue() actually makes dm_stop_queue()
> > work for blk-mq? Whereby fixing blk-mq race(s)?
> >
> > 2) switching away from QUEUE_FLAG_STOPPED to BLK_MQ_S_STOPPED (via
> > blk_mq_queue_stopped)
> > - not clear to me that dm-mq's use of QUEUE_FLAG_STOPPED wasn't fine;
> > NVMe also uses it for blk-mq
>
> Hello Mike,
>
> Adding the blk_mq_{freeze,unfreeze}_queue() calls is indeed what fixes
> the race conditions related to stopping dm queues and what makes
> dm_stop_queue() work.
OK, thanks for confirming.
> If other blk-mq users and developers agree that QUEUE_FLAG_STOPPED
> should be set for stopped blk-mq queues then I think the code to set
> that flag should be moved into the blk-mq core. However, setting that
> flag for blk-mq queues seems redundant to me. Hence my proposal to
> introduce blk_mq_queue_stopped() instead.
This is a secondary issue that can be dealt with independent of the rest
of your DM patchset.
I've staged most of your changes (with slight tweaks), see:
https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/log/?h=dm-4.9
Only remaining issue is the queue dying race(s) in dm-multipath.
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
