xfs
[Top] [All Lists]

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

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 1/7] xfs: ensure sync write errors are returned
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 17 Dec 2010 08:57:12 +1100
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20101216115738.GB20445@xxxxxxxxxxxxx>
References: <1292376208-16282-1-git-send-email-david@xxxxxxxxxxxxx> <1292376208-16282-2-git-send-email-david@xxxxxxxxxxxxx> <20101216115738.GB20445@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
On Thu, Dec 16, 2010 at 06:57:38AM -0500, Christoph Hellwig wrote:
> >             error2 = filemap_write_and_wait_range(mapping, pos, end);
> > -           if (!error)
> > +           if (error2)
> >                     error = error2;
> >             if (need_i_mutex)
> >                     mutex_lock(&inode->i_mutex);
> > @@ -777,7 +777,7 @@ write_retry:
> >  
> >             error2 = -xfs_file_fsync(file,
> >                                      (file->f_flags & __O_SYNC) ? 0 : 1);
> > -           if (!error)
> > +           if (error2 && error >= 0)
> >                     error = error2;
> 
> Shouldn't the filemap_write_and_wait_range clause use a similar check as
> this one?

No need, because when we enter the branch, error is guaranteed to be
greater than zero as the code jumps over this branch if it is <= 0.
Hence we only need to overwrite error  with error2 from
filemap_write_and_wait_range() if error2 actually contains an error
(i.e. < 0).

The second hunk then has to deal with the fact that error can have
any value, and we only want to overwrite error if it doesn't already
contain an error value and error2 does. Hence the check for error >=
0 to ensure we don't overwrite an error from
filemap_write_and_wait_range().

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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