Re: [PATCH v2 10/12] NFS: Do not serialise O_DIRECT reads and writes

[Christoph Hellwig <hch@infradead.org>]

On Wed, Jun 22, 2016 at 05:24:50PM +0000, Trond Myklebust wrote:
> If we?re going to worry about write atomicity in the buffered I/O case,
> then we really should also make sure that O_DIRECT writes are atomic
> w.r.t. page cache updates too.  With this locking model, a buffered
> read() can race with the O_DIRECT write() and get a mixture of old
> data and new.

The difference between buffered I/O and direct I/O is that the former
is covered by standards, and the latter is a Linux extension with very
lose semantics.  But I'm perfectly fine with removing the buffered
reader shared lock for now - for the purposes of direct I/O
synchronization it's not nessecary.


Yes.

> > +	if (mapping->nrpages) {
> > +		inode_lock(inode);
> 
> This is unnecessary now that we have a rw_semaphore. You don?t need to
> take an exclusive lock in order to serialise w.r.t. new writes, and by
> doing so you end up serialising all reads if there happens to be pages
> in the page cache. This is true whether or not those pages are dirty.

Traditionally we needed the exclusive lock around
invalidate_inode_pages2 and unmap_mapping_range, and from a quick look
that's what the existing callers all have.  I don't actually see that
requirement documented anywhere, though.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>