lkml.org 
[lkml]   [2018]   [Dec]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH V11 0/4] blk-mq: refactor code of issue directly
    From
    Date


    On 12/7/18 11:42 AM, Jens Axboe wrote:
    > On 12/6/18 8:41 PM, jianchao.wang wrote:
    >>
    >>
    >> On 12/7/18 11:34 AM, Jens Axboe wrote:
    >>> On 12/6/18 8:32 PM, Jens Axboe wrote:
    >>>> On 12/6/18 8:26 PM, jianchao.wang wrote:
    >>>>>
    >>>>>
    >>>>> On 12/7/18 11:16 AM, Jens Axboe wrote:
    >>>>>> On 12/6/18 8:09 PM, Jianchao Wang wrote:
    >>>>>>> Hi Jens
    >>>>>>>
    >>>>>>> Please consider this patchset for 4.21.
    >>>>>>>
    >>>>>>> It refactors the code of issue request directly to unify the interface
    >>>>>>> and make the code clearer and more readable.
    >>>>>>>
    >>>>>>> This patch set is rebased on the recent for-4.21/block and add the 1st
    >>>>>>> patch which inserts the non-read-write request to hctx dispatch
    >>>>>>> list to avoid to involve merge and io scheduler when bypass_insert
    >>>>>>> is true, otherwise, inserting is ignored, BLK_STS_RESOURCE is returned
    >>>>>>> and the caller will fail forever.
    >>>>>>>
    >>>>>>> The 2nd patch refactors the code of issue request directly to unify the
    >>>>>>> helper interface which could handle all the cases.
    >>>>>>>
    >>>>>>> The 3rd patch make blk_mq_sched_insert_requests issue requests directly
    >>>>>>> with 'bypass' false, then it needn't to handle the non-issued requests
    >>>>>>> any more.
    >>>>>>>
    >>>>>>> The 4th patch replace and kill the blk_mq_request_issue_directly.
    >>>>>>
    >>>>>> Sorry to keep iterating on this, but let's default to inserting to
    >>>>>> the dispatch list if we ever see busy from a direct dispatch. I'm fine
    >>>>>> with doing that for 4.21, as suggested by Ming, I just didn't want to
    >>>>>> fiddle with it for 4.20. This will prevent any merging on the request
    >>>>>> going forward, which I think is a much safer default.
    >>>>>>
    >>>>>> You do this already for some cases. Let's do it unconditionally for
    >>>>>> a request that was ever subjected to ->queue_rq() and we didn't either
    >>>>>> error or finish after the fact.
    >>>>>>
    >>>>> I have done it in this version if I get your point correctly.
    >>>>> Please refer to the following fragment in the 2nd patch.
    >>>>>
    >>>>> + /*
    >>>>> + * If the request is issued unsuccessfully with
    >>>>> + * BLK_STS_DEV_RESOURCE or BLK_STS_RESOURCE, insert
    >>>>> + * the request to hctx dispatch list due to attached
    >>>>> + * lldd resource.
    >>>>> + */
    >>>>> + force = true;
    >>>>> + ret = __blk_mq_issue_directly(hctx, rq, cookie, last);
    >>>>> +out_unlock:
    >>>>> + hctx_unlock(hctx, srcu_idx);
    >>>>> +out:
    >>>>> + switch (ret) {
    >>>>> + case BLK_STS_OK:
    >>>>> + break;
    >>>>> + case BLK_STS_DEV_RESOURCE:
    >>>>> + case BLK_STS_RESOURCE:
    >>>>> + if (force) {
    >>>>> + blk_mq_request_bypass_insert(rq, run_queue);
    >>>>> + ret = bypass ? BLK_STS_OK : ret;
    >>>>> + } else if (!bypass) {
    >>>>> + blk_mq_sched_insert_request(rq, false,
    >>>>> + run_queue, false);
    >>>>> + }
    >>>>> + break;
    >>>>> + default:
    >>>>
    >>>> You are right, I missed that you set force = true before doing the
    >>>> issue. So this looks good to me!
    >>>
    >>> I applied your series. With this, we should be good to remove the
    >>> REQ_NOMERGE logic that was added for the corruption case, and the
    >>> blk_rq_can_direct_dispatch() as well?
    >>>
    >>
    >> Yes, it should be that.
    >> Every thing rejected by .queue_rq is ended or inserted into hctx dispatch
    >> list. And also direct-issue path is unified with normal path.
    >
    > Why are we doing that return value dance, depending on whether this
    > is a bypass insert or not? That seems confusing.
    >

    For the 'bypass == false' case, it need to know whether the request is issued
    successfully. This is for the 3rd patch.
    I used to use the returned cookie to identify the result, but you don't like it.
    So I have to use this return value.

    Thanks
    Jianchao

    \
     
     \ /
      Last update: 2018-12-07 04:47    [W:2.663 / U:0.088 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site