xfs
[Top] [All Lists]

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

To: Eric Sandeen <sandeen@xxxxxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
Subject: Re: [PATCH, RFC] xfs: add heuristic to flush on rename
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Fri, 25 Apr 2014 15:00:50 -0500
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <535ABA9D.2060305@xxxxxxxxxx>
References: <535ABA9D.2060305@xxxxxxxxxx>
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:24.0) Gecko/20100101 Thunderbird/24.4.0
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@xxxxxxxxxx>
> ---
> 
> 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@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
> 

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