xfs
[Top] [All Lists]

Re: [patch 0/9] writeback data integrity and other fixes (take 3)

To: Nick Piggin <npiggin@xxxxxxx>, akpm@xxxxxxxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, Chris Mason <chris.mason@xxxxxxxxxx>
Subject: Re: [patch 0/9] writeback data integrity and other fixes (take 3)
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 29 Oct 2008 14:26:01 +1100
In-reply-to: <20081029031645.GE4985@disturbed>
Mail-followup-to: Nick Piggin <npiggin@xxxxxxx>, akpm@xxxxxxxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, Chris Mason <chris.mason@xxxxxxxxxx>
References: <20081028144715.683011000@xxxxxxx> <20081028153953.GB3082@xxxxxxxxxxxxx> <20081028222746.GB4985@disturbed> <20081029001653.GF15599@xxxxxxxxxxxxx> <20081029031645.GE4985@disturbed>
User-agent: Mutt/1.5.18 (2008-05-17)
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....

I'll write up a patch that covers all of these.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>