LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jens Axboe <jens.axboe@oracle.com>
To: Nikanth Karthikesan <knikanth@suse.de>
Cc: linux-kernel@vger.kernel.org, nickpiggin@yahoo.com.au
Subject: Re: [PATCH] [RFC] block/elevator: change nr_requests handling
Date: Tue, 29 Jan 2008 19:19:15 +0100	[thread overview]
Message-ID: <20080129181915.GD15220@kernel.dk> (raw)
In-Reply-To: <1201627774.14644.5.camel@nikanth-laptop.blr.novell.com>

On Tue, Jan 29 2008, Nikanth Karthikesan wrote:
> The /sys/block/whateverdisk/queue/nr_requests is used to limit the
> number of requests allocated per queue. There can be atmost nr_requests
> read requests and nr_requests write requests allocated.
> 
> But this can lead to starvation when there are many tasks doing heavy
> IO. And the ioc_batching code lets those who had failed to allocate a
> request once, to allocate upto 150% of nr_requests in next round. This
> just makes the limit a little higher, when there are too many tasks
> doing heavy IO. IO schedulers have no control over this to choose which
> task should get the requests. During such situations, even ionice cannot
> help as CFQ cannot serve a task which cannot allocate any requests.
> Setting nr_requests to a higher value is like disabling this queue depth
> check at all.
> 
> This patch defers the nr_requests check till dispatching the requests
> instead of limiting while creating them. There can be more than
> nr_requests allocated, but only upto nr_requests chosen by the io
> scheduler will be dispatched at a time. This lets the io scheduler to
> guarantee some sort of interactivity even when there are many batchers,
> if it wants to.
> 
> This patch removes the ioc_batching code. Also the schedulers stop
> dispatching when it chooses a Read request and Read queue is full and
> vice versa. On top of this patch, the individual schedulers can be
> changed to take advantage of knowing the no of read and write requests
> that can be dispatched. Or atleast dispatch until both read and write
> queues are full.

This is all a bit backwards, I think. The io schedulers CAN choose which
processes get to allocate requests, through the ->may_queue() hook.

I definitely think the batching logic could do with some improvements,
so I'd encourage you to try and fix that instead. It'd be nice if it did
honor the max number of requests limit. The current batching works well
for allowing a process to queue some IO when it gets the allocation
'token', that should be retained.

Did you do any performance numbers with your patch, btw?

-- 
Jens Axboe


  parent reply	other threads:[~2008-01-29 18:20 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-29 17:29 Nikanth Karthikesan
2008-01-29 17:47 ` Nikanth Karthikesan
2008-01-29 18:19 ` Jens Axboe [this message]
2008-01-30 13:53   ` Nikanth Karthikesan
2008-01-30 13:56     ` 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=20080129181915.GD15220@kernel.dk \
    --to=jens.axboe@oracle.com \
    --cc=knikanth@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nickpiggin@yahoo.com.au \
    --subject='Re: [PATCH] [RFC] block/elevator: change nr_requests handling' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).