From: Al Viro <viro@ZenIV.linux.org.uk>
To: Dave Chinner <david@fromorbit.com>
Cc: Jens Axboe <axboe@fb.com>,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>,
Christoph Hellwig <hch@lst.de>, Theodore Ts'o <tytso@mit.edu>,
"Elliott, Robert (Server Storage)" <elliott@hp.com>
Subject: Re: [PATCH 1/3] direct-io: only inc/dec inode->i_dio_count for file systems
Date: Wed, 15 Apr 2015 23:56:25 +0100 [thread overview]
Message-ID: <20150415225625.GR889@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20150415223620.GU13731@dastard>
On Thu, Apr 16, 2015 at 08:36:20AM +1000, Dave Chinner wrote:
> Bikeshedding: I think this would be better suited to inode_dio_begin()
> and inode_dio_end() because now we are trying to say "this is where
> the DIO starts, and this is where it ends". It's not really
> "reference counting" interface, we're trying to annotate the
> boundaries of where DIO iis protected against truncate....
*nod*
And while we are at, inode_dio_begin() could be static inline just fine.
> And, realistically, if we are pushing this up into the filesystems
> again, we should push it up into *all* filesystems and get rid of it
> completely from the DIO layer. That way no new twisty passages in
> the direct IO code are needed.
Maybe... FWIW, since the code *checking* ->i_dio_count is already
inside the individual filesystems (and right now I'm trying to verify
it), we might as well have the code changing it there as well. AFAICS,
the main problem with that is the need to do it after the end of async
operation. Sure, we do have callbacks, but we'd need something like
* start protection
* submit DIO
* if that has failed synchronously, drop protection and fuck off
* otherwise let the callback deal with dropping it.
What's more, we'd get not just a slew of boilerplate callbacks, but make
life interesting for branch predictor - indirect calls are not fun...
next prev parent reply other threads:[~2015-04-15 22:56 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-15 22:01 [PATCH v2] direct-io: only inc/dec inode->i_dio_count for file systems Jens Axboe
2015-04-15 22:01 ` [PATCH 1/3] " Jens Axboe
2015-04-15 22:36 ` Dave Chinner
2015-04-15 22:56 ` Al Viro [this message]
2015-04-15 23:05 ` Jens Axboe
2015-04-15 23:30 ` Al Viro
2015-04-15 23:50 ` Jens Axboe
2015-04-15 22:57 ` Jens Axboe
2015-04-15 22:01 ` [PATCH 2/3] btrfs: pass in DIO_SKIP_DIO_COUNT to do_blockdev_direct_IO() Jens Axboe
2015-04-15 22:01 ` [PATCH 3/3] ext4: " Jens Axboe
2015-04-15 22:05 ` [PATCH v2] direct-io: only inc/dec inode->i_dio_count for file systems Al Viro
2015-04-15 22:06 ` Jens Axboe
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=20150415225625.GR889@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=akpm@linux-foundation.org \
--cc=axboe@fb.com \
--cc=david@fromorbit.com \
--cc=elliott@hp.com \
--cc=hch@lst.de \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tytso@mit.edu \
/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).