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@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxx
|