On 3/27/20 10:31 AM, Bijan Mottahedeh wrote:
> Does io_uring though have to deal with BLK_QC_T_NONE at all? Or are you
> saying that it should never receive that result?
> That's one of the things I'm not clear about.
BLK_QC_T_* are block cookies, they are only valid in the block layer.
Only the poll handler called should have to deal with them, inside
their f_op->iopoll() handler. It's simply passed from the queue to
the poll side.
So no, io_uring shouldn't have to deal with them at all.
The problem, as I see it, is if the block layer returns BLK_QC_T_NONE
and the IO was actually queued and requires polling to be found. We'd
end up with IO timeouts for handling those requests, and that's not a
good thing...
>> On 3/26/20 8:57 PM, Bijan Mottahedeh wrote:
>>> I'm seeing poll threads hang as I increase the number of threads in
>>> polled fio tests. I think this is because of polling on BLK_QC_T_NONE
>>> cookie, which will never succeed.
>>>
>>> A related problem however, is that the meaning of BLK_QC_T_NONE seems to
>>> be ambiguous.
>>>
>>> Specifically, the following cases return BLK_QC_T_NONE which I think
>>> would be problematic for polled io:
>>>
>>>
>>> generic_make_request()
>>> ...
>>> if (current->bio_list) {
>>> bio_list_add(¤t->bio_list[0], bio);
>>> goto out;
>>> }
>>>
>>> In this case the request is delayed but should get a cookie eventually.
>>> How does the caller know what the right action is in this case for a
>>> polled request? Polling would never succeed.
>>>
>>>
>>> __blk_mq_issue_directly()
>>> ...
>>> case BLK_STS_RESOURCE:
>>> case BLK_STS_DEV_RESOURCE:
>>> blk_mq_update_dispatch_busy(hctx, true);
>>> __blk_mq_requeue_request(rq);
>>> break;
>>>
>>> In this case, cookie is not updated and would keep its default
>>> BLK_QC_T_NONE value from blk_mq_make_request(). However, this request
>>> will eventually be reissued, so again, how would the caller poll for the
>>> completion of this request?
>>>
>>> blk_mq_try_issue_directly()
>>> ...
>>> ret = __blk_mq_try_issue_directly(hctx, rq, cookie, false, true);
>>> if (ret == BLK_STS_RESOURCE || ret == BLK_STS_DEV_RESOURCE)
>>> blk_mq_request_bypass_insert(rq, false, true);
>>>
>>> Am I missing something here?
>>>
>>> Incidentally, I don't see BLK_QC_T_EAGAIN used anywhere, should it be?
Pretty sure that's a leftover from when the attempts was made to pass
back -EAGAIN inline instead of through the bio end_io handler.
--
Jens Axboe
