[Cluster-devel] [PATCH 1/3] gfs2: Use inode_newsize_ok and get_write_access in fallocate

[Cluster-devel] [PATCH 1/3] gfs2: Use inode_newsize_ok and get_write_access in fallocate

[Andrew Price]

gfs2_fallocate wasn't checking inode_newsize_ok nor get_write_access.
Split out the context setup and inode locking pieces into a separate
function to make it more clear and add these missing calls.

inode_newsize_ok is called conditional on FALLOC_FL_KEEP_SIZE as there
is no need to enforce a file size limit if it isn't going to change.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 fs/gfs2/file.c | 66 ++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 44 insertions(+), 22 deletions(-)

diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index 80dd44d..08ee6ce 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -796,8 +796,7 @@ static void calc_max_reserv(struct gfs2_inode *ip, loff_t max, loff_t *len,
 	}
 }
 
-static long gfs2_fallocate(struct file *file, int mode, loff_t offset,
-			   loff_t len)
+static long __gfs2_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
 {
 	struct inode *inode = file_inode(file);
 	struct gfs2_sbd *sdp = GFS2_SB(inode);
@@ -811,14 +810,9 @@ static long gfs2_fallocate(struct file *file, int mode, loff_t offset,
 	loff_t bsize_mask = ~((loff_t)sdp->sd_sb.sb_bsize - 1);
 	loff_t next = (offset + len - 1) >> sdp->sd_sb.sb_bsize_shift;
 	loff_t max_chunk_size = UINT_MAX & bsize_mask;
-	struct gfs2_holder gh;
 
 	next = (next + 1) << sdp->sd_sb.sb_bsize_shift;
 
-	/* We only support the FALLOC_FL_KEEP_SIZE mode */
-	if (mode & ~FALLOC_FL_KEEP_SIZE)
-		return -EOPNOTSUPP;
-
 	offset &= bsize_mask;
 
 	len = next - offset;
@@ -829,17 +823,6 @@ static long gfs2_fallocate(struct file *file, int mode, loff_t offset,
 	if (bytes == 0)
 		bytes = sdp->sd_sb.sb_bsize;
 
-	error = gfs2_rs_alloc(ip);
-	if (error)
-		return error;
-
-	mutex_lock(&inode->i_mutex);
-
-	gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &gh);
-	error = gfs2_glock_nq(&gh);
-	if (unlikely(error))
-		goto out_uninit;
-
 	gfs2_size_hint(file, offset, len);
 
 	while (len > 0) {
@@ -852,8 +835,7 @@ static long gfs2_fallocate(struct file *file, int mode, loff_t offset,
 		}
 		error = gfs2_quota_lock_check(ip);
 		if (error)
-			goto out_unlock;
-
+			return error;
 retry:
 		gfs2_write_calc_reserv(ip, bytes, &data_blocks, &ind_blocks);
 
@@ -897,18 +879,58 @@ retry:
 
 	if (error == 0)
 		error = generic_write_sync(file, pos, count);
-	goto out_unlock;
+	return error;
 
 out_trans_fail:
 	gfs2_inplace_release(ip);
 out_qunlock:
 	gfs2_quota_unlock(ip);
+	return error;
+}
+
+static long gfs2_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
+{
+	struct inode *inode = file_inode(file);
+	struct gfs2_inode *ip = GFS2_I(inode);
+	struct gfs2_holder gh;
+	int ret;
+
+	if (mode & ~FALLOC_FL_KEEP_SIZE)
+		return -EOPNOTSUPP;
+
+	mutex_lock(&inode->i_mutex);
+
+	gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &gh);
+	ret = gfs2_glock_nq(&gh);
+	if (ret)
+		goto out_uninit;
+
+	if (!(mode & FALLOC_FL_KEEP_SIZE) &&
+	    (offset + len) > inode->i_size) {
+		ret = inode_newsize_ok(inode, offset + len);
+		if (ret)
+			goto out_unlock;
+	}
+
+	ret = get_write_access(inode);
+	if (ret)
+		goto out_unlock;
+
+	ret = gfs2_rs_alloc(ip);
+	if (ret)
+		goto out_putw;
+
+	ret = __gfs2_fallocate(file, mode, offset, len);
+	if (ret)
+		gfs2_rs_deltree(ip->i_res);
+out_putw:
+	put_write_access(inode);
 out_unlock:
 	gfs2_glock_dq(&gh);
 out_uninit:
 	gfs2_holder_uninit(&gh);
 	mutex_unlock(&inode->i_mutex);
-	return error;
+	return ret;
 }
 
 #ifdef CONFIG_GFS2_FS_LOCKING_DLM
-- 
1.9.3

[Cluster-devel] [PATCH 2/3] gfs2: Update i_size properly on fallocate

[Andrew Price]

This addresses an issue caught by fsx where the inode size was not being
updated to the expected value after fallocate(2) with mode 0.

The problem was caused by the offset and len parameters being converted
to multiples of the file system's block size, so i_size would be rounded
up to the nearest block size multiple instead of the requested size.

This replaces the per-chunk i_size updates with a single i_size_write on
successful completion of the operation.  With this patch gfs2 gets
through a complete run of fsx.

For clarity, the check for (error == 0) following the loop is removed as
all failures before that point jump to out_* labels or return.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 fs/gfs2/file.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index 08ee6ce..b6d91bc 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -728,7 +728,6 @@ static int fallocate_chunk(struct inode *inode, loff_t offset, loff_t len,
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct buffer_head *dibh;
 	int error;
-	loff_t size = len;
 	unsigned int nr_blks;
 	sector_t lblock = offset >> inode->i_blkbits;
 
@@ -762,11 +761,6 @@ static int fallocate_chunk(struct inode *inode, loff_t offset, loff_t len,
 			goto out;
 		}
 	}
-	if (offset + size > inode->i_size && !(mode & FALLOC_FL_KEEP_SIZE))
-		i_size_write(inode, offset + size);
-
-	mark_inode_dirty(inode);
-
 out:
 	brelse(dibh);
 	return error;
@@ -877,9 +871,12 @@ retry:
 		gfs2_quota_unlock(ip);
 	}
 
-	if (error == 0)
-		error = generic_write_sync(file, pos, count);
-	return error;
+	if (!(mode & FALLOC_FL_KEEP_SIZE) && (pos + count) > inode->i_size) {
+		i_size_write(inode, pos + count);
+		mark_inode_dirty(inode);
+	}
+
+	return generic_write_sync(file, pos, count);
 
 out_trans_fail:
 	gfs2_inplace_release(ip);
-- 
1.9.3

[Cluster-devel] [PATCH 3/3] gfs2: Update timestamps on fallocate

[Andrew Price]

gfs2_fallocate() wasn't updating ctime and mtime when modifying the
inode. Add a call to file_update_time() to do that.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 fs/gfs2/file.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index b6d91bc..5b673cc 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -873,7 +873,8 @@ retry:
 
 	if (!(mode & FALLOC_FL_KEEP_SIZE) && (pos + count) > inode->i_size) {
 		i_size_write(inode, pos + count);
-		mark_inode_dirty(inode);
+		/* Marks the inode as dirty */
+		file_update_time(file);
 	}
 
 	return generic_write_sync(file, pos, count);
-- 
1.9.3

[Cluster-devel] [PATCH 1/3] gfs2: Use inode_newsize_ok and get_write_access in fallocate

[Steven Whitehouse]

Hi,

All three patches now in the -nmw tree. Thanks,

Steve.

On 12/11/14 17:24, Andrew Price wrote:
> gfs2_fallocate wasn't checking inode_newsize_ok nor get_write_access.
> Split out the context setup and inode locking pieces into a separate
> function to make it more clear and add these missing calls.
>
> inode_newsize_ok is called conditional on FALLOC_FL_KEEP_SIZE as there
> is no need to enforce a file size limit if it isn't going to change.
>
> Signed-off-by: Andrew Price <anprice@redhat.com>
> ---
>   fs/gfs2/file.c | 66 ++++++++++++++++++++++++++++++++++++++--------------------
>   1 file changed, 44 insertions(+), 22 deletions(-)
>
> diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
> index 80dd44d..08ee6ce 100644
> --- a/fs/gfs2/file.c
> +++ b/fs/gfs2/file.c
> @@ -796,8 +796,7 @@ static void calc_max_reserv(struct gfs2_inode *ip, loff_t max, loff_t *len,
>   	}
>   }
>   
> -static long gfs2_fallocate(struct file *file, int mode, loff_t offset,
> -			   loff_t len)
> +static long __gfs2_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
>   {
>   	struct inode *inode = file_inode(file);
>   	struct gfs2_sbd *sdp = GFS2_SB(inode);
> @@ -811,14 +810,9 @@ static long gfs2_fallocate(struct file *file, int mode, loff_t offset,
>   	loff_t bsize_mask = ~((loff_t)sdp->sd_sb.sb_bsize - 1);
>   	loff_t next = (offset + len - 1) >> sdp->sd_sb.sb_bsize_shift;
>   	loff_t max_chunk_size = UINT_MAX & bsize_mask;
> -	struct gfs2_holder gh;
>   
>   	next = (next + 1) << sdp->sd_sb.sb_bsize_shift;
>   
> -	/* We only support the FALLOC_FL_KEEP_SIZE mode */
> -	if (mode & ~FALLOC_FL_KEEP_SIZE)
> -		return -EOPNOTSUPP;
> -
>   	offset &= bsize_mask;
>   
>   	len = next - offset;
> @@ -829,17 +823,6 @@ static long gfs2_fallocate(struct file *file, int mode, loff_t offset,
>   	if (bytes == 0)
>   		bytes = sdp->sd_sb.sb_bsize;
>   
> -	error = gfs2_rs_alloc(ip);
> -	if (error)
> -		return error;
> -
> -	mutex_lock(&inode->i_mutex);
> -
> -	gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &gh);
> -	error = gfs2_glock_nq(&gh);
> -	if (unlikely(error))
> -		goto out_uninit;
> -
>   	gfs2_size_hint(file, offset, len);
>   
>   	while (len > 0) {
> @@ -852,8 +835,7 @@ static long gfs2_fallocate(struct file *file, int mode, loff_t offset,
>   		}
>   		error = gfs2_quota_lock_check(ip);
>   		if (error)
> -			goto out_unlock;
> -
> +			return error;
>   retry:
>   		gfs2_write_calc_reserv(ip, bytes, &data_blocks, &ind_blocks);
>   
> @@ -897,18 +879,58 @@ retry:
>   
>   	if (error == 0)
>   		error = generic_write_sync(file, pos, count);
> -	goto out_unlock;
> +	return error;
>   
>   out_trans_fail:
>   	gfs2_inplace_release(ip);
>   out_qunlock:
>   	gfs2_quota_unlock(ip);
> +	return error;
> +}
> +
> +static long gfs2_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
> +{
> +	struct inode *inode = file_inode(file);
> +	struct gfs2_inode *ip = GFS2_I(inode);
> +	struct gfs2_holder gh;
> +	int ret;
> +
> +	if (mode & ~FALLOC_FL_KEEP_SIZE)
> +		return -EOPNOTSUPP;
> +
> +	mutex_lock(&inode->i_mutex);
> +
> +	gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &gh);
> +	ret = gfs2_glock_nq(&gh);
> +	if (ret)
> +		goto out_uninit;
> +
> +	if (!(mode & FALLOC_FL_KEEP_SIZE) &&
> +	    (offset + len) > inode->i_size) {
> +		ret = inode_newsize_ok(inode, offset + len);
> +		if (ret)
> +			goto out_unlock;
> +	}
> +
> +	ret = get_write_access(inode);
> +	if (ret)
> +		goto out_unlock;
> +
> +	ret = gfs2_rs_alloc(ip);
> +	if (ret)
> +		goto out_putw;
> +
> +	ret = __gfs2_fallocate(file, mode, offset, len);
> +	if (ret)
> +		gfs2_rs_deltree(ip->i_res);
> +out_putw:
> +	put_write_access(inode);
>   out_unlock:
>   	gfs2_glock_dq(&gh);
>   out_uninit:
>   	gfs2_holder_uninit(&gh);
>   	mutex_unlock(&inode->i_mutex);
> -	return error;
> +	return ret;
>   }
>   
>   #ifdef CONFIG_GFS2_FS_LOCKING_DLM