Re: [PATCH] Fix ext2 error reporting on fsync

[Jan Kara <jack@suse.cz>]

> Jan Kara <jack@suse.cz> wrote:
> >
  <snip>
> > +	 */
> > +	if (test_and_clear_bit(AS_EIO, &sb->s_bdev->bd_inode->i_mapping->flags)) {
> > +		ext2_error(sb, "ext2_sync_file", "metadata io error");
> > +		if (!ret)
> > +			ret = -EIO;
> > +	}
> >  	if (!(inode->i_state & I_DIRTY))
> >  		return ret;
> >  	if (datasync && !(inode->i_state & I_DIRTY_DATASYNC))
> > 
> 
> This is desperation stuff, isn't it?
> 
> It would be better to stick a BH_WriteError into buffer_head.b_state, set
> that in the I/O completion handler, check it in fsync_buffers_list()?
  It actually depends on the desired behaviour of fsync() in case of
IO errors. Consider the following scenario:
  * process writes some data
    - async writing starts in background and gets an IO error on some
      metadata buffer
  * process calls fsync()
    - all the remaining buffers are written without problems

  If I understand your solution correctly, it need not report any error
in this case as buffer with IO error could be already removed from
mapping->private_list by try_to_free_buffers(). But it would be nice to
report to the user that we were not able to write all the data...

								Honza
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs