All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <jens.axboe@oracle.com>
To: Carl Henrik Lunde <chlunde@ping.uio.no>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: CFQ: Preemption/timeout logic reversed?
Date: Mon, 20 Apr 2009 08:16:02 +0200	[thread overview]
Message-ID: <20090420061602.GO4593@kernel.dk> (raw)
In-Reply-To: <ed038eb20904191756y5a5bcf8fn53473405b3929de6@mail.gmail.com>

On Mon, Apr 20 2009, Carl Henrik Lunde wrote:
> Hi!
> 
> It seems the preemption "bonus" logic in CFQ is reversed, a preempted
> process is given an additional delay in start time instead of a bonus.
>  This seems unfair.  I'm not sure if it's a good idea to let

Hmm? ->slice_resid is a long, so if we preempt the process 10 jiffies
before it was supposed to end, the resid will be -10. So it'll not
increase the rb_key, it'll decrease it.

> slice_resid grow without limit as shown below, but isn't this more
> like the way it was intended to work?  Or did I misunderstand
> something?

->slice_resid is reset when it gets repositioned in the rb tree. The
intent was not to increase the slice length, but instead allow it sooner
service again.

> PS! The comment above cfq_preempt_queue seems outdated too.

Yep, the slice length comment is out dated indeed.

> Code not tested, just showing what I mean:
> 
> diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
> index 664ebfd..ea18d45 100644
> --- a/block/cfq-iosched.c
> +++ b/block/cfq-iosched.c
> @@ -292,7 +292,8 @@ cfq_prio_to_slice(struct cfq_data *cfqd, struct
> cfq_queue *cfqq)
>  static inline void
>  cfq_set_prio_slice(struct cfq_data *cfqd, struct cfq_queue *cfqq)
>  {
> -       cfqq->slice_end = cfq_prio_to_slice(cfqd, cfqq) + jiffies;
> +       cfqq->slice_end = cfq_prio_to_slice(cfqd, cfqq) +
> cfqq->slice_resid + jiffies;

So if ->slice_resid is negative because we preempted this queue, it'll
now get a shorter slice. That's not very nice :-)

-- 
Jens Axboe


  reply	other threads:[~2009-04-20  6:16 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-20  0:56 CFQ: Preemption/timeout logic reversed? Carl Henrik Lunde
2009-04-20  6:16 ` Jens Axboe [this message]
2009-04-20  6:48   ` Carl Henrik Lunde
2009-04-20  8:24     ` 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=20090420061602.GO4593@kernel.dk \
    --to=jens.axboe@oracle.com \
    --cc=chlunde@ping.uio.no \
    --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 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.