All of lore.kernel.org
 help / color / mirror / Atom feed
* [Cluster-devel] [PATCH v2] GFS: write vfs inode attributes to disk
@ 2014-05-30  5:19 Benjamin Marzinski
  2014-05-30 10:11 ` Steven Whitehouse
  0 siblings, 1 reply; 2+ messages in thread
From: Benjamin Marzinski @ 2014-05-30  5:19 UTC (permalink / raw)
  To: cluster-devel.redhat.com

GFS wasn't ever updating mtime during mmaps to a file.  This patch makes
gfs_write_inode write the vfs inode attributes out to disk, so that they
get updated when the mmap data is written back to disk. It also makes
sure that the updated vfs timestamps don't get overwritten by older gfs
ones before they can be written out.

This patch is similar to my earlier version, but includes more checks in
gfs_write_inode to make sure that it can start a transaction, and changes
to inode_attr_in to keep updated vfs timestamps from getting overwritten.
These changes were made to resolve issues that caused bz #1066181 to fail
QA. I have verified that it is still possible to manually reset the inode
mtime to an earlier time using the touch command.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 gfs-kernel/src/gfs/inode.c     | 15 +++++++++------
 gfs-kernel/src/gfs/ops_super.c | 43 ++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/gfs-kernel/src/gfs/inode.c b/gfs-kernel/src/gfs/inode.c
index 454366a..52b0819 100644
--- a/gfs-kernel/src/gfs/inode.c
+++ b/gfs-kernel/src/gfs/inode.c
@@ -46,7 +46,7 @@
  */
 
 static void
-inode_attr_in(struct gfs_inode *ip, struct inode *inode)
+inode_attr_in(struct gfs_inode *ip, struct inode *inode, int force)
 {
 	unsigned int mode;
 
@@ -93,9 +93,12 @@ inode_attr_in(struct gfs_inode *ip, struct inode *inode)
 	inode->i_uid = ip->i_di.di_uid;
 	inode->i_gid = ip->i_di.di_gid;
 	i_size_write(inode, ip->i_di.di_size);
-	inode->i_atime.tv_sec = ip->i_di.di_atime;
-	inode->i_mtime.tv_sec = ip->i_di.di_mtime;
-	inode->i_ctime.tv_sec = ip->i_di.di_ctime;
+	if (force || ip->i_di.di_atime > inode->i_atime.tv_sec)
+		inode->i_atime.tv_sec = ip->i_di.di_atime;
+	if (force || ip->i_di.di_mtime > inode->i_mtime.tv_sec)
+		inode->i_mtime.tv_sec = ip->i_di.di_mtime;
+	if (force || ip->i_di.di_ctime > inode->i_ctime.tv_sec)
+		inode->i_ctime.tv_sec = ip->i_di.di_ctime;
 	inode->i_atime.tv_nsec = inode->i_mtime.tv_nsec = inode->i_ctime.tv_nsec = 0;
 	inode->i_blocks = ip->i_di.di_blocks <<
 		(ip->i_sbd->sd_sb.sb_bsize_shift - GFS_BASIC_BLOCK_SHIFT);
@@ -125,7 +128,7 @@ gfs_inode_attr_in(struct gfs_inode *ip)
 
 	inode = gfs_iget(ip, NO_CREATE);
 	if (inode) {
-		inode_attr_in(ip, inode);
+		inode_attr_in(ip, inode, 0);
 		iput(inode);
 	}
 
@@ -185,7 +188,7 @@ gfs_iget(struct gfs_inode *ip, int create)
 	if (!tmp)
 		return NULL;
 
-	inode_attr_in(ip, tmp);
+	inode_attr_in(ip, tmp, 1);
 
 	/* Attach GFS-specific ops vectors */
 	if (ip->i_di.di_type == GFS_FILE_REG) {
diff --git a/gfs-kernel/src/gfs/ops_super.c b/gfs-kernel/src/gfs/ops_super.c
index c111a2e..10dbff7 100644
--- a/gfs-kernel/src/gfs/ops_super.c
+++ b/gfs-kernel/src/gfs/ops_super.c
@@ -39,6 +39,7 @@
 #include "super.h"
 #include "sys.h"
 #include "mount.h"
+#include "trans.h"
 
 /**
  * gfs_write_inode - Make sure the inode is stable on the disk
@@ -51,14 +52,52 @@
 static int
 gfs_write_inode(struct inode *inode, int sync)
 {
+	int ret = 0;
 	struct gfs_inode *ip = get_v2ip(inode);
+	struct buffer_head *dibh;
+	struct gfs_holder i_gh;
+	int need_unlock = 0;
+
+	if (!ip)
+		return 0;
 
 	atomic_inc(&ip->i_sbd->sd_ops_super);
 
-	if (ip && sync)
+	if (current->flags & PF_MEMALLOC || get_transaction)
+		goto do_flush;
+
+	if (!gfs_glock_is_locked_by_me(ip->i_gl)) {
+		ret = gfs_glock_nq_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &i_gh);
+		if (ret)
+			goto do_flush;
+		need_unlock = 1;
+	}
+	else if (!gfs_glock_is_held_excl(ip->i_gl))
+		goto do_flush;
+	/* Trans may require:
+	   one dinode block. */
+	ret = gfs_trans_begin(ip->i_sbd, 1, 0);
+	if (ret)
+		goto do_unlock;
+
+	ret = gfs_get_inode_buffer(ip, &dibh);
+	if (ret == 0) {
+		gfs_inode_attr_out(ip);
+		gfs_trans_add_bh(ip->i_gl, dibh);
+		gfs_dinode_out(&ip->i_di, dibh->b_data);
+		brelse(dibh);
+	}
+
+	gfs_trans_end(ip->i_sbd);
+
+do_unlock:
+	if (need_unlock)
+		gfs_glock_dq_uninit(&i_gh);
+do_flush:
+	if (sync)
 		gfs_log_flush_glock(ip->i_gl, 0);
 
-	return 0;
+	return ret;
 }
 
 /**
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [Cluster-devel] [PATCH v2] GFS: write vfs inode attributes to disk
  2014-05-30  5:19 [Cluster-devel] [PATCH v2] GFS: write vfs inode attributes to disk Benjamin Marzinski
@ 2014-05-30 10:11 ` Steven Whitehouse
  0 siblings, 0 replies; 2+ messages in thread
