Re: [PATCH, RFC] xfs: add heuristic to flush on rename

[Eric Sandeen <sandeen@sandeen.net>]

On 4/25/14, 2:42 PM, Eric Sandeen wrote:
> Add a heuristic to flush data to a file which looks like it's
> going through a tmpfile/rename dance, but not fsynced.
> 
> I had a report of a system with many 0-length files after
> package updates; as it turns out, the user had basically
> done 'yum update' and punched the power button when it was
> done.
> 
> Granted, the admin should not do this.  Granted, the package
> manager should ensure persistence of files it updated.
> 
> Ext4, however, added a heuristic like this for just this case;
> someone who writes file.tmp, then renames over file, but
> never issues an fsync.
> 
> Now, this does smack of O_PONIES, but I would hope that it's
> fairly benign.  If someone already synced the tmpfile, it's
> a no-op.
> 
> And it's not THAT far off our "flush on close if the file was
> truncated" heuristic.
> 
> Comments?  Flames?  Testing anyone would like to see?
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index ef1ca01..5c95ef5 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -371,6 +371,19 @@ xfs_vn_rename(
>  	xfs_dentry_to_name(&oname, odentry, 0);
>  	xfs_dentry_to_name(&nname, ndentry, odentry->d_inode->i_mode);
>  
> +	/*
> +	 * If we are renaming a just-written file over an existing
> +	 * file, be pedantic and flush it out if it looks like somebody
> +	 * is doing a tmpfile dance, and didn't fsync.  Best effort;
> +	 * ignore errors.
> +	 */
> +	if (new_inode) {
> +		xfs_inode_t *ip = XFS_I(odentry->d_inode);
> +
> +		if (VN_DIRTY(VFS_I(ip)) && ip->i_delayed_blks > 0)
> +			filemap_flush(new_inode->i_mapping);

Uhhh I flushed the wrong inode (thanks Brian!) but you get the idea ;)

should be:

+			filemap_flush(odentry->d_inode->i_mapping);

-Eric

> +	}
> +
>  	return -xfs_rename(XFS_I(odir), &oname, XFS_I(odentry->d_inode),
>  			   XFS_I(ndir), &nname, new_inode ?
>  						XFS_I(new_inode) : NULL);
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs