Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935746Ab1ETTVI (ORCPT ); Fri, 20 May 2011 15:21:08 -0400 Received: from 0122700014.0.fullrate.dk ([95.166.99.235]:38998 "EHLO kernel.dk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935701Ab1ETTVF (ORCPT ); Fri, 20 May 2011 15:21:05 -0400 Message-ID: <4DD6BF1F.7010403@kernel.dk> Date: Fri, 20 May 2011 21:21:03 +0200 From: Jens Axboe MIME-Version: 1.0 To: Vivek Goyal CC: Namhyung Kim , Shaohua Li , linux-kernel@vger.kernel.org, Divyesh Shah Subject: Re: [PATCH] block: call elv_bio_merged() when merged References: <1305869337-4375-1-git-send-email-namhyung@gmail.com> <1305885108.1633.11.camel@leonhard> <4DD6B807.6040309@kernel.dk> <20110520191907.GA6379@redhat.com> In-Reply-To: <20110520191907.GA6379@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2953 Lines: 78 On 2011-05-20 21:19, Vivek Goyal wrote: > On Fri, May 20, 2011 at 08:50:47PM +0200, Jens Axboe wrote: >> On 2011-05-20 11:51, Namhyung Kim wrote: >>> Hello, >>> >>> 2011-05-20 (금), 16:31 +0800, Shaohua Li: >>>> 2011/5/20 Namhyung Kim : >>>>> 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 >>>>> Cc: Divyesh Shah >>>>> --- >>>>> 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. > > I think wer can make MERGED per cpu as I have done in my other patch > series for following dispatch stats. > > BLKIO_STAT_CPU_SECTORS, > /* Total bytes transferred */ > BLKIO_STAT_CPU_SERVICE_BYTES, > /* Total IOs serviced, post merge */ > BLKIO_STAT_CPU_SERVICED, Yep, lets please do that and we can re-instate the merge calls with that, too. > Jens are you planning to merge lockless throttling series? Once that gets > merged, we can convert this MERGED stat also per cpu and get rid of need of > taking blkg->stats_lock. Merged them about an hour ago :-) Send further updates relative to for-2.6.40/core. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/