| Message ID | 87y43dd4oe.fsf@notabene.neil.brown.name (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2016-08-31 at 12:59 +1000, NeilBrown wrote: > If O_DIRECT writes are racing with buffered writes, then > the call to invalidate_inode_pages2_range() can call ceph_releasepage() > on dirty pages. > > Most filesystems hold inode_lock() across O_DIRECT writes so they do not > suffer this race, but cephfs deliberately drops the lock, and opens a window > for the race. > > This race can be triggered with the generic/036 test from the xfstests > test suite. It doesn't happen every time, but it does happen often. > > As the possibilty is expected, remove the warning, and instead include > the PageDirty() status in the debug message. > > > Signed-off-by: NeilBrown <neilb@suse.com> > --- > fs/ceph/addr.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c > index d5b6f959a3c3..f657da7d12ff 100644 > --- a/fs/ceph/addr.c > +++ b/fs/ceph/addr.c > @@ -175,9 +175,8 @@ static void ceph_invalidatepage(struct page *page, unsigned int offset, > > static int ceph_releasepage(struct page *page, gfp_t g) > { > > - dout("%p releasepage %p idx %lu\n", page->mapping->host, > > - page, page->index); > > - WARN_ON(PageDirty(page)); > > + dout("%p releasepage %p idx %lu (%sdirty)\n", page->mapping->host, > > + page, page->index, PageDirty(page) ? "" : "not "); > > > /* Can we release the page from the cache? */ > > if (!ceph_release_fscache_page(page, g)) Reviewed-by: Jeff Layton <jlayton@redhat.com> -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c index d5b6f959a3c3..f657da7d12ff 100644 --- a/fs/ceph/addr.c +++ b/fs/ceph/addr.c @@ -175,9 +175,8 @@ static void ceph_invalidatepage(struct page *page, unsigned int offset, static int ceph_releasepage(struct page *page, gfp_t g) { - dout("%p releasepage %p idx %lu\n", page->mapping->host, - page, page->index); - WARN_ON(PageDirty(page)); + dout("%p releasepage %p idx %lu (%sdirty)\n", page->mapping->host, + page, page->index, PageDirty(page) ? "" : "not "); /* Can we release the page from the cache? */ if (!ceph_release_fscache_page(page, g))
If O_DIRECT writes are racing with buffered writes, then the call to invalidate_inode_pages2_range() can call ceph_releasepage() on dirty pages. Most filesystems hold inode_lock() across O_DIRECT writes so they do not suffer this race, but cephfs deliberately drops the lock, and opens a window for the race. This race can be triggered with the generic/036 test from the xfstests test suite. It doesn't happen every time, but it does happen often. As the possibilty is expected, remove the warning, and instead include the PageDirty() status in the debug message. Signed-off-by: NeilBrown <neilb@suse.com> --- fs/ceph/addr.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)