All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <jens.axboe@oracle.com>
To: Shaohua Li <shaohua.li@intel.com>
Cc: Corrado Zoccolo <czoccolo@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"vgoyal@redhat.com" <vgoyal@redhat.com>,
	"jmoyer@redhat.com" <jmoyer@redhat.com>,
	"guijianfeng@cn.fujitsu.com" <guijianfeng@cn.fujitsu.com>,
	"Shi, Alex" <alex.shi@intel.com>
Subject: Re: [RFC]cfq-iosched: fix a kbuild regression
Date: Fri, 19 Mar 2010 08:57:19 +0100	[thread overview]
Message-ID: <20100319075719.GM5768@kernel.dk> (raw)
In-Reply-To: <20100318010459.GA26716@sli10-desk.sh.intel.com>

On Thu, Mar 18 2010, Shaohua Li wrote:
> On Thu, Mar 18, 2010 at 02:24:10AM +0800, Corrado Zoccolo wrote:
> > On Wed, Mar 17, 2010 at 2:30 PM, Jens Axboe <jens.axboe@oracle.com> wrote:
> > > On Tue, Mar 16 2010, Shaohua Li wrote:
> > >> Alex Shi reported a kbuild regression which is about 10% performance lost.
> > >> He bisected to this commit: 3dde36ddea3e07dd025c4c1ba47edec91606fec0.
> > >> The reason is cfqq_close() can't find close cooperator. If we store the seek
> > >> distance to the value before the commit like below, the regression fully goes
> > >> away. If this is too invasive, just changing the cfq_rq_close() for the
> > >> !for_preempt is ok too.
> > >
> > > Corrado, any objections to widening the seek threshold?
> > 
> > The previous seek threshold value was meant to be compared with the
> > average seek distance, so it was large to account for variations.
> > Since we handle the variations differently, we should have a smaller
> > value now (the 100 * 8 was the result of a benchmark run on several
> > disks).
> Our test doesn't find any issue with the seek threshold so far, so it proberly
> is ok.
> 
> > I agreee, though, that for merging queues, it is now too small, so we
> > should have two thresholds, the current one used to determine if a
> > request causes a seek, and an other to be used inside cfq_close (with
> > the older value, regardless of for_preempt).
> That makes sense. Updated patch.
> 
> 
> Alex Shi reported a kbuild regression which is about 10% performance lost.
> He bisected to this commit: 3dde36ddea3e07dd025c4c1ba47edec91606fec0.
> The reason is cfqq_close() can't find close cooperator. Restoring 
> cfq_rq_close()'s threshold to original value makes the regression go away.
> 
> Since for_preempt parameter isn't used anymore, this patch deletes it.

Thanks, I have applied it.

-- 
Jens Axboe


      parent reply	other threads:[~2010-03-19  7:57 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-16  2:56 [RFC]cfq-iosched: fix a kbuild regression Shaohua Li
2010-03-17 13:30 ` Jens Axboe
2010-03-17 18:24   ` Corrado Zoccolo
2010-03-18  1:05     ` Shaohua Li
2010-03-18 16:55       ` Corrado Zoccolo
2010-03-19  7:57       ` Jens Axboe [this message]

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=20100319075719.GM5768@kernel.dk \
    --to=jens.axboe@oracle.com \
    --cc=alex.shi@intel.com \
    --cc=czoccolo@gmail.com \
    --cc=guijianfeng@cn.fujitsu.com \
    --cc=jmoyer@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=shaohua.li@intel.com \
    --cc=vgoyal@redhat.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.