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@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>