linux-kernel - Re: [PATCH] block: call elv_bio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 20 May 2011 20:50:47 +0200
From:	Jens Axboe <axboe@...nel.dk>
To:	Namhyung Kim <namhyung@...il.com>
CC:	Shaohua Li <shaohua.li@...el.com>, linux-kernel@...r.kernel.org,
	Divyesh Shah <dpshah@...gle.com>
Subject: Re: [PATCH] block: call elv_bio_merged() when merged

On 2011-05-20 11:51, Namhyung Kim wrote:
> Hello,
> 
> 2011-05-20 (금), 16:31 +0800, Shaohua Li:
>> 2011/5/20 Namhyung Kim <namhyung@...il.com>:
>>> Commit 73c101011926 ("block: initial patch for on-stack per-task plugging")
>>> removed calls to elv_bio_merged() when @bio merged with @req. Re-add them.
>>>
>>> Signed-off-by: Namhyung Kim <namhyung@...il.com>
>>> Cc: Divyesh Shah <dpshah@...gle.com>
>>> ---
>>>  block/blk-core.c |    2 ++
>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/block/blk-core.c b/block/blk-core.c
>>> index 3fe00a14822a..4dc02ef5fc82 100644
>>> --- a/block/blk-core.c
>>> +++ b/block/blk-core.c
>>> @@ -1132,6 +1132,7 @@ static bool bio_attempt_back_merge(struct request_queue *q, struct request *req,
>>>        req->ioprio = ioprio_best(req->ioprio, bio_prio(bio));
>>>
>>>        drive_stat_acct(req, 0);
>>> +       elv_bio_merged(q, req, bio);
>>>        return true;
>>>  }
>>>
>>> @@ -1173,6 +1174,7 @@ static bool bio_attempt_front_merge(struct request_queue *q,
>>>        req->ioprio = ioprio_best(req->ioprio, bio_prio(bio));
>>>
>>>        drive_stat_acct(req, 0);
>>> +       elv_bio_merged(q, req, bio);
>>>        return true;
>>>  }
>> Looks you should do this in __make_request. when the routine is called
>> in attempt_plug_merge, the request isn't added to elevator yet.
>>
> 
> Hmm.. anyway it is merged. Is there any reason why we shouldn't collect
> the stat - or invoke the callback routine - if the @req is not in the
> elevator? Or we need to add a separate stat item for this case?

Your patch should be safe, it's essentially only for the cgroup stuff
that does its own accounting and has appropriate protection for it. We'd
want to do this for both the plug and not-plugged merge case.

It's a bit of a shame to add this though, since now we are hitting the
cgroup lock for each merge.

-- 
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/