Re: blk-mq request allocation stalls

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

 



On Tue, Jan 13 2015 at 10:11am -0500,
Keith Busch <keith.busch@xxxxxxxxx> wrote:

> On Tue, 13 Jan 2015, Jens Axboe wrote:
> >On 01/12/2015 11:35 AM, Keith Busch wrote:
> >>On Mon, 12 Jan 2015, Keith Busch wrote:
> >>>Oh, let's look at "__blk_rq_prep_clone". dm calls that after
> >>>blk_get_request() for the blk-mq based multipath types and overrides the
> >>>destinations cmd_flags with the source's even though the source was not
> >>>allocated from a blk-mq based queue, much less a shared tag.
> >>
> >>Untested patch. This will also preserve the failfast cmd_flag dm-mpath
> >>set after allocating.
> >>
> >>---
> >>diff --git a/block/blk-core.c b/block/blk-core.c
> >>index 7e78931..6201090 100644
> >>--- a/block/blk-core.c
> >>+++ b/block/blk-core.c
> >>@@ -2895,7 +2895,10 @@ EXPORT_SYMBOL_GPL(blk_rq_unprep_clone);
> >>  static void __blk_rq_prep_clone(struct request *dst, struct request *src)
> >>  {
> >>      dst->cpu = src->cpu;
> >>-    dst->cmd_flags = (src->cmd_flags & REQ_CLONE_MASK) | REQ_NOMERGE;
> >>+    if (dst->q->mq_ops)
> >>+        dst->cmd_flags |= (src->cmd_flags & REQ_CLONE_MASK) | REQ_NOMERGE;
> >>+    else
> >>+        dst->cmd_flags = (src->cmd_flags & REQ_CLONE_MASK) | REQ_NOMERGE;
> >
> >Making the two cases different is a bit... nonsensical. We should
> >do this for both cases, if safe, or move the MQ_INFLIGHT flag and
> >expand the CLONE_MASK.
> 
> Expanding the clone mask won't do any good since the src doesn't come
> from blk-mq and wouldn't ever have MQ_INFLIGHT set. Blk-mq initializes
> the cmd_flags when you get one so I assumed OR'ing was safe. For the
> non-blk-mq case, we have no guarantees how the req was initialized and
> could have nonsense cmd_flags.

It _could_ but the only consumer of blk_rq_prep_clone() is request-based
DM, and the non-blk-mq case uses blk_rq_init() prior to calling
blk_rq_prep_clone() so the cmd_flags will be 0.

I've revised the patch to just do it for both cases since it will be
safe, see:
https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-for-3.20-blk-mq&id=6a55c9861326dc2a731c7978d93567dd4e62d2f7

--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel



[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux