On Wed, Oct 29, 2008 at 02:26:01PM +1100, Dave Chinner wrote: > On Wed, Oct 29, 2008 at 02:16:45PM +1100, Dave Chinner wrote: > > On Wed, Oct 29, 2008 at 01:16:53AM +0100, Nick Piggin wrote: > > > XFS: fix fsync errors not being propogated back to userspace. > > > --- > > > Index: linux-2.6/fs/xfs/xfs_vnodeops.c > > > =================================================================== > > > --- linux-2.6.orig/fs/xfs/xfs_vnodeops.c > > > +++ linux-2.6/fs/xfs/xfs_vnodeops.c > > > @@ -715,7 +715,7 @@ xfs_fsync( > > > /* capture size updates in I/O completion before writing the inode. */ > > > error = filemap_fdatawait(VFS_I(ip)->i_mapping); > > > if (error) > > > - return XFS_ERROR(error); > > > + return XFS_ERROR(-error); > > > > <groan> > > > > Yeah, that'd do it. Good catch. I can't believe I recently fixed a > > bug that touched these lines of code without noticing the inversion. > > Sometimes I wonder if we should just conver the entire of XFS to > > return negative errors - mistakes in handling negative error numbers > > in the core XFS code happen all the time. > > Ok, I was right - these problems happen all the time. The above call > should really call xfs_flush_pages() to do the flush and wait. I > note that xfs_flush_pages() returns negative errors, and all the > callers expect positive errors. I bet the same occurs for > xfs_flushinval_pages() and xfs_tosspages() which are the wrappers > that core XFS code is supposed to be using for flushing and > invalidating file ranges.... Just be careful -- in your xfs_flush_pages, I think after the first filemap_fdatawrite, the mapping may no longer be tagged with PAGECACHE_TAG_DIRTY, so you may not pick up those writeback ones you need to wait on. Might need a different variant, or we could just bite the bullet and push through the ->fsync conversion so you get full control of the writeout. BTW. the Linux pagecache APIs should support range operations quite nicely for these. Any reason not to use them (it looks like the wrappers can take ranges)? -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
