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

[Christoph Hellwig <hch@infradead.org>]

On Wed, Jun 15, 2016 at 02:29:42PM +0000, Trond Myklebust wrote:
> The locking is actually simpler than XFS.

It looks way more complicated.  And totally undocumented.

> We have 2 I/O modes: buffered I/O and direct I/O. The write lock is there to ensure safe transitions between those 2 modes, but once the mode is set,
> we _only_ use shared locks in order to allow parallelism.

>From reading the patch that's not what actually happens - I think you're
still taking i_rwsem exclusive for buffered writes, aren't you?

Doing that is absolutely mandatory for Posix atomicy requirements, or
you'll break tons of of applications.

But XFS allows full parallelism for direct reads and writes as long
as there is no more pagecache to flush.  But if you have pages in
the pagecache you need the exclusive lock to prevent against new
pagecache pages being added.

> >The nice thing is than in 4.7-rc i_mutex has been replaced with a
> >rw_mutex so you can just use that in shared mode for direct I/O
> >as-is without needing any new lock.
> 
> We would end up serialising reads and writes, since the latter grab an
> exclusive lock in generic_file_write(). Why do that if we don???t have to?

Looks at the XFS code - no serialization between direct reads and writes
as long as no buffered I/O came in inbetween.

And don't use generic_file_{read,write}_iter if you want to do direct I/O,
unfortunately locking in mm/filemap.c is totally screwed for direct I/O,
take a look at XFS which is where direct I/O came from and where we get
the locking right.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs