[Cluster-devel] [PATCH] gfs2: Fix xattr fsync

[Andrew Price <anprice@redhat.com>]

On Wed, Oct 04, 2017 at 08:17:38PM +0200, Andreas Gruenbacher wrote:
> Make sure that changing xattrs marks the corresponding inode dirty so
> that a subsequent fsync will sync those changes to disk.  We set
> I_DIRTY_SYNC as well as I_DIRTY_DATASYNC so that both fsync and
> fdatasync will sync xattr changes: xattrs can contain information
> critical to how the data can be accessed, so we don't want fdatasync
> to skip them.
> 
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>

Reviewed-by: Andrew Price <anprice@redhat.com>

The new __mark_inode_dirty() -> gfs2_dirty_inode() path looks ok in this context.

Andy

> ---
>  fs/gfs2/xattr.c | 40 ++++++++--------------------------------
>  1 file changed, 8 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c
> index 36ca70924935..588d4f21a273 100644
> --- a/fs/gfs2/xattr.c
> +++ b/fs/gfs2/xattr.c
> @@ -231,7 +231,6 @@ static int ea_dealloc_unstuffed(struct gfs2_inode *ip, struct buffer_head *bh,
>  	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
>  	struct gfs2_rgrpd *rgd;
>  	struct gfs2_holder rg_gh;
> -	struct buffer_head *dibh;
>  	__be64 *dataptrs;
>  	u64 bn = 0;
>  	u64 bstart = 0;
> @@ -308,13 +307,8 @@ static int ea_dealloc_unstuffed(struct gfs2_inode *ip, struct buffer_head *bh,
>  		ea->ea_num_ptrs = 0;
>  	}
>  
> -	error = gfs2_meta_inode_buffer(ip, &dibh);
> -	if (!error) {
> -		ip->i_inode.i_ctime = current_time(&ip->i_inode);
> -		gfs2_trans_add_meta(ip->i_gl, dibh);
> -		gfs2_dinode_out(ip, dibh->b_data);
> -		brelse(dibh);
> -	}
> +	ip->i_inode.i_ctime = current_time(&ip->i_inode);
> +	__mark_inode_dirty(&ip->i_inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
>  
>  	gfs2_trans_end(sdp);
>  
> @@ -755,7 +749,6 @@ static int ea_alloc_skeleton(struct gfs2_inode *ip, struct gfs2_ea_request *er,
>  			     ea_skeleton_call_t skeleton_call, void *private)
>  {
>  	struct gfs2_alloc_parms ap = { .target = blks };
> -	struct buffer_head *dibh;
>  	int error;
>  
>  	error = gfs2_rindex_update(GFS2_SB(&ip->i_inode));
> @@ -780,13 +773,8 @@ static int ea_alloc_skeleton(struct gfs2_inode *ip, struct gfs2_ea_request *er,
>  	if (error)
>  		goto out_end_trans;
>  
> -	error = gfs2_meta_inode_buffer(ip, &dibh);
> -	if (!error) {
> -		ip->i_inode.i_ctime = current_time(&ip->i_inode);
> -		gfs2_trans_add_meta(ip->i_gl, dibh);
> -		gfs2_dinode_out(ip, dibh->b_data);
> -		brelse(dibh);
> -	}
> +	ip->i_inode.i_ctime = current_time(&ip->i_inode);
> +	__mark_inode_dirty(&ip->i_inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
>  
>  out_end_trans:
>  	gfs2_trans_end(GFS2_SB(&ip->i_inode));
> @@ -897,7 +885,6 @@ static int ea_set_simple_noalloc(struct gfs2_inode *ip, struct buffer_head *bh,
>  				 struct gfs2_ea_header *ea, struct ea_set *es)
>  {
>  	struct gfs2_ea_request *er = es->es_er;
> -	struct buffer_head *dibh;
>  	int error;
>  
>  	error = gfs2_trans_begin(GFS2_SB(&ip->i_inode), RES_DINODE + 2 * RES_EATTR, 0);
> @@ -914,14 +901,9 @@ static int ea_set_simple_noalloc(struct gfs2_inode *ip, struct buffer_head *bh,
>  	if (es->es_el)
>  		ea_set_remove_stuffed(ip, es->es_el);
>  
> -	error = gfs2_meta_inode_buffer(ip, &dibh);
> -	if (error)
> -		goto out;
>  	ip->i_inode.i_ctime = current_time(&ip->i_inode);
> -	gfs2_trans_add_meta(ip->i_gl, dibh);
> -	gfs2_dinode_out(ip, dibh->b_data);
> -	brelse(dibh);
> -out:
> +	__mark_inode_dirty(&ip->i_inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
> +
>  	gfs2_trans_end(GFS2_SB(&ip->i_inode));
>  	return error;
>  }
> @@ -1117,7 +1099,6 @@ static int ea_remove_stuffed(struct gfs2_inode *ip, struct gfs2_ea_location *el)
>  {
>  	struct gfs2_ea_header *ea = el->el_ea;
>  	struct gfs2_ea_header *prev = el->el_prev;
> -	struct buffer_head *dibh;
>  	int error;
>  
>  	error = gfs2_trans_begin(GFS2_SB(&ip->i_inode), RES_DINODE + RES_EATTR, 0);
> @@ -1138,13 +1119,8 @@ static int ea_remove_stuffed(struct gfs2_inode *ip, struct gfs2_ea_location *el)
>  		ea->ea_type = GFS2_EATYPE_UNUSED;
>  	}
>  
> -	error = gfs2_meta_inode_buffer(ip, &dibh);
> -	if (!error) {
> -		ip->i_inode.i_ctime = current_time(&ip->i_inode);
> -		gfs2_trans_add_meta(ip->i_gl, dibh);
> -		gfs2_dinode_out(ip, dibh->b_data);
> -		brelse(dibh);
> -	}
> +	ip->i_inode.i_ctime = current_time(&ip->i_inode);
> +	__mark_inode_dirty(&ip->i_inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
>  
>  	gfs2_trans_end(GFS2_SB(&ip->i_inode));
>  
> -- 
> 2.13.5
>