mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Corrado Zoccolo <czoccolo@gmail.com>
To: Jens Axboe <jens.axboe@oracle.com>
Cc: Linux-Kernel <linux-kernel@vger.kernel.org>,
	Jeff Moyer <jmoyer@redhat.com>,
	aaronc@gelato.unsw.edu.au
Subject: Re: [RFC, PATCH] cfq-iosched: remove redundant queuing detection code
Date: Tue, 10 Nov 2009 18:56:01 +0100	[thread overview]
Message-ID: <4e5e476b0911100956u47bb3111jeba9fbcc3d992960@mail.gmail.com> (raw)
In-Reply-To: <20091110151431.GI8742@kernel.dk>

On Tue, Nov 10, 2009 at 4:14 PM, Jens Axboe <jens.axboe@oracle.com> wrote:
> On Tue, Nov 10 2009, Corrado Zoccolo wrote:
>> The core block layer already has code to detect presence of command
>> queuing devices. We convert cfq to use that instead of re-doing the
>> computation.
>
> There's is the major difference that the CFQ variant is dynamic and the
> block layer one is not. This change came from Aaron some time ago IIRC,
> see commit 45333d5. It's a bit of a chicken and egg problem.

The comment by Aaron:
    CFQ's detection of queueing devices assumes a non-queuing device and detects
    if the queue depth reaches a certain threshold.  Under some workloads (e.g.
    synchronous reads), CFQ effectively forces a unit queue depth,
thus defeating
    the detection logic.  This leads to poor performance on queuing hardware,
    since the idle window remains enabled.

makes me think that the dynamic-off detection in cfq may really be
buggy (BTW this could explain the bad results on SSD Jeff observed
before my patch set).
The problem is, that once the hw_tag is 0, it is difficult for it to
become 1 again, as explained by Aaron, since cfq will hardly send more
than 1 request at a time. My patch set fixes this for SSDs (the seeky
readers will still be sent without idling, and if they are enough, the
logic will see a large enough depth to reconsider the initial
decision).

So the only sound way to do the detection is to start in an
indeterminate state, in which CFQ behaves as if hw_tag = 1, and then,
if for a long observation period we never saw large depth, we switch
to hw_tag = 0, otherwise we stick to hw_tag = 1, without reconsidering
it.

I think the correct logic could be pushed to the blk-core, by
introducing also an indeterminate bit.

Corrado
>
> --
> Jens Axboe
>
>

  parent reply	other threads:[~2009-11-10 17:55 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-10 13:54 Corrado Zoccolo
2009-11-10 15:03 ` Jeff Moyer
2009-11-10 15:14 ` Jens Axboe
2009-11-10 15:20   ` Jeff Moyer
2009-11-10 15:27     ` Jens Axboe
2009-11-10 15:41       ` Jeff Moyer
2009-11-10 15:48         ` Jens Axboe
2009-11-10 15:55           ` Jeff Moyer
2009-11-10 17:56   ` Corrado Zoccolo [this message]
2009-11-12 12:16     ` Jens Axboe
2009-11-12 13:14       ` Corrado Zoccolo
2009-11-12 13:16         ` Jens Axboe

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=4e5e476b0911100956u47bb3111jeba9fbcc3d992960@mail.gmail.com \
    --to=czoccolo@gmail.com \
    --cc=aaronc@gelato.unsw.edu.au \
    --cc=jens.axboe@oracle.com \
    --cc=jmoyer@redhat.com \
    --cc=linux-kernel@vger.kernel.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

all inboxes | Powered by JetHome®