linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Moyer <jmoyer@redhat.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Jan Kara <jack@suse.cz>,
	linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org,
	xfs@oss.sgi.com
Subject: Re: [PATCH 3/3] filemap: don't call generic_write_sync for -EIOCBQUEUED
Date: Tue, 07 Feb 2012 15:39:06 -0500	[thread overview]
Message-ID: <x49pqdqml91.fsf@segfault.boston.devel.redhat.com> (raw)
In-Reply-To: <20120206195546.GA22640@infradead.org> (Christoph Hellwig's message of "Mon, 6 Feb 2012 14:55:47 -0500")

Christoph Hellwig <hch@infradead.org> writes:

> On Mon, Feb 06, 2012 at 11:33:29AM -0500, Jeff Moyer wrote:
>> > code, right? Before that we'd drain the IO queue when cache flush is issued
>> > and thus effectively wait for IO completion...
>> 
>> Right, though hch seems to think even then the problem existed.
>
> I was wrong, using -o barrier it didn't.  That was however not something
> people using O_SYNC heavy production loads would do, they'd use disabled
> caches and nobarrier.
>
>> > Also I was thinking whether we couldn't implement the fix in VFS. Basically
>> > it would be the same like the fix for ext4. Like having a per-sb workqueue
>> > and queue work calling generic_write_sync() from end_io handler when the
>> > file is O_SYNC? That would solve the issue for all filesystems...
>> 
>> Well, that would require buy-in from the other file system developers.
>> What do the XFS folks think?
>
> I don't think using that code for XFS makes sene.  But just like
> generic_write_sync there's no reason it can't be added to generic code,
> just make sure only generic_file_aio_write/__generic_file_aio_write use
> it, but generic_file_buffered_write and generic_file_direct_write stay
> clear of it.

ext4_file_write (ext4's .aio_write routine) calls into
generic_file_aio_write.  So, I don't think we can generalize that this
routine means that the file system doesn't install its own endio
handler.  What's more, we'd have to pass an endio routine down the call
stack quite a ways.  In all, I think that would be an uglier solution to
the problem.  Did I miss something?

Cheers,
Jeff

  reply	other threads:[~2012-02-07 20:39 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-27 21:15 [patch|rfc][0/3] fix aio+dio+O_SYNC writes Jeff Moyer
2012-01-27 21:15 ` [PATCH 1/3] xfs: honor the O_SYNC flag for aysnchronous direct I/O requests Jeff Moyer
2012-01-28 14:59   ` Christoph Hellwig
2012-01-27 21:15 ` [PATCH 2/3] ext4: " Jeff Moyer
2012-02-02 17:31   ` Jan Kara
2012-02-06 16:20     ` Jeff Moyer
2012-02-06 16:58       ` Jan Kara
2012-02-08 15:11     ` Jeff Moyer
2012-02-13 18:27       ` Jan Kara
2012-01-27 21:15 ` [PATCH 3/3] filemap: don't call generic_write_sync for -EIOCBQUEUED Jeff Moyer
2012-01-28 15:08   ` Martin Steigerwald
2012-02-02 17:52   ` Jan Kara
2012-02-06 16:33     ` Jeff Moyer
2012-02-06 19:55       ` Christoph Hellwig
2012-02-07 20:39         ` Jeff Moyer [this message]
2012-02-08 16:09           ` Jan Kara
2012-02-08 16:38             ` Jeff Moyer

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=x49pqdqml91.fsf@segfault.boston.devel.redhat.com \
    --to=jmoyer@redhat.com \
    --cc=hch@infradead.org \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=xfs@oss.sgi.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).