Re: [PATCH 1/7] xfs: ensure sync write errors are returned

[Dave Chinner <david@fromorbit.com>]

On Thu, Dec 16, 2010 at 02:50:49PM -0600, Alex Elder wrote:
> On Wed, 2010-12-15 at 12:23 +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > xfs_file_aio_write() only returns the error from synchronous
> > flushing of the data and inode if error == 0. At the point where
> > error is being checked, it is guaranteed to be > 0. Therefore any
> 
> Actually, we have this above the affected code:
>         ...
>         error = -ret;   
>         if (ret <= 0)
>                 goto out_unlock_internal;
> 
> So ret must be positive, therefore error is negative
> (the negative of the number of bytes written).  In
> other words, we enter this block without having seen
> an error.
> 
> The return at the end of the function is:
> 	return -error;
> 
> And by the convoluted logic here, that means that the
> value of error should be a negative byte count if
> successful, or a positive errno otherwise.

Oh, right, yeah, I screwed that up, didn't I?

> And since filemap_write_and_wait_range() returns a
> negative errno, your fix doesn't look right to me.

I will fix it up.

> The existing code is wrong and should be fixed, but a
> better fix might make the meaning of the variable "error"
> a little less weird.

The real problem is that xfs functions return positive errors, and
the linux functions return negative errors.  It would be much less
of a hassle if we fixed that problem...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs