From: Vivek Goyal <vgoyal@redhat.com>
To: Jens Axboe <jaxboe@fusionio.com>
Cc: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>,
"jmarchan@redhat.com" <jmarchan@redhat.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] Don't merge different partition's IOs
Date: Wed, 8 Dec 2010 10:58:31 -0500 [thread overview]
Message-ID: <20101208155831.GF31703@redhat.com> (raw)
In-Reply-To: <20101208155137.GE31703@redhat.com>
On Wed, Dec 08, 2010 at 10:51:37AM -0500, Vivek Goyal wrote:
> On Wed, Dec 08, 2010 at 10:46:04PM +0800, Jens Axboe wrote:
> > On 2010-12-08 16:11, Satoru Takeuchi wrote:
> > > Hi Jens,
> > >
> > > (2010/12/08 17:06), Jens Axboe wrote:
> > >>>>> I hit on another approach. Although it doesn'tprevent any merge as Linus
> > >>>>> preferred, it can fix the problem anyway. In this idea, in_flight is
> > >>>>> incremented and decremented for the partition which the request belonged
> > >>>>> to in its creation. It has the following merits.
> > >>>
> > >>> Revert is already finished. 2.6.37-rc-5 and latest stable kernel doesn't
> > >>> contain Yasuaki's former logic.
> > >>>
> > >>> https://lkml.org/lkml/2010/10/24/118
> > >>
> > >> Yes I know, that is why I said:
> > >>
> > >>>> I really would prefer if we fixed up the patchset we ended up reverting..
> > >>>> At least that had a purpose with growing struct request, since we saved
> > >>>> on doing the partition lookups.
> > >>
> > >> That I prefer we fix that code up, since I think it's the best solution
> > >> to the problem.
> > >>
> > >
> > > I already postedit.
> > >
> > > https://lkml.org/lkml/2010/12/8/12
> > >
> > > I think it is OK without mail subject :-)
> >
> > No, that's not it at all. What I mean (and what was reverted) was
> > caching the partition lookup, and using that for the stats. The problem
> > with that approach turned out to be the elevator queiscing logic not
> > being fully correct. One easier way to fix that would be to reference
> > count the part stats, instead of having to drain the queue.
>
> Taking reference to hd_struct and storing it in rq, will definitely save
> us 1 lookup while doing accounting on completion path. It does not save
> on rq size though.
>
> IIUC, current patch does not increase the number of existing lookups. So
> current situation does not deteriorate with the patch.
>
> But storing a reference in rq and avoiding 1 lookup in completion path
> definitely sounds better.
>
Storing a pointer to partition in rq also got the advantage that we can
easily not allow merging of requests across partitions for better
accounting.
Satoru, so yes, if you can implement what jens is suggesting, would be
good.
Thanks
Vivek
next prev parent reply other threads:[~2010-12-08 16:37 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-06 9:44 [PATCH 1/2] Don't merge different partition's IOs Yasuaki Ishimatsu
2010-12-06 16:08 ` Linus Torvalds
2010-12-07 7:18 ` Satoru Takeuchi
2010-12-07 18:39 ` Vivek Goyal
2010-12-08 7:33 ` Jens Axboe
2010-12-08 7:59 ` Satoru Takeuchi
2010-12-08 8:06 ` Jens Axboe
2010-12-08 8:11 ` Satoru Takeuchi
2010-12-08 14:46 ` Jens Axboe
2010-12-08 15:51 ` Vivek Goyal
2010-12-08 15:58 ` Vivek Goyal [this message]
2010-12-10 11:22 ` Jerome Marchand
2010-12-10 16:12 ` Jerome Marchand
2010-12-10 16:55 ` Vivek Goyal
2010-12-14 20:25 ` Jens Axboe
2010-12-17 13:42 ` [PATCH] block: fix accounting bug on cross partition merges Jerome Marchand
2010-12-17 19:06 ` Jens Axboe
2010-12-17 22:32 ` Vivek Goyal
2010-12-23 15:10 ` Jerome Marchand
2010-12-23 15:39 ` Vivek Goyal
2010-12-23 17:04 ` Jerome Marchand
2010-12-24 19:29 ` Vivek Goyal
2011-01-04 15:52 ` [PATCH 1/2] kref: add kref_test_and_get Jerome Marchand
2011-01-04 15:55 ` [PATCH 2/2] block: fix accounting bug on cross partition merges Jerome Marchand
2011-01-04 21:00 ` Greg KH
2011-01-05 13:51 ` Jerome Marchand
2011-01-05 16:00 ` Greg KH
2011-01-05 16:19 ` Jerome Marchand
2011-01-05 16:27 ` Greg KH
2011-01-05 13:55 ` Jens Axboe
2011-01-05 15:58 ` Greg KH
2011-01-05 18:46 ` Jens Axboe
2011-01-05 20:08 ` Greg KH
2011-01-05 21:38 ` Jens Axboe
2011-01-05 22:16 ` Greg KH
2011-01-06 9:46 ` Jens Axboe
2011-01-05 14:00 ` Jens Axboe
2011-01-05 14:09 ` Jerome Marchand
2011-01-05 14:17 ` Jens Axboe
2011-01-04 16:05 ` [PATCH 1/2] kref: add kref_test_and_get Eric Dumazet
2011-01-05 15:02 ` [PATCH 1/2 v2] " Jerome Marchand
2011-01-05 15:43 ` Alexey Dobriyan
2011-01-05 15:57 ` Greg KH
2011-01-05 15:56 ` Greg KH
2011-01-04 20:57 ` [PATCH 1/2] " Greg KH
2011-01-05 13:35 ` Jerome Marchand
2011-01-05 15:55 ` Greg KH
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20101208155831.GF31703@redhat.com \
--to=vgoyal@redhat.com \
--cc=isimatu.yasuaki@jp.fujitsu.com \
--cc=jaxboe@fusionio.com \
--cc=jmarchan@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=takeuchi_satoru@jp.fujitsu.com \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox