Re: trying to understand READ_META, READ_SYNC, WRITE_SYNC & co

Christoph Hellwig
2010-06-23T10:01:54Z

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.