Re: [PATCH 4/4] populate: improve runtime of __populate_fill_fs

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, Jan 19, 2023 at 01:04:06PM +0800, Zorro Lang wrote:
> On Wed, Jan 18, 2023 at 11:22:15AM -0800, Darrick J. Wong wrote:
> > On Wed, Jan 18, 2023 at 11:54:03PM +0800, Zorro Lang wrote:
> > > On Tue, Jan 17, 2023 at 04:44:33PM -0800, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <djwong@kernel.org>
> > > > 
> > > > Run the copy loop in parallel to reduce runtime.  If filling the
> > > > populated fs is selected (which it isn't by default in xfs/349), this
> > > > reduces the runtime from ~18s to ~15s, since it's only making enough
> > > > copies to reduce the free space by 5%.
> > > > 
> > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > > ---
> > > >  common/populate |    3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > 
> > > > 
> > > > diff --git a/common/populate b/common/populate
> > > > index f34551d272..1c3c28463f 100644
> > > > --- a/common/populate
> > > > +++ b/common/populate
> > > > @@ -151,8 +151,9 @@ __populate_fill_fs() {
> > > >  	echo "FILL FS"
> > > >  	echo "src_sz $SRC_SZ fs_sz $FS_SZ nr $NR"
> > > >  	seq 2 "${NR}" | while read nr; do
> > > > -		cp -pRdu "${dir}/test/1" "${dir}/test/${nr}"
> > > > +		cp -pRdu "${dir}/test/1" "${dir}/test/${nr}" &
> > > >  	done
> > > > +	wait
> > > 
> > > I'm thinking about what'll happen if we do "Ctrl+c" on a running test which
> > > is waiting for these cp operations.
> > 
> > Hmm.  In the context of fstests running on a system with systemd, we run
> > each test within a systemd scope and kill the scope when the test script
> > exits.  That will tear down unclaimed background processes, but it's not
> > a hard and fast guarantee that everyone has systemd.
> > 
> > As for *general* bashisms, I guess the only solution is:
> > 
> > trap 'pkill -P $$' INT TERM QUIT EXIT
> > 
> > To kill all the children of the test script.  Maybe we want that?  But I
> > hate wrapping my brain around bash child process management, so yuck.
> > 
> > I'll drop the parallel populate work, it's creating a lot of problems
> > that I don't have time to solve while delivering only modest gains.
> 
> Yeah, that makes things become complex. So I think if above change can bring
> in big performance improvement, we can do that (or use another way to do that,
> e.g. an independent program which main process can deal with its children).
> If the improvement is not obvious, I'd like not to bring in too many
> multi-bash-processes in common helper. What do you think?

It's easier to drop the multi subprocess complexity, so I'll do that. :)

Most of the speedup was from algorithmic improvement, not throwing more
CPUs at the problem.

--D

> Thanks,
> Zorro
> 
> > 
> > --D
> > 
> > > >  }
> > > >  
> > > >  # For XFS, force on all the quota options if quota is enabled
> > > > 
> > > 
> > 
>