Re: [PATCH 0/7] repair: Phase 6 performance improvements

From: "Darrick J. Wong" <djwong@kernel.org>
To: Gao Xiang <hsiangkao@redhat.com>
Cc: Dave Chinner <david@fromorbit.com>, linux-xfs@vger.kernel.org
Subject: Re: [PATCH 0/7] repair: Phase 6 performance improvements
Date: Tue, 23 Mar 2021 19:08:10 -0700	[thread overview]
Message-ID: <20210324020810.GP22100@magnolia> (raw)
In-Reply-To: <20210324012655.GA2245176@xiangao.remote.csb>

On Wed, Mar 24, 2021 at 09:26:55AM +0800, Gao Xiang wrote:
> Hi Dave and Darrick,
> 
> On Sat, Mar 20, 2021 at 10:09:31AM +0800, Gao Xiang wrote:
> > On Fri, Mar 19, 2021 at 11:22:21AM -0700, Darrick J. Wong wrote:
> > > On Fri, Mar 19, 2021 at 09:38:45AM +0800, Gao Xiang wrote:
> > > > On Fri, Mar 19, 2021 at 12:33:48PM +1100, Dave Chinner wrote:
> > > > > Hi folks,
> > > > > 
> > > > > This is largely a repost of my current code so that Xiang can take
> > > > > over and finish it off. It applies against 5.11.0 and the
> > > > > performance numbers are still valid. I can't remember how much of
> > > > > the review comments I addressed from the first time I posted it, so
> > > > > the changelog is poor....
> > > > 
> > > > Yeah, I will catch what's missing (now looking the previous review),
> > > > and follow up then...
> > > 
> > > :)
> > > 
> > > While you're revising the patches, you might as well convert:
> > > 
> > > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > 
> > > into:
> > > 
> > > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > because Exchange is so awful for inline replies that I don't use that
> > > email address anymore.
> > 
> > Yeah, I'm just starting sorting out all previous opinions
> > and patches diff. Will update in the next version.
> > 
> 
> Sorry for bothering... After reading the previous discussion for a while,
> I'm fine with the trivial cleanups. Yet, it seems that there are mainly 2
> remaining open discussions unsolved yet...
> 
> 1 is magic number 1000,
> https://lore.kernel.org/r/20201029172045.GP1061252@magnolia
> 
> while I also don't have better ideas of this (and have no idea why queue
> depth 1000 is optimal compared with other configurations), so it'd be better
> to get your thoughts about this in advance (e.g. just leave it as-is, or...
> plus, I don't have such test setting with such many cpus)
> 
> 2 is the hash size modificiation,
> https://lore.kernel.org/r/20201029162922.GM1061252@magnolia/
> 
> it seems previously hash entires are limited to 64k, and this patch relaxes
> such limitation, but for huge directories I'm not sure the hash table
> utilization but from the previous commit message it seems the extra memory
> usage can be ignored.
> 
> Anyway, I'm fine with just leave them as-is if agreed on these.

FWIW I didn't have any specific objections to either magic number, I
simply wanted to know where they came from and why. :)

--D

> Thanks,
> Gao Xiang
> 
> > Thanks,
> > Gao Xiang
> > 
> > > 
> > > --D
> > > 
> > > > Thanks,
> > > > Gao Xiang
> > > > 
> > > 
> 