mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Jeff Moyer <jmoyer@redhat.com>
Cc: Jens Axboe <axboe@kernel.dk>, Jan Kara <jack@suse.cz>,
	Shaohua Li <shli@kernel.org>,
	linux-fsdevel@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: Read starvation by sync writes
Date: Thu, 13 Dec 2012 13:30:32 +0100	[thread overview]
Message-ID: <20121213123032.GB18843@quack.suse.cz> (raw)
In-Reply-To: <x49obhzkqhy.fsf@segfault.boston.devel.redhat.com>

On Wed 12-12-12 14:41:13, Jeff Moyer wrote:
> Jeff Moyer <jmoyer@redhat.com> writes:
> 
> >> I agree. This isn't about scheduling, we haven't even reached that part
> >> yet. Back when we split the queues into read vs write, this problem
> >> obviously wasn't there. Now we have sync writes and reads, both eating
> >> from the same pool. The io scheduler can impact this a bit by forcing
> >> reads to must allocate (Jan, which io scheduler are you using?). CFQ
> >> does this when it's expecting a request from this process queue.
> >>
> >> Back in the day, we used to have one list. To avoid a similar problem,
> >> we reserved the top of the list for reads. With the batching, it's a bit
> >> more complicated. If we make the request allocation (just that, not the
> >> scheduling) be read vs write instead of sync vs async, then we have the
> >> same issue for sync vs buffered writes.
> >>
> >> How about something like the below? Due to the nature of sync reads, we
> >> should allow a much longer timeout. The batch is really tailored towards
> >> writes at the moment. Also shrink the batch count, 32 is pretty large...
> >
> > Does batching even make sense for dependent reads?  I don't think it
> > does.
> 
> Having just read the batching code in detail, I'd like to ammend this
> misguided comment.  Batching logic kicks in when you happen to be lucky
> enough to use up the last request.  As such, I'd be surprised if the
> patch you posted helped.  Jens, don't you think the writer is way more
> likely to become the batcher?  I do agree with shrinking the batch count
> to 16, whether or not the rest of the patch goes in.
  Well, batching logic also triggers unconditionally after you waited for
a request...

> >  Assuming you disagree, then you'll have to justify that fixed
> > time value of 2 seconds.  The amount of time between dependent reads
> > will vary depending on other I/O sent to the device, the properties of
> > the device, the I/O scheduler, and so on.  If you do stick 2 seconds in
> > there, please comment it.  Maybe it's time we started keeping track of
> > worst case Q->C time?  That could be used to tell worst case latency,
> > and adjust magic timeouts like this one.
> >
> > I'm still thinking about how we might solve this in a cleaner way.
> 
> The way things stand today, you can do a complete end run around the I/O
> scheduler by queueing up enough I/O.  To address that, I think we need
> to move to a request list per io_context as Jan had suggested.  That
> way, we can keep the logic about who gets to submit I/O when in one
> place.
> 
> Jens, what do you think?
> 
> Jan, for now, try bumping nr_requests up really high.  ;-)
  Good idea. I tried bumping nr_requests to 100000 (so one can queue 50 GB
of streaming writes - one transaction can carry several hundred MB) and
reader can now progress at a reasonable speed. So it indeed worked.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

  reply	other threads:[~2012-12-13 12:30 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-10 22:12 Jan Kara
2012-12-11 21:44 ` Jeff Moyer
2012-12-12  2:31   ` Jan Kara
2012-12-12  4:18     ` Dave Chinner
2012-12-12 10:26       ` Jan Kara
2012-12-12 23:33         ` Dave Chinner
2012-12-12  0:13 ` Jan Engelhardt
2012-12-12  2:55 ` Shaohua Li
2012-12-12 10:11   ` Jan Kara
2012-12-12 15:19     ` Jens Axboe
2012-12-12 16:38       ` Jeff Moyer
2012-12-12 19:41         ` Jeff Moyer
2012-12-13 12:30           ` Jan Kara [this message]
2012-12-13 13:30           ` Jens Axboe
2012-12-13 14:55             ` Jeff Moyer
2012-12-13 15:02             ` Jan Kara
2012-12-13 18:03               ` Jens Axboe
2013-01-23 17:35                 ` Jeff Moyer
2012-12-13  1:43     ` Shaohua Li
2012-12-13 10:32       ` Jan Kara

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=20121213123032.GB18843@quack.suse.cz \
    --to=jack@suse.cz \
    --cc=axboe@kernel.dk \
    --cc=jmoyer@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=shli@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

all inboxes | Powered by JetHome®