On Tue, Oct 4, 2016 at 6:00 AM, Keith Busch <keith.busch@xxxxxxxxx> wrote:
> 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@xxxxxxxxx> 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?
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
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
