Re: [PATCH] cifs: Detect errors in cifs_writepages when called from pdflush

On Fri, 17 Apr 2009 22:44:38 +1000
Peter Schwenke <[email protected]> wrote:

> 
> The return code isn't checked in the case of cifs_writepages being
> invoked as a result of pdflush. This results in data integrity
> problems when errors such as a network break occur when
> CIFSSMBWrite2 is called.  The resulting EAGAIN isn't acted
> on.
> 
> Signed-off-by: Peter Schwenke <[email protected]>
> ---
>  fs/cifs/file.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 042b122..8cc1094 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -1374,6 +1374,17 @@ retry:
>  						set_bit(AS_ENOSPC, &mapping->flags);
>  					else
>  						set_bit(AS_EIO, &mapping->flags);
> +					/*
> +					 * The return code isn't checked in the
> +					 * case of cifs_writepages being
> +					 * invoked as a result of pdflush.
> +					 * See generic_sync_sb_inodes()
> +					 */
> +					if (current_is_pdflush())
> +						if (rc == -ENOSPC)
> +							CIFS_I(mapping->host)->write_behind_rc =  rc;
> +						else
> +							CIFS_I(mapping->host)->write_behind_rc =  -EIO;
>  				} else {
>  					cifs_stats_bytes_written(cifs_sb->tcon,
>  								 bytes_written);

Looks reasonable, but doesn't the compiler complain about ambiguous
else's here? You might need to add some curly braces here. Also, we should
already have a 'cifsi' variable in that function that's usable instead
of having to call CIFS_I. 

Thanks,
-- 
Jeff Layton <[email protected]>