Re: [PATCH v8 16/18] ext4: use errseq_t based error handling for reporting data writeback errors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2017-06-29 at 07:12 -0700, Christoph Hellwig wrote:
> > -	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
> > -		return -EIO;
> > +	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb)))) {
> > +		ret = -EIO;
> > +		goto out;
> > +	}
> 
> This just seems to add a call to trace_ext4_sync_file_exit for this
> case, which seems unrelated to the patch.
> 
> >  	if (ret)
> > -		return ret;
> > +		goto out;
> > +
> 
> Same here.
> 
> >  	/*
> >  	 * data=writeback,ordered:
> >  	 *  The caller's filemap_fdatawrite()/wait will sync the data.
> > @@ -152,7 +155,7 @@ int ext4_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
> >  		needs_barrier = true;
> >  	ret = jbd2_complete_transaction(journal, commit_tid);
> >  	if (needs_barrier) {
> > -	issue_flush:
> > +issue_flush:
> >  		err = blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL);
> 
> And while I much prefer your new label placement it also doesn't
> seem to belong into this patch.

I revised the patch description earlier to say:

    While we're at it, ensure we always "goto out" instead of just
    returning directly, so that we always hit the exit tracepoint.

...but I'm fine with taking that out if you prefer.
-- 
Jeff Layton <jlayton@xxxxxxxxxxxxxxx>



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux