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

Gao Xiang
2021-03-24T01:28:13Z

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.

Thanks,
Gao Xiang

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