From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Whitehouse Date: Fri, 06 Jun 2014 20:51:09 +0100 Subject: [Cluster-devel] [PATCH] GFS: write vfs inode attributes to disk In-Reply-To: <1402082219-31814-1-git-send-email-bmarzins@redhat.com> References: <1402082219-31814-1-git-send-email-bmarzins@redhat.com> Message-ID: <53921BAD.3000802@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Hi, Yes, its not ideal, but it is a worthwhile step forward I think. ACK, Steve. On 06/06/14 20:16, 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 if it can, so > that they get updated when the mmap data is written back to disk. > Unfortunately, gfs_write_inode can only do this if it was called with the > inode already exclusively locked, and not already in a transaction. The > patch 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 previous version, except that in now doesn't > do it's own locking. This means it can only work when it happens to get > called with the glock already held. This is necessary to preserve the > correct locking order. The down side is that it does not write out the > mtime in many of the cases where it should. > > Signed-off-by: Benjamin Marzinski > --- > gfs-kernel/src/gfs/inode.c | 15 +++++++++------ > gfs-kernel/src/gfs/ops_super.c | 31 +++++++++++++++++++++++++++++-- > 2 files changed, 38 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..bfb236b 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,40 @@ > 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; > + > + if (!ip) > + return 0; > > atomic_inc(&ip->i_sbd->sd_ops_super); > > - if (ip && sync) > + if (current->flags & PF_MEMALLOC || get_transaction || > + !gfs_glock_is_locked_by_me(ip->i_gl) || !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_flush; > + > + 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_flush: > + if (sync) > gfs_log_flush_glock(ip->i_gl, 0); > > - return 0; > + return ret; > } > > /**