public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <jens.axboe@oracle.com>
To: Jeff Moyer <jmoyer@redhat.com>
Cc: Corrado Zoccolo <czoccolo@gmail.com>,
	Linux-Kernel <linux-kernel@vger.kernel.org>,
	aaronc@gelato.unsw.edu.au
Subject: Re: [RFC, PATCH] cfq-iosched: remove redundant queuing detection code
Date: Tue, 10 Nov 2009 16:27:45 +0100	[thread overview]
Message-ID: <20091110152745.GK8742@kernel.dk> (raw)
In-Reply-To: <x49ocna5rv1.fsf@segfault.boston.devel.redhat.com>

On Tue, Nov 10 2009, Jeff Moyer wrote:
> Jens Axboe <jens.axboe@oracle.com> writes:
> 
> > 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.
> 
> Really?  blk_dequeue_request sure looks like it updates things
> dynamically, but only one way (not queueing -> queueing).  Would it make

Yes of course the block layer one is dynamically on as well. The ideal
goal would be to have every driver use the block layer tagging in which
case we'd know without checking, but alas it isn't so (yet). My point is
that the CFQ variant is dynamically off as well. Corrado presents his
patch as a direct functional equivelant, which it definitely isn't.

> sense to just put CFQ's logic into the block layer so that everyone uses
> the same implementation?  It makes little sense to have two notions of
> whether or not queueing is supported for a device.

The one use in the block layer cares about the static property of the
device, not the current behaviour. So I'm not sure it makes a lot of
sense to unify these. It's not really a case of code duplication either,
the block layer one is two checks and a bit. The cfq variant is a bit
more involved in that it tracks the state continually.

-- 
Jens Axboe


  reply	other threads:[~2009-11-10 15:27 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-10 13:54 [RFC, PATCH] cfq-iosched: remove redundant queuing detection code 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 [this message]
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
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=20091110152745.GK8742@kernel.dk \
    --to=jens.axboe@oracle.com \
    --cc=aaronc@gelato.unsw.edu.au \
    --cc=czoccolo@gmail.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