aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorZach Brown <zach.brown@oracle.com>2007-10-30 14:45:46 -0400
committerLinus Torvalds <torvalds@woody.linux-foundation.org>2007-10-30 15:14:06 -0400
commitbdb76ef5a4bc8676a81034a443f1eda450b4babb (patch)
treeb4ec8736e6d4bed26f96c94d5c7c8eec0896fcd0
parente58b7dab272ecee09cd7bafb89d6b224cd17bbe3 (diff)
dio: fix cache invalidation after sync writes
Commit commit 65b8291c4000e5f38fc94fb2ca0cb7e8683c8a1b ("dio: invalidate clean pages before dio write") introduced a bug which stopped dio from ever invalidating the page cache after writes. It still invalidated it before writes so most users were fine. Karl Schendel reported ( http://lkml.org/lkml/2007/10/26/481 ) hitting this bug when he had a buffered reader immediately reading file data after an O_DIRECT wirter had written the data. The kernel issued read-ahead beyond the position of the reader which overlapped with the O_DIRECT writer. The failure to invalidate after writes caused the reader to see stale data from the read-ahead. The following patch is originally from Karl. The following commentary is his: The below 3rd try takes on your suggestion of just invalidating no matter what the retval from the direct_IO call. I ran it thru the test-case several times and it has worked every time. The post-invalidate is probably still too early for async-directio, but I don't have a testcase for that; just sync. And, this won't be any worse in the async case. I added a test to the aio-dio-regress repository which mimics Karl's IO pattern. It verifed the bad behaviour and that the patch fixed it. I agree with Karl, this still doesn't help the case where a buffered reader follows an AIO O_DIRECT writer. That will require a bit more work. This gives up on the idea of returning EIO to indicate to userspace that stale data remains if the invalidation failed. Signed-off-by: Zach Brown <zach.brown@oracle.com> Cc: Karl Schendel <kschendel@datallegro.com> Cc: Benjamin LaHaise <bcrl@kvack.org> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Nick Piggin <nickpiggin@yahoo.com.au> Cc: Leonid Ananiev <leonid.i.ananiev@linux.intel.com> Cc: Chris Mason <chris.mason@oracle.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r--mm/filemap.c16
1 files changed, 6 insertions, 10 deletions
diff --git a/mm/filemap.c b/mm/filemap.c
index 7c8643630023..9940895f734c 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2511,21 +2511,17 @@ generic_file_direct_IO(int rw, struct kiocb *iocb, const struct iovec *iov,
2511 } 2511 }
2512 2512
2513 retval = mapping->a_ops->direct_IO(rw, iocb, iov, offset, nr_segs); 2513 retval = mapping->a_ops->direct_IO(rw, iocb, iov, offset, nr_segs);
2514 if (retval)
2515 goto out;
2516 2514
2517 /* 2515 /*
2518 * Finally, try again to invalidate clean pages which might have been 2516 * Finally, try again to invalidate clean pages which might have been
2519 * faulted in by get_user_pages() if the source of the write was an 2517 * cached by non-direct readahead, or faulted in by get_user_pages()
2520 * mmap()ed region of the file we're writing. That's a pretty crazy 2518 * if the source of the write was an mmap'ed region of the file
2521 * thing to do, so we don't support it 100%. If this invalidation 2519 * we're writing. Either one is a pretty crazy thing to do,
2522 * fails and we have -EIOCBQUEUED we ignore the failure. 2520 * so we don't support it 100%. If this invalidation
2521 * fails, tough, the write still worked...
2523 */ 2522 */
2524 if (rw == WRITE && mapping->nrpages) { 2523 if (rw == WRITE && mapping->nrpages) {
2525 int err = invalidate_inode_pages2_range(mapping, 2524 invalidate_inode_pages2_range(mapping, offset >> PAGE_CACHE_SHIFT, end);
2526 offset >> PAGE_CACHE_SHIFT, end);
2527 if (err && retval >= 0)
2528 retval = err;
2529 } 2525 }
2530out: 2526out:
2531 return retval; 2527 return retval;