From: Steven Whitehouse @ 2014-05-30 10:11 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Hi,

Looks good to me. Thanks for fixing this,

Steve.

On 30/05/14 06:19, Benjamin Marzinski wrote:
> GFS wasn't ever updating mtime during mmaps to a file.  This patch makes
> gfs_write_inode write the vfs inode attributes out to disk, so that they
> get updated when the mmap data is written back to disk. It also makes
> sure that the updated vfs timestamps don't get overwritten by older gfs
> ones before they can be written out.
>
> This patch is similar to my earlier version, but includes more checks in
> gfs_write_inode to make sure that it can start a transaction, and changes
> to inode_attr_in to keep updated vfs timestamps from getting overwritten.
> These changes were made to resolve issues that caused bz #1066181 to fail
> QA. I have verified that it is still possible to manually reset the inode
> mtime to an earlier time using the touch command.
>
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>   gfs-kernel/src/gfs/inode.c     | 15 +++++++++------
>   gfs-kernel/src/gfs/ops_super.c | 43 ++++++++++++++++++++++++++++++++++++++++--
>   2 files changed, 50 insertions(+), 8 deletions(-)
>
> diff --git a/gfs-kernel/src/gfs/inode.c b/gfs-kernel/src/gfs/inode.c
> index 454366a..52b0819 100644
> --- a/gfs-kernel/src/gfs/inode.c
> +++ b/gfs-kernel/src/gfs/inode.c
> @@ -46,7 +46,7 @@
>    */
>   
>   static void
> -inode_attr_in(struct gfs_inode *ip, struct inode *inode)
> +inode_attr_in(struct gfs_inode *ip, struct inode *inode, int force)
>   {
>   	unsigned int mode;
>   
> @@ -93,9 +93,12 @@ inode_attr_in(struct gfs_inode *ip, struct inode *inode)
>   	inode->i_uid = ip->i_di.di_uid;
>   	inode->i_gid = ip->i_di.di_gid;
>   	i_size_write(inode, ip->i_di.di_size);
> -	inode->i_atime.tv_sec = ip->i_di.di_atime;
> -	inode->i_mtime.tv_sec = ip->i_di.di_mtime;
> -	inode->i_ctime.tv_sec = ip->i_di.di_ctime;
> +	if (force || ip->i_di.di_atime > inode->i_atime.tv_sec)
> +		inode->i_atime.tv_sec = ip->i_di.di_atime;
> +	if (force || ip->i_di.di_mtime > inode->i_mtime.tv_sec)
> +		inode->i_mtime.tv_sec = ip->i_di.di_mtime;
> +	if (force || ip->i_di.di_ctime > inode->i_ctime.tv_sec)
> +		inode->i_ctime.tv_sec = ip->i_di.di_ctime;
>   	inode->i_atime.tv_nsec = inode->i_mtime.tv_nsec = inode->i_ctime.tv_nsec = 0;
>   	inode->i_blocks = ip->i_di.di_blocks <<
>   		(ip->i_sbd->sd_sb.sb_bsize_shift - GFS_BASIC_BLOCK_SHIFT);
> @@ -125,7 +128,7 @@ gfs_inode_attr_in(struct gfs_inode *ip)
>   
>   	inode = gfs_iget(ip, NO_CREATE);
>   	if (inode) {
> -		inode_attr_in(ip, inode);
> +		inode_attr_in(ip, inode, 0);
>   		iput(inode);
>   	}
>   
> @@ -185,7 +188,7 @@ gfs_iget(struct gfs_inode *ip, int create)
>   	if (!tmp)
>   		return NULL;
>   
> -	inode_attr_in(ip, tmp);
> +	inode_attr_in(ip, tmp, 1);
>   
>   	/* Attach GFS-specific ops vectors */
>   	if (ip->i_di.di_type == GFS_FILE_REG) {
> diff --git a/gfs-kernel/src/gfs/ops_super.c b/gfs-kernel/src/gfs/ops_super.c
> index c111a2e..10dbff7 100644
> --- a/gfs-kernel/src/gfs/ops_super.c
> +++ b/gfs-kernel/src/gfs/ops_super.c
> @@ -39,6 +39,7 @@
>   #include "super.h"
>   #include "sys.h"
>   #include "mount.h"
> +#include "trans.h"
>   
>   /**
>    * gfs_write_inode - Make sure the inode is stable on the disk
> @@ -51,14 +52,52 @@
>   static int
>   gfs_write_inode(struct inode *inode, int sync)
>   {
> +	int ret = 0;
>   	struct gfs_inode *ip = get_v2ip(inode);
> +	struct buffer_head *dibh;
> +	struct gfs_holder i_gh;
> +	int need_unlock = 0;
> +
> +	if (!ip)
> +		return 0;
>   
>   	atomic_inc(&ip->i_sbd->sd_ops_super);
>   
> -	if (ip && sync)
> +	if (current->flags & PF_MEMALLOC || get_transaction)
> +		goto do_flush;
> +
> +	if (!gfs_glock_is_locked_by_me(ip->i_gl)) {
> +		ret = gfs_glock_nq_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &i_gh);
> +		if (ret)
> +			goto do_flush;
> +		need_unlock = 1;
> +	}
> +	else if (!gfs_glock_is_held_excl(ip->i_gl))
> +		goto do_flush;
> +	/* Trans may require:
> +	   one dinode block. */
> +	ret = gfs_trans_begin(ip->i_sbd, 1, 0);
> +	if (ret)
> +		goto do_unlock;
> +
> +	ret = gfs_get_inode_buffer(ip, &dibh);
> +	if (ret == 0) {
> +		gfs_inode_attr_out(ip);
> +		gfs_trans_add_bh(ip->i_gl, dibh);
> +		gfs_dinode_out(&ip->i_di, dibh->b_data);
> +		brelse(dibh);
> +	}
> +
> +	gfs_trans_end(ip->i_sbd);
> +
> +do_unlock:
> +	if (need_unlock)
> +		gfs_glock_dq_uninit(&i_gh);
> +do_flush:
> +	if (sync)
>   		gfs_log_flush_glock(ip->i_gl, 0);
>   
> -	return 0;
> +	return ret;
>   }
>   
>   /**



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2014-05-30 10:11 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-30  5:19 [Cluster-devel] [PATCH v2] GFS: write vfs inode attributes to disk Benjamin Marzinski
2014-05-30 10:11 ` Steven Whitehouse

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.