| Message ID | 167400103096.1915094.8399897640768588035.stgit@magnolia (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fstests: filesystem population fixes | expand |
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. > } > > # For XFS, force on all the quota options if quota is enabled >
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. --D > > } > > > > # For XFS, force on all the quota options if quota is enabled > > >
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? Thanks, Zorro > > --D > > > > } > > > > > > # For XFS, force on all the quota options if quota is enabled > > > > > >
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 > > > > > > > > > >
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 } # For XFS, force on all the quota options if quota is enabled