Re: [PATCH] blk-mq: Return invalid cookie if bio was split

On Tue, Oct 4, 2016 at 6:00 AM, Keith Busch [off-list ref] wrote:

On Tue, Sep 27, 2016 at 05:25:36PM +0800, Ming Lei wrote:

quoted

On Mon, 26 Sep 2016 19:00:30 -0400
Keith Busch [off-list ref] wrote:

quoted

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?

But .poll() need to check if the specific request is completed or not,
then blk_poll() can set 'current' as RUNNING if it is completed.

blk_poll():
                ...
                ret = q->mq_ops->poll(hctx, blk_qc_t_to_tag(cookie));
                if (ret > 0) {
                        hctx->poll_success++;
                        set_current_state(TASK_RUNNING);
                        return true;
                }
                ...

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.

I am glad to take a look the patch if you post it out.

Thanks,
Ming Lei