From mboxrd@z Thu Jan  1 00:00:00 1970
From: Steven Whitehouse <swhiteho@redhat.com>
Date: Tue, 28 May 2013 15:20:23 +0100
Subject: [Cluster-devel] [GFS2 PATCH] GFS2: Increase i_writecount during
 gfs2_setattr_size
In-Reply-To: <967486797.29571199.1369749884232.JavaMail.root@redhat.com>
References: <967486797.29571199.1369749884232.JavaMail.root@redhat.com>
Message-ID: <1369750823.2700.54.camel@menhir>
List-Id: <cluster-devel.redhat.com>
To: cluster-devel.redhat.com
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

Hi,

On Tue, 2013-05-28 at 10:04 -0400, Bob Peterson wrote:
> Hi,
> 
> This patch calls get_write_access in a few functions. This
> merely increases inode->i_writecount for the duration of the function.
> That will ensure that any file closes won't delete the inode's
> multi-block reservation while the function is running.
> 
> Regards,
> 
> Bob Peterson
> Red Hat File Systems
> 
> Signed-off-by: Bob Peterson <rpeterso@redhat.com> 
> ---
> diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
> index 1dc9a13..3f8ad82 100644
> --- a/fs/gfs2/bmap.c
> +++ b/fs/gfs2/bmap.c
> @@ -1286,17 +1286,26 @@ int gfs2_setattr_size(struct inode *inode, u64 newsize)
>  	if (ret)
>  		return ret;
>  
> +	ret = get_write_access(inode);
> +	if (ret)
> +		return ret;
> +	
>  	inode_dio_wait(inode);
>  
>  	ret = gfs2_rs_alloc(GFS2_I(inode));
>  	if (ret)
> -		return ret;
> +		goto out;
>  
>  	oldsize = inode->i_size;
> -	if (newsize >= oldsize)
> -		return do_grow(inode, newsize);
> +	if (newsize >= oldsize) {
> +		ret = do_grow(inode, newsize);
> +		goto out;
> +	}
>  
> -	return do_shrink(inode, oldsize, newsize);
> +	ret = do_shrink(inode, oldsize, newsize);
> +out:
> +	put_write_access(inode);
> +	return ret;
>  }
>  
>  int gfs2_truncatei_resume(struct gfs2_inode *ip)
> diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
> index acd1676..ad0dc38 100644
> --- a/fs/gfs2/file.c
> +++ b/fs/gfs2/file.c
> @@ -402,16 +402,20 @@ static int gfs2_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
>  	/* Update file times before taking page lock */
>  	file_update_time(vma->vm_file);
>  
> +	ret = get_write_access(inode);
> +	if (ret)
> +		goto out;
> +
>  	ret = gfs2_rs_alloc(ip);
>  	if (ret)
> -		return ret;
> +		goto out_write_access;
>  
>  	gfs2_size_hint(vma->vm_file, pos, PAGE_CACHE_SIZE);
>  
>  	gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &gh);
>  	ret = gfs2_glock_nq(&gh);
>  	if (ret)
> -		goto out;
> +		goto out_uninit;
>  
>  	set_bit(GLF_DIRTY, &ip->i_gl->gl_flags);
>  	set_bit(GIF_SW_PAGED, &ip->i_flags);
> @@ -480,12 +484,15 @@ out_quota_unlock:
>  	gfs2_quota_unlock(ip);
>  out_unlock:
>  	gfs2_glock_dq(&gh);
> -out:
> +out_uninit:
>  	gfs2_holder_uninit(&gh);
>  	if (ret == 0) {
>  		set_page_dirty(page);
>  		wait_for_stable_page(page);
>  	}
> +out_write_access:
> +	put_write_access(inode);
> +out:
>  	sb_end_pagefault(inode->i_sb);
>  	return block_page_mkwrite_return(ret);
>  }
> @@ -594,10 +601,10 @@ static int gfs2_release(struct inode *inode, struct file *file)
>  	kfree(file->private_data);
>  	file->private_data = NULL;
>  
> -	if ((file->f_mode & FMODE_WRITE) &&
> -	    (atomic_read(&inode->i_writecount) == 1))
> -		gfs2_rs_delete(ip);
> +	if (!(file->f_mode & FMODE_WRITE))
> +		return 0;
>  
> +	gfs2_rs_delete(ip);
>  	return 0;
>  }
>  
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index 5232525..9809156 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -638,8 +638,10 @@ void gfs2_rs_deltree(struct gfs2_blkreserv *rs)
>   */
>  void gfs2_rs_delete(struct gfs2_inode *ip)
>  {
> +	struct inode *inode = &ip->i_inode;
> +
>  	down_write(&ip->i_rw_mutex);
> -	if (ip->i_res) {
> +	if (ip->i_res && atomic_read(&inode->i_writecount) <= 1) {
>  		gfs2_rs_deltree(ip->i_res);
>  		BUG_ON(ip->i_res->rs_free);
>  		kmem_cache_free(gfs2_rsrv_cachep, ip->i_res);
> 

Are there any other callers of gfs2_rs_delete where it is no appropriate
to have this new test?

I assume that the issue is that this writecount test needs to be under
the i_rw_mutex?

Steve.