linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Keith Busch <keith.busch@intel.com>
To: Ming Lei <tom.leiming@gmail.com>
Cc: linux-block@vger.kernel.org, Jens Axboe <axboe@fb.com>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH] blk-mq: Return invalid cookie if bio was split
Date: Mon, 3 Oct 2016 18:00:41 -0400	[thread overview]
Message-ID: <20161003220040.GA1259@localhost.localdomain> (raw)
In-Reply-To: <20160927172536.53a1315f@tom-ThinkPad-T450>

On Tue, Sep 27, 2016 at 05:25:36PM +0800, Ming Lei wrote:
> On Mon, 26 Sep 2016 19:00:30 -0400
> Keith Busch <keith.busch@intel.com> wrote:
> 
> > The only user of polling requires its original request be completed in
> > its entirety before continuing execution. If the bio needs to be split
> > and chained for any reason, the direct IO path would have waited for just
> > that split portion to complete, leading to potential data corruption if
> > the remaining transfer has not yet completed.
> 
> The issue looks a bit tricky because there is no per-bio place for holding
> the cookie, and generic_make_request() only returns the cookie for the
> last bio in the current bio list, so maybe we need the following patch too.

I'm looking more into this, and I can't see why we're returning a cookie
to poll on in the first place. blk_poll is only invoked when we could have
called io_schedule, so we expect the task state gets set to TASK_RUNNING
when all the work completes. So why do we need to poll for a specific
tag instead of polling until task state is set back to running?

I've tried this out and it seems to work fine, and should fix any issues
from split IO requests. It also helps direct IO polling, since it can
have a list of bios, but can only save one cookie.

  parent reply	other threads:[~2016-10-03 21:49 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-26 23:00 [PATCH] blk-mq: Return invalid cookie if bio was split Keith Busch
2016-09-27  9:25 ` Ming Lei
2016-09-27 18:24   ` Keith Busch
2016-10-03 22:00   ` Keith Busch [this message]
2016-10-05  3:19     ` Ming Lei
2016-10-05 16:51       ` Keith Busch
2016-10-06 16:06         ` Ming Lei
2016-10-06 16:39           ` Keith Busch

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161003220040.GA1259@localhost.localdomain \
    --to=keith.busch@intel.com \
    --cc=axboe@fb.com \
    --cc=hch@lst.de \
    --cc=linux-block@vger.kernel.org \
    --cc=tom.leiming@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).