linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Vivek Goyal <vgoyal@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>, Jens Axboe <axboe@kernel.dk>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: trying to understand READ_META, READ_SYNC, WRITE_SYNC & co
Date: Wed, 23 Jun 2010 12:01:38 +0200	[thread overview]
Message-ID: <20100623100138.GA9575@lst.de> (raw)
In-Reply-To: <20100621213618.GC6474@redhat.com>

On Mon, Jun 21, 2010 at 05:36:18PM -0400, Vivek Goyal wrote:
> I have got very little understanding of file system layer, but if I had
> to guess, i think following might have happened.
> 
> - We switched from WRITE to WRITE_SYNC for fsync() path.

Yes.  WRITE_SYNC_PLUG to be exact.  Note that we don't juse do this
for fsync but also for O_SYNC writes which use ->fsync, and also sync(2)
and the unmount path, which all end up submitting WB_SYNC_ALL writeback
requests.

> - This might have caused issues with idling as for SYNC_WRITE we will idle
>   in CFQ but probably it is not desirable in certain cases where next set
>   of WRITES is going to come from journaling thread. 

I'm still a bit confused about what the idling logic actually does.  Is
it some sort of additional plugging where we wait for more I/O to
accumulate?

> - That might have prompted us to introduce the rq_noidle() to make sure
>   we don't idle in WRITE_SYNC path but direct IO path was avoided to make
>   sure good throughput is maintained. But this left one question open 
>   and that is it good to disable idling on all WRITE_SYNC path in kernel.

I still fail to see why we should make any difference in the I/O
scheduler for O_DIRECT vs O_SYNC/fsync workloads.  In both cases the
caller blocks waiting for the I/O completion. 

> - Slowly cfq code emerged and as it stands today, to me rq_noidle() is
>   practically of not much use. For sync-idle tree (where idling is
>   enabled), we are ignoring the rq_noidle() and always arming the timer.
>   For sync-noidle, we choose not to idle based on if there was some other
>   thread who did even a single IO with rq_noidle=0.
> 
>   I think in practice, there is on thread of other which is doing some
>   read or write with rq_noidle=0 and if that's the case, we will end up
>   idling on sync-noidle tree also and rq_noidle() practically does
>   not take effect.
> 
> So if rq_noidle() was introduced to solve the issue of not idling on 
> fsync() path (as jbd thread will send more data now), then probably slice
> yielding patch of jeff might come handy here and and we can get rid of
> rq_noidle() logic. This is just a guess work and I might be completely
> wrong here...

Getting rid of the noidle logic and more bio flag that us filesystem
developers have real trouble understanding would be a good thing.

After that we're down to three bio modifiers for filesystem use, of
which at least two are very easy to grasp:

 - REQ_SYNC - treat a request as synchronous, implicitly enabled for
   reads anyway
 - REQ_UNPLUG - explicitly unplug the queue after I/O submission

and

 - REQ_META - which we're currenly trying to define in detail


REQ_NOIDLE currenly really is a lot of deep magic.

  reply	other threads:[~2010-06-23 10:01 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-21  9:48 trying to understand READ_META, READ_SYNC, WRITE_SYNC & co Christoph Hellwig
2010-06-21 10:04 ` Jens Axboe
2010-06-21 11:04   ` Christoph Hellwig
2010-06-21 18:56     ` Jens Axboe
2010-06-21 19:14       ` Christoph Hellwig
2010-06-21 19:16         ` Jens Axboe
2010-06-21 19:20           ` Christoph Hellwig
2010-06-21 21:36         ` Vivek Goyal
2010-06-23 10:01           ` Christoph Hellwig [this message]
2010-06-24  1:44             ` Vivek Goyal
2010-06-25 11:03               ` Christoph Hellwig
2010-06-26  3:35                 ` Vivek Goyal
2010-06-26 10:05                   ` Christoph Hellwig
2010-06-26 11:20                     ` Jens Axboe
2010-06-26 11:56                       ` Christoph Hellwig
2010-06-27 15:44                   ` Jeff Moyer
2010-06-29  9:06                     ` Corrado Zoccolo
2010-06-29 12:30                       ` Vivek Goyal
2010-06-30 15:30                         ` Corrado Zoccolo
2010-06-26  9:25                 ` Nick Piggin
2010-06-26  9:27                   ` Christoph Hellwig
2010-06-26 10:10                     ` Nick Piggin
2010-06-26 10:16                       ` Christoph Hellwig
2010-06-21 18:52   ` Jeff Moyer
2010-06-21 18:58     ` Jens Axboe
2010-06-21 19:08       ` Jeff Moyer
2010-06-23  9:26       ` Christoph Hellwig
2010-06-21 20:25   ` Vivek Goyal
2010-06-23 10:02     ` Christoph Hellwig

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=20100623100138.GA9575@lst.de \
    --to=hch@lst.de \
    --cc=axboe@kernel.dk \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=vgoyal@redhat.com \
    /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;
as well as URLs for NNTP newsgroup(s).