All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nick Piggin <piggin@cyberone.com.au>
To: Jens Axboe <axboe@suse.de>
Cc: Linus Torvalds <torvalds@osdl.org>, Andrew Morton <akpm@osdl.org>,
	Jens Axboe <axboe@kernel.dk>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] fix IO hangs
Date: Fri, 05 Sep 2003 17:16:49 +1000	[thread overview]
Message-ID: <3F583861.6070109@cyberone.com.au> (raw)
In-Reply-To: <20030905070426.GP840@suse.de>



Jens Axboe wrote:

>On Fri, Sep 05 2003, Nick Piggin wrote:
>
>>Hi, sorry for the hangs, everyone. I think I have it worked out, but
>>testers and an ack from Jens would be good.
>>
>>The insert_here code now does as advertised. The big difference being
>>that regular blk_fs_requests will be subject to it (required for SCSI
>>requeue). Unfortunately ll_rw_blk.c misuses it and will sometimes try
>>to insert at requests which are not on the dispatch list, causing the
>>badness.
>>
>>It looks like the code was maybe used to provide an insertion hint
>>for the elevator. The RB tree has now eliminated that requirement even
>>if the code did work. Which it doesn't.
>>
>>I can't reproduce the hangs with this patch. Please test.
>>
>>
>>Aside, insert_here really seems to be quite dangerous to me. I think
>>combination of barriers and an "insert at start/end" flag would be
>>enough.
>>
>
>Please just kill insert_here, it's exceeded its life expectancy. The 2.2
>io scheduler did a merge scan followed by an insertion scan, 2.3
>collapsed them into one scan as an optimization. Performance oriented io
>schedulers need to use better data structures.
>
>Best would be to change it to pass a request back even for the NO_MERGE
>case, if it has found a good insertion point. It's still a good idea to
>be able to pass hints back like this, as it could still be a viable
>optimization for _other_ io schedulers.
>

OK, that is sort of an ACK! Pending wider testing this patch needs
to get in.

Jens, if insert_here is dead, there is no point to passing back a hint
because it can't get back to the elevator anyway.

I'd very much like to kill insert_here and be done with it. If another
io scheduler comes along with a good use for it then the writers can
come up with an elegant solution ;) Hey, if they know a NO_MERGE return
means an insert will soon happen under the same lock, they could keep
it cached privately.



  reply	other threads:[~2003-09-05  7:17 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-09-05  6:56 [PATCH] fix IO hangs Nick Piggin
2003-09-05  7:04 ` Jens Axboe
2003-09-05  7:16   ` Nick Piggin [this message]
2003-09-05  7:18     ` Jens Axboe
2003-09-05  8:31       ` Jens Axboe
2003-09-05 22:03         ` Martin Schlemmer
2003-09-06  1:34           ` Nick Piggin

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=3F583861.6070109@cyberone.com.au \
    --to=piggin@cyberone.com.au \
    --cc=akpm@osdl.org \
    --cc=axboe@kernel.dk \
    --cc=axboe@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@osdl.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.