gfs2: Use inode_newsize_ok and get_write_access in fallocate

PATCH New Andrew Price 2 participants 4 messages Nov 12, 2014 – Nov 17, 2014 GFS2 file system

Share & tools

Andrew Price

author

2014-11-12 17:24 UTC #0 lore raw

PATCH 1/3 gfs2: Use inode_newsize_ok and get_write_access in fallocate

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 <[email protected]>

---
 fs/gfs2/file.c | 66 ++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 44 insertions(+), 22 deletions(-)

fs/gfs2/file.c +44 ~~-22~~

index 80dd44d..08ee6ce 100644
@@ -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

Signature · 1 line

1.9.3

Andrew Price

author

2014-11-12 17:24 UTC #1 lore raw

PATCH 2/3 gfs2: Update i_size properly on fallocate

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 <[email protected]>

---
 fs/gfs2/file.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

fs/gfs2/file.c +6 -9

index 08ee6ce..b6d91bc 100644
@@ -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);

Signature · 1 line

1.9.3

Andrew Price

author

2014-11-12 17:24 UTC #2 lore raw

PATCH 3/3 gfs2: Update timestamps on fallocate

In reply to 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 <[email protected]>

---
 fs/gfs2/file.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

fs/gfs2/file.c +2 -1

index b6d91bc..5b673cc 100644
@@ -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);

Signature · 1 line

1.9.3

Steven Whitehouse

2014-11-17 12:03 UTC #3 lore raw

PATCH 1/3 gfs2: Use inode_newsize_ok and get_write_access in fallocate

In reply to Andrew Price

Hi,

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

Steve.

Quoted 130 lines · Andrew Price

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 <[email protected]>

---
  fs/gfs2/file.c | 66 ++++++++++++++++++++++++++++++++++++++--------------------
  1 file changed, 44 insertions(+), 22 deletions(-)

fs/gfs2/file.c +44 ~~-22~~

index 80dd44d..08ee6ce 100644
@@ -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