From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Whitehouse Date: Thu, 09 Aug 2012 10:04:16 +0100 Subject: [Cluster-devel] [GFS2 PATCH] GFS2: Reduce number of log flushes In-Reply-To: <1354979953.4033641.1344456619278.JavaMail.root@redhat.com> References: <1354979953.4033641.1344456619278.JavaMail.root@redhat.com> Message-ID: <1344503056.2710.3.camel@menhir> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Hi, On Wed, 2012-08-08 at 16:10 -0400, Bob Peterson wrote: > Hi, > > This patch reduces the number of log flushes. > > Regards, > > Bob Peterson > Red Hat File Systems > > Signed-off-by: Bob Peterson > --- > diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c > index 4bdcf37..7d795e7 100644 > --- a/fs/gfs2/glops.c > +++ b/fs/gfs2/glops.c > @@ -118,7 +118,13 @@ void gfs2_ail_flush(struct gfs2_glock *gl, bool fsync) > return; > __gfs2_ail_flush(gl, fsync); > gfs2_trans_end(sdp); > - gfs2_log_flush(sdp, NULL); > + /* if fsync is true, we were called from gfs2_fsync, which means > + we just called sync_inode_metadata, which calls sync_inode, > + which calls writeback_single_inode, which calls write_inode, > + which calls write_inode which calls gfs2_log_flush. So the call > + is redundant. */ > + if (!fsync) > + gfs2_log_flush(sdp, NULL); > } What ensures that the revokes are on disk? > > /** > diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c > index 3cbac68..5904ce0 100644 > --- a/fs/gfs2/super.c > +++ b/fs/gfs2/super.c > @@ -1542,7 +1542,11 @@ static void gfs2_evict_inode(struct inode *inode) > goto out_unlock; > > out_truncate: > - gfs2_log_flush(sdp, ip->i_gl); > + /* write_inode_now calls writeback_single_inode, which calls > + gfs2_write_inode. gfs2_write_inode calls gfs2_log_flush if > + (wbc->sync_mode == WB_SYNC_ALL), which it is since we pass 1. > + therefore, this call to gfs2_log_flush is redundant: > + gfs2_log_flush(sdp, ip->i_gl); */ > write_inode_now(inode, 1); > gfs2_ail_flush(ip->i_gl, 0); > > This one looks like it is probably correct, however there is no point in adding a comment to refer to code that has just been removed. Just remove that call and put the explanation in the patch description, or maybe include just a short comment so say that there is a log flush implicit in the write_inode_now() call, Steve.