[Cluster-devel] [GFS2 PATCH] gfs2: simplify code with nq_init functions

[Cluster-devel] [GFS2 PATCH] gfs2: simplify code with nq_init functions

[Bob Peterson]

Hi,

This patch simply cleans up some code to make it more readable by
using helper functions gfs2_glock_nq_init and dq_uninit instead of
doing things manually.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/aops.c | 10 ++++------
 fs/gfs2/bmap.c | 25 +++++++------------------
 fs/gfs2/file.c | 34 +++++++++++++---------------------
 3 files changed, 24 insertions(+), 45 deletions(-)

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 72c9560f4467..64bde9796ac3 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -600,14 +600,12 @@ static void gfs2_readahead(struct readahead_control *rac)
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct gfs2_holder gh;
 
-	gfs2_holder_init(ip->i_gl, LM_ST_SHARED, 0, &gh);
-	if (gfs2_glock_nq(&gh))
-		goto out_uninit;
+	if (gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED, 0, &gh))
+		return;
+
 	if (!gfs2_is_stuffed(ip))
 		mpage_readahead(rac, gfs2_block_map);
-	gfs2_glock_dq(&gh);
-out_uninit:
-	gfs2_holder_uninit(&gh);
+	gfs2_glock_dq_uninit(&gh);
 }
 
 /**
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 6306eaae378b..90313009dc12 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -990,28 +990,17 @@ int gfs2_lblk_to_dblk(struct inode *inode, u32 lblock, u64 *dblock)
 static int gfs2_write_lock(struct inode *inode)
 {
 	struct gfs2_inode *ip = GFS2_I(inode);
+	struct gfs2_inode *m_ip;
 	struct gfs2_sbd *sdp = GFS2_SB(inode);
 	int error;
 
-	gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &ip->i_gh);
-	error = gfs2_glock_nq(&ip->i_gh);
-	if (error)
-		goto out_uninit;
-	if (&ip->i_inode == sdp->sd_rindex) {
-		struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
-
-		error = gfs2_glock_nq_init(m_ip->i_gl, LM_ST_EXCLUSIVE,
-					   GL_NOCACHE, &m_ip->i_gh);
-		if (error)
-			goto out_unlock;
-	}
-	return 0;
+	error = gfs2_glock_nq_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &ip->i_gh);
+	if (error || &ip->i_inode != sdp->sd_rindex)
+		return error;
 
-out_unlock:
-	gfs2_glock_dq(&ip->i_gh);
-out_uninit:
-	gfs2_holder_uninit(&ip->i_gh);
-	return error;
+	m_ip = GFS2_I(sdp->sd_statfs_inode);
+	return gfs2_glock_nq_init(m_ip->i_gl, LM_ST_EXCLUSIVE, GL_NOCACHE,
+				  &m_ip->i_gh);
 }
 
 static void gfs2_write_unlock(struct inode *inode)
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index fe305e4bfd37..fa2fe43053ac 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -762,8 +762,8 @@ static int gfs2_fsync(struct file *file, loff_t start, loff_t end,
 
 static ssize_t gfs2_file_direct_read(struct kiocb *iocb, struct iov_iter *to)
 {
-	struct file *file = iocb->ki_filp;
-	struct gfs2_inode *ip = GFS2_I(file->f_mapping->host);
+	struct inode *inode = file_inode(iocb->ki_filp);
+	struct gfs2_inode *ip = GFS2_I(inode);
 	size_t count = iov_iter_count(to);
 	struct gfs2_holder gh;
 	ssize_t ret;
@@ -771,24 +771,21 @@ static ssize_t gfs2_file_direct_read(struct kiocb *iocb, struct iov_iter *to)
 	if (!count)
 		return 0; /* skip atime */
 
-	gfs2_holder_init(ip->i_gl, LM_ST_DEFERRED, 0, &gh);
-	ret = gfs2_glock_nq(&gh);
+	ret = gfs2_glock_nq_init(ip->i_gl, LM_ST_DEFERRED, 0, &gh);
 	if (ret)
-		goto out_uninit;
+		goto out;
 
 	ret = iomap_dio_rw(iocb, to, &gfs2_iomap_ops, NULL,
 			   is_sync_kiocb(iocb));
 
-	gfs2_glock_dq(&gh);
-out_uninit:
-	gfs2_holder_uninit(&gh);
+	gfs2_glock_dq_uninit(&gh);
+out:
 	return ret;
 }
 
 static ssize_t gfs2_file_direct_write(struct kiocb *iocb, struct iov_iter *from)
 {
-	struct file *file = iocb->ki_filp;
-	struct inode *inode = file->f_mapping->host;
+	struct inode *inode = file_inode(iocb->ki_filp);
 	struct gfs2_inode *ip = GFS2_I(inode);
 	size_t len = iov_iter_count(from);
 	loff_t offset = iocb->ki_pos;
@@ -803,22 +800,17 @@ static ssize_t gfs2_file_direct_write(struct kiocb *iocb, struct iov_iter *from)
 	 * unfortunately, have the option of only flushing a range like the
 	 * VFS does.
 	 */
-	gfs2_holder_init(ip->i_gl, LM_ST_DEFERRED, 0, &gh);
-	ret = gfs2_glock_nq(&gh);
+	ret = gfs2_glock_nq_init(ip->i_gl, LM_ST_DEFERRED, 0, &gh);
 	if (ret)
-		goto out_uninit;
-
-	/* Silently fall back to buffered I/O when writing beyond EOF */
-	if (offset + len > i_size_read(&ip->i_inode))
 		goto out;
 
-	ret = iomap_dio_rw(iocb, from, &gfs2_iomap_ops, NULL,
-			   is_sync_kiocb(iocb));
+	/* Silently fall back to buffered I/O when writing beyond EOF */
+	if (offset + len <= i_size_read(&ip->i_inode))
+		ret = iomap_dio_rw(iocb, from, &gfs2_iomap_ops, NULL,
+				   is_sync_kiocb(iocb));
 
+	gfs2_glock_dq_uninit(&gh);
 out:
-	gfs2_glock_dq(&gh);
-out_uninit:
-	gfs2_holder_uninit(&gh);
 	return ret;
 }
 

[Cluster-devel] [GFS2 PATCH] gfs2: simplify code with nq_init functions

[Andreas Gruenbacher]

Hi Bob,

On Tue, Jun 16, 2020 at 7:19 PM Bob Peterson <rpeterso@redhat.com> wrote:
> This patch simply cleans up some code to make it more readable by
> using helper functions gfs2_glock_nq_init and dq_uninit instead of
> doing things manually.

the more complicated call sequence [gfs2_holder_init ... gfs2_glock_nq
... gfs2_glock_dq ... gfs2_holder_uninit] produces better code than
just [gfs2_glock_nq_init ... gfs2_glock_dq_uninit]. I wonder if
there's a way to implement those primitives so that the compiler will
automatically produce the better code?

Andreas