From mboxrd@z Thu Jan 1 00:00:00 1970 From: Abhijith Das Date: Fri, 10 Aug 2018 10:26:50 -0400 (EDT) Subject: [Cluster-devel] [GFS2 PATCH] GFS2: Simplify iterative add loop in foreach_descriptor In-Reply-To: <4848596d-dbcd-6208-8349-4df7eb7a042b@redhat.com> References: <183182471.770089.1533754323408.JavaMail.zimbra@redhat.com> <2e040139-cc86-3256-884e-8e52e48e6df7@redhat.com> <4848596d-dbcd-6208-8349-4df7eb7a042b@redhat.com> Message-ID: <1568273559.56549529.1533911210200.JavaMail.zimbra@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit ----- Original Message ----- > From: "Steven Whitehouse" > To: "Andreas Gruenbacher" > Cc: "Bob Peterson" , "cluster-devel" , "Abhijith Das" > > Sent: Friday, August 10, 2018 7:21:28 AM > Subject: Re: [Cluster-devel] [GFS2 PATCH] GFS2: Simplify iterative add loop in foreach_descriptor > > Hi, > > > On 10/08/18 13:13, Andreas Gruenbacher wrote: > > On 9 August 2018 at 11:35, Steven Whitehouse wrote: > >> Hi, > >> > >> > >> > >> On 08/08/18 19:52, Bob Peterson wrote: > >>> Hi, > >>> > >>> Before this patch, function foreach_descriptor repeatedly called > >>> function gfs2_replay_incr_blk which just incremented the value while > >>> decrementing another, and checked for wrap. This is a waste of time. > >>> This patch just adds the value and adjusts it if a wrap occurred. > >>> > >>> Signed-off-by: Bob Peterson > >>> --- > >>> fs/gfs2/recovery.c | 5 +++-- > >>> 1 file changed, 3 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/fs/gfs2/recovery.c b/fs/gfs2/recovery.c > >>> index 0f501f938d1c..6c6b19263b82 100644 > >>> --- a/fs/gfs2/recovery.c > >>> +++ b/fs/gfs2/recovery.c > >>> @@ -354,8 +354,9 @@ static int foreach_descriptor(struct gfs2_jdesc *jd, > >>> unsigned int start, > >>> return error; > >>> } > >>> - while (length--) > >>> - gfs2_replay_incr_blk(jd, &start); > >>> + start += length; > >>> + if (start >= jd->jd_blocks) > >>> + start -= jd->jd_blocks; > >>> brelse(bh); > >>> } > >>> > >> Now you've hidden the increment of the replay block. Please don't open > >> code > >> this, but just add an argument to gfs2_replay_incr_blk() such that you can > >> tell it how many blocks to increment, rather than just assuming a single > >> block as it does at the moment. Otherwise this can easily get missed when > >> someone looks at the code in future, and expects gfs2_replay_incr_blk to > >> be > >> the only thing that changes the position during recovery, > > If we really want to encapsulate "add modulo jd->jd_blocks", it's also > > open-coded in find_good_lh and jhead_scan. > > > > Andreas > > I wonder if those will go away with Abhi's patch set in due course? > > Steve. > > Yeah... find_good_lh and jhead_scan will go away with my patch set. --Abhi