All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@suse.de>
To: Nick Piggin <piggin@cyberone.com.au>
Cc: linux-kernel@vger.kernel.org, ckrm-tech@lists.sourceforge.net
Subject: Re: [PATCH] cfq-prio #2
Date: Mon, 10 Nov 2003 15:39:39 +0100	[thread overview]
Message-ID: <20031110143939.GJ32637@suse.de> (raw)
In-Reply-To: <3FAFA1E8.8080800@cyberone.com.au>

On Tue, Nov 11 2003, Nick Piggin wrote:
> 
> 
> Jens Axboe wrote:
> 
> >On Tue, Nov 11 2003, Nick Piggin wrote:
> >
> >>
> >>Jens Axboe wrote:
> >>
> >>
> >>>Hi,
> >>>
> >>>
> >>>
> >>Hi Jens
> >>
> >>
> >>>@@ -1553,6 +1559,10 @@
> >>>	struct io_context *ioc = get_io_context(gfp_mask);
> >>>
> >>>	spin_lock_irq(q->queue_lock);
> >>>+
> >>>+	if (!elv_may_queue(q, rw))
> >>>+		goto out_lock;
> >>>+
> >>>	if (rl->count[rw]+1 >= q->nr_requests) {
> >>>		/*
> >>>		 * The queue will fill after this allocation, so set it as
> >>>@@ -1566,15 +1576,12 @@
> >>>		}
> >>>	}
> >>>
> >>>-	if (blk_queue_full(q, rw)
> >>>-			&& !ioc_batching(ioc) && !elv_may_queue(q, rw)) {
> >>>
> >>>
> >>I know I hijacked elv_may_queue from you... any chance we could seperate
> >>these so our schedulers can live in peace? ;)
> >>
> >
> >IOW, you completely broke it! I'm just changing it back to the
> >original. When was this done, btw? Just discovered it when updating the
> >patch. Pretty annoying...
> >
> 
> You acked the change actually :P
> I guess it was done in mainline when AS was merged.

Probably missed the semantic change of may_queue.

> >>Maybe my version should be called elv_force_queue?
> >>
> >
> >I just hate to see more of these, really. The original idea for
> >may_queue was just that, may this process queue io or not. We can make
> >it return something else, though, to indicate whether the process must
> >be able to queue. Is it really needed?
> >
> 
> Its quite important. If the queue is full, and AS is waiting for a process
> to submit a request, its got a long wait.
> 
> Maybe a lower limit for per process nr_requests. Ie. you may queue if this
> queue has less than 128 requests _or_ you have less than 8 requests
> outstanding. This would solve my problem. It would also give you a much more
> appropriate scaling for server workloads, I think. Still, thats quite a
> change in behaviour (simple to code though).

That basically belongs inside your may_queue for the io scheduler, imo.

-- 
Jens Axboe


  reply	other threads:[~2003-11-10 14:39 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-11-10 14:00 [PATCH] cfq-prio #2 Jens Axboe
2003-11-10 14:16 ` Nick Piggin
2003-11-10 14:23   ` Jens Axboe
2003-11-10 14:34     ` Nick Piggin
2003-11-10 14:39       ` Jens Axboe [this message]
2003-11-10 14:43         ` Nick Piggin
2003-11-10 14:44           ` Jens Axboe
2003-11-10 14:48             ` Nick Piggin
2003-11-10 14:52               ` 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=20031110143939.GJ32637@suse.de \
    --to=axboe@suse.de \
    --cc=ckrm-tech@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=piggin@cyberone.com.au \
    /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.