On Thu, Aug 10, 2017 at 10:22:47AM -0400, Jeff Moyer wrote: > Jan Kara <jack@xxxxxxx> writes: > > > On Thu 10-08-17 14:59:57, Lukas Czerner wrote: > >> Currently when mixing buffered reads and asynchronous direct writes it > >> is possible to end up with the situation where we have stale data in the > >> page cache while the new data is already written to disk. This is > >> permanent until the affected pages are flushed away. Despite the fact > >> that mixing buffered and direct IO is ill-advised it does pose a thread > >> for a data integrity, is unexpected and should be fixed. > >> > >> Fix this by deferring completion of asynchronous direct writes to a > >> process context in the case that there are mapped pages to be found in > >> the inode. Later before the completion in dio_complete() invalidate > >> the pages in question. This ensures that after the completion the pages > >> in the written area are either unmapped, or populated with up-to-date > >> data. Also do the same for the iomap case which uses > >> iomap_dio_complete() instead. > >> > >> This has a side effect of deferring the completion to a process context > >> for every AIO DIO that happens on inode that has pages mapped. However > >> since the consensus is that this is ill-advised practice the performance > >> implication should not be a problem. > >> > >> This was based on proposal from Jeff Moyer, thanks! > > > > It seems the invalidation can be also removed from > > generic_file_direct_write(), can't it? It is duplicit there the same way as > > it was in the iomap code... > > Yep, sure looks that way. Hrm, ok. Technically speaking generic_file_direct_write does not have to eventually end up with dio_complete() being called. This will change the behaviour for those that implement dio differently. Looking at the users now, vast majority will end up with complete_dio() so maybe this is not a problem. This is in contrast with iomap_dio_rw() which will end up calling iomap_dio_complete() so the situation is different there. Maybe adding mapping->nrpages check would be better than outright removing it ? -Lukas > > -Jeff
