* [PATCH] check: disable HAVE_PRIVATENS by default
@ 2025-03-03 20:50 Zorro Lang
2025-03-04 20:04 ` Darrick J. Wong
0 siblings, 1 reply; 5+ messages in thread
From: Zorro Lang @ 2025-03-03 20:50 UTC (permalink / raw)
To: fstests; +Cc: Darrick J . Wong
Currently we have 3 ways to run a test case in _run_seq():
if [ -n "${HAVE_PRIVATENS}" ]; then
./tools/run_privatens "./$seq"
...
elif [ -n "${HAVE_SYSTEMD_SCOPES}" ]; then
systemd-run --quiet --unit "${unit}" --scope \
./tools/run_setsid "./$seq" &
...
else
./tools/run_setsid "./$seq" &
...
fi
The "privatens" way brings in some regressions. We need more time
to develop and test this way, it's not time let it to be the
first default choice, so isolate the HAVE_PRIVATENS initialization
by a TRY_PRIVATENS parameter, and disable it by default.
Set TRY_PRIVATENS=yes to give "privatens" a try, otherwise run in
old ways. This patch can be removed after "privatens" way is stable.
Signed-off-by: Zorro Lang <zlang@kernel.org>
---
Hi,
This patch aims to be talked. Refer to above commit log. This patch
is a workaround for 2 targets:
1) Avoid the regressions of lastest xfstests release.
2) Give us more time to improve the "privatens" method.
And compare with revert that commit directly, this patch trys to
give "privatens" method a chance to be enabled and tested (by
export TRY_PRIVATENS=yes).
Thanks,
Zorro
check | 22 +++++++---------------
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/check b/check
index ea92b0d62..33eb3e085 100755
--- a/check
+++ b/check
@@ -674,10 +674,13 @@ _stash_test_status() {
esac
}
-# Can we run in a private pid/mount namespace?
-HAVE_PRIVATENS=
-./tools/run_privatens bash -c "exit 77"
-test $? -eq 77 && HAVE_PRIVATENS=yes
+# Don't try "privatens" by default, it's experimental for now.
+if [ "$TRY_PRIVATENS" = "yes" ];then
+ # Can we run in a private pid/mount namespace?
+ HAVE_PRIVATENS=
+ ./tools/run_privatens bash -c "exit 77"
+ test $? -eq 77 && HAVE_PRIVATENS=yes
+fi
# Can we run systemd scopes?
HAVE_SYSTEMD_SCOPES=
@@ -692,15 +695,6 @@ function _adjust_oom_score() {
}
_adjust_oom_score -500
-warn_deprecated_sessionid()
-{
- if [ -z "$WARNED_DEPRECATED_SESSIONID" ]; then
- echo "WARNING: Running fstests without private pid/mount namespace"
- echo "support is deprecated and will be removed in February 2026."
- WARNED_DEPRECATED_SESSIONID=1
- fi
-}
-
# ...and make the tests themselves somewhat more attractive to it, so that if
# the system runs out of memory it'll be the test that gets killed and not the
# test framework. The test is run in a separate process without any of our
@@ -900,8 +894,6 @@ function run_section()
seqres="$check"
_check_test_fs
- test -n "$HAVE_PRIVATENS" || warn_deprecated_sessionid
-
loop_status=() # track rerun-on-failure state
local tc_status ix
local -a _list=( $list )
--
2.47.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] check: disable HAVE_PRIVATENS by default 2025-03-03 20:50 [PATCH] check: disable HAVE_PRIVATENS by default Zorro Lang @ 2025-03-04 20:04 ` Darrick J. Wong 2025-03-04 20:43 ` Zorro Lang 0 siblings, 1 reply; 5+ messages in thread From: Darrick J. Wong @ 2025-03-04 20:04 UTC (permalink / raw) To: Zorro Lang; +Cc: fstests On Tue, Mar 04, 2025 at 04:50:46AM +0800, Zorro Lang wrote: > Currently we have 3 ways to run a test case in _run_seq(): > > if [ -n "${HAVE_PRIVATENS}" ]; then > ./tools/run_privatens "./$seq" > ... > elif [ -n "${HAVE_SYSTEMD_SCOPES}" ]; then > systemd-run --quiet --unit "${unit}" --scope \ > ./tools/run_setsid "./$seq" & > ... > else > ./tools/run_setsid "./$seq" & > ... > fi > > The "privatens" way brings in some regressions. We need more time > to develop and test this way, it's not time let it to be the > first default choice, so isolate the HAVE_PRIVATENS initialization > by a TRY_PRIVATENS parameter, and disable it by default. > > Set TRY_PRIVATENS=yes to give "privatens" a try, otherwise run in > old ways. This patch can be removed after "privatens" way is stable. I think it's ok to turn off privatens for now, seeing as it's clearly caused some regressions that I don't know how to fix. The ones I know about are: 1. The btrfs/14[01] use of BASHPID (not fixed) 2. The weird flock thing in generic/504 (fixed) But then there's a bunch more complaints about "everything" being broken. If things are still broken for you, please send detailed dumps and let Zorro and I figure that out. Note that run_privatens is a lot cleaner than run_setsid -- as I've stated elsewhere, using a new unix process session id to run a test is very messy -- there's no longer a controlling tty, so SIGSTOP doesn't work and SIGINT is masked by default. As I suggested elsewhere today, maybe the solution is to have the tests that *require* the global pid namespace (anything calling the btrfs*read*mirror* helpers) turn themselves off with a "_require_non_privatens"; and then ./check can select setsid for any test containing that phrase. Thoughts on this part? -- I'm ok with taking the quickest route to something that makes everyone happy so that we can push to master again, and merge other peoples' changes because I bet those have been backing up for a while. It's clearly time to talk about reorganizing who's responsible for reviews and for pushing git tree changes, and to take pressure off Zorro. Anand has been signalling interest in taking on some of the btrfs workload, and I think that should happen in some form because Zorro and I are clearly outmatched. > Signed-off-by: Zorro Lang <zlang@kernel.org> > --- > > Hi, > > This patch aims to be talked. Refer to above commit log. This patch > is a workaround for 2 targets: > 1) Avoid the regressions of lastest xfstests release. > 2) Give us more time to improve the "privatens" method. > > And compare with revert that commit directly, this patch trys to > give "privatens" method a chance to be enabled and tested (by > export TRY_PRIVATENS=yes). Everyone else: do things work better if you manually patch out the run_privatens selection code so that run_setsid is always used? The setsid code itself is also new; before that fstests just used killall. > Thanks, > Zorro > > check | 22 +++++++--------------- > 1 file changed, 7 insertions(+), 15 deletions(-) > > diff --git a/check b/check > index ea92b0d62..33eb3e085 100755 > --- a/check > +++ b/check > @@ -674,10 +674,13 @@ _stash_test_status() { > esac > } > > -# Can we run in a private pid/mount namespace? > -HAVE_PRIVATENS= > -./tools/run_privatens bash -c "exit 77" > -test $? -eq 77 && HAVE_PRIVATENS=yes > +# Don't try "privatens" by default, it's experimental for now. > +if [ "$TRY_PRIVATENS" = "yes" ];then > + # Can we run in a private pid/mount namespace? > + HAVE_PRIVATENS= > + ./tools/run_privatens bash -c "exit 77" > + test $? -eq 77 && HAVE_PRIVATENS=yes > +fi > # Can we run systemd scopes? > HAVE_SYSTEMD_SCOPES= > @@ -692,15 +695,6 @@ function _adjust_oom_score() { > } > _adjust_oom_score -500 > > -warn_deprecated_sessionid() > -{ > - if [ -z "$WARNED_DEPRECATED_SESSIONID" ]; then > - echo "WARNING: Running fstests without private pid/mount namespace" > - echo "support is deprecated and will be removed in February 2026." > - WARNED_DEPRECATED_SESSIONID=1 > - fi > -} > - > # ...and make the tests themselves somewhat more attractive to it, so that if > # the system runs out of memory it'll be the test that gets killed and not the > # test framework. The test is run in a separate process without any of our > @@ -900,8 +894,6 @@ function run_section() > seqres="$check" > _check_test_fs > > - test -n "$HAVE_PRIVATENS" || warn_deprecated_sessionid > - Removal of the deprecation should be a separate patch, for which I will pre-offer a Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> --D > loop_status=() # track rerun-on-failure state > local tc_status ix > local -a _list=( $list ) > -- > 2.47.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] check: disable HAVE_PRIVATENS by default 2025-03-04 20:04 ` Darrick J. Wong @ 2025-03-04 20:43 ` Zorro Lang 2025-03-04 20:59 ` Darrick J. Wong 0 siblings, 1 reply; 5+ messages in thread From: Zorro Lang @ 2025-03-04 20:43 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Zorro Lang, fstests On Tue, Mar 04, 2025 at 12:04:58PM -0800, Darrick J. Wong wrote: > On Tue, Mar 04, 2025 at 04:50:46AM +0800, Zorro Lang wrote: > > Currently we have 3 ways to run a test case in _run_seq(): > > > > if [ -n "${HAVE_PRIVATENS}" ]; then > > ./tools/run_privatens "./$seq" > > ... > > elif [ -n "${HAVE_SYSTEMD_SCOPES}" ]; then > > systemd-run --quiet --unit "${unit}" --scope \ > > ./tools/run_setsid "./$seq" & > > ... > > else > > ./tools/run_setsid "./$seq" & > > ... > > fi > > > > The "privatens" way brings in some regressions. We need more time > > to develop and test this way, it's not time let it to be the > > first default choice, so isolate the HAVE_PRIVATENS initialization > > by a TRY_PRIVATENS parameter, and disable it by default. > > > > Set TRY_PRIVATENS=yes to give "privatens" a try, otherwise run in > > old ways. This patch can be removed after "privatens" way is stable. > > I think it's ok to turn off privatens for now, seeing as it's clearly > caused some regressions that I don't know how to fix. The ones I know > about are: > > 1. The btrfs/14[01] use of BASHPID (not fixed) > 2. The weird flock thing in generic/504 (fixed) And a bug report when /tmp isn't a mount point. > > But then there's a bunch more complaints about "everything" being > broken. If things are still broken for you, please send detailed dumps > and let Zorro and I figure that out. > > Note that run_privatens is a lot cleaner than run_setsid -- as I've > stated elsewhere, using a new unix process session id to run a test is > very messy -- there's no longer a controlling tty, so SIGSTOP doesn't > work and SIGINT is masked by default. > > As I suggested elsewhere today, maybe the solution is to have the tests > that *require* the global pid namespace (anything calling the > btrfs*read*mirror* helpers) turn themselves off with a > "_require_non_privatens"; and then ./check can select setsid for any > test containing that phrase. > > Thoughts on this part? More likes a trick, anyway I think we can talk about the way to fix btrfs BASHPID related problems later. And I still don't know if it's the same root cause with Dave Sterba hit. > > -- > > I'm ok with taking the quickest route to something that makes everyone > happy so that we can push to master again, and merge other peoples' > changes because I bet those have been backing up for a while. > > It's clearly time to talk about reorganizing who's responsible for > reviews and for pushing git tree changes, and to take pressure off > Zorro. Anand has been signalling interest in taking on some of the > btrfs workload, and I think that should happen in some form because > Zorro and I are clearly outmatched. Acutally I've tried to test most of major fs which fstests supports, before a release. Btrfs testing is supported by my test scripts now. Currently only default btrfs be tested. If Anand would like to test more for btrfs before each release, I think that helps btrfs more. > > > Signed-off-by: Zorro Lang <zlang@kernel.org> > > --- > > > > Hi, > > > > This patch aims to be talked. Refer to above commit log. This patch > > is a workaround for 2 targets: > > 1) Avoid the regressions of lastest xfstests release. > > 2) Give us more time to improve the "privatens" method. > > > > And compare with revert that commit directly, this patch trys to > > give "privatens" method a chance to be enabled and tested (by > > export TRY_PRIVATENS=yes). > > Everyone else: do things work better if you manually patch out the > run_privatens selection code so that run_setsid is always used? The > setsid code itself is also new; before that fstests just used killall. If no one reports bugs to run_setsid, I'll think it's stable (temporarily), then we'll pay more attention to "privatens". > > > Thanks, > > Zorro > > > > check | 22 +++++++--------------- > > 1 file changed, 7 insertions(+), 15 deletions(-) > > > > diff --git a/check b/check > > index ea92b0d62..33eb3e085 100755 > > --- a/check > > +++ b/check > > @@ -674,10 +674,13 @@ _stash_test_status() { > > esac > > } > > > > -# Can we run in a private pid/mount namespace? > > -HAVE_PRIVATENS= > > -./tools/run_privatens bash -c "exit 77" > > -test $? -eq 77 && HAVE_PRIVATENS=yes > > +# Don't try "privatens" by default, it's experimental for now. > > +if [ "$TRY_PRIVATENS" = "yes" ];then > > + # Can we run in a private pid/mount namespace? > > + HAVE_PRIVATENS= > > + ./tools/run_privatens bash -c "exit 77" > > + test $? -eq 77 && HAVE_PRIVATENS=yes > > +fi > > # Can we run systemd scopes? > > HAVE_SYSTEMD_SCOPES= > > @@ -692,15 +695,6 @@ function _adjust_oom_score() { > > } > > _adjust_oom_score -500 > > > > -warn_deprecated_sessionid() > > -{ > > - if [ -z "$WARNED_DEPRECATED_SESSIONID" ]; then > > - echo "WARNING: Running fstests without private pid/mount namespace" > > - echo "support is deprecated and will be removed in February 2026." > > - WARNED_DEPRECATED_SESSIONID=1 > > - fi > > -} > > - > > # ...and make the tests themselves somewhat more attractive to it, so that if > > # the system runs out of memory it'll be the test that gets killed and not the > > # test framework. The test is run in a separate process without any of our > > @@ -900,8 +894,6 @@ function run_section() > > seqres="$check" > > _check_test_fs > > > > - test -n "$HAVE_PRIVATENS" || warn_deprecated_sessionid > > - > > Removal of the deprecation should be a separate patch, for which I will > pre-offer a > Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> Ok, I'll splite this patch and send a v2. Thanks your reviewing! Thanks, Zorro > > --D > > > loop_status=() # track rerun-on-failure state > > local tc_status ix > > local -a _list=( $list ) > > -- > > 2.47.1 > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] check: disable HAVE_PRIVATENS by default 2025-03-04 20:43 ` Zorro Lang @ 2025-03-04 20:59 ` Darrick J. Wong 2025-03-06 8:05 ` Zorro Lang 0 siblings, 1 reply; 5+ messages in thread From: Darrick J. Wong @ 2025-03-04 20:59 UTC (permalink / raw) To: Zorro Lang; +Cc: Zorro Lang, fstests On Wed, Mar 05, 2025 at 04:43:01AM +0800, Zorro Lang wrote: > On Tue, Mar 04, 2025 at 12:04:58PM -0800, Darrick J. Wong wrote: > > On Tue, Mar 04, 2025 at 04:50:46AM +0800, Zorro Lang wrote: > > > Currently we have 3 ways to run a test case in _run_seq(): > > > > > > if [ -n "${HAVE_PRIVATENS}" ]; then > > > ./tools/run_privatens "./$seq" > > > ... > > > elif [ -n "${HAVE_SYSTEMD_SCOPES}" ]; then > > > systemd-run --quiet --unit "${unit}" --scope \ > > > ./tools/run_setsid "./$seq" & > > > ... > > > else > > > ./tools/run_setsid "./$seq" & > > > ... > > > fi > > > > > > The "privatens" way brings in some regressions. We need more time > > > to develop and test this way, it's not time let it to be the > > > first default choice, so isolate the HAVE_PRIVATENS initialization > > > by a TRY_PRIVATENS parameter, and disable it by default. > > > > > > Set TRY_PRIVATENS=yes to give "privatens" a try, otherwise run in > > > old ways. This patch can be removed after "privatens" way is stable. > > > > I think it's ok to turn off privatens for now, seeing as it's clearly > > caused some regressions that I don't know how to fix. The ones I know > > about are: > > > > 1. The btrfs/14[01] use of BASHPID (not fixed) > > 2. The weird flock thing in generic/504 (fixed) > > And a bug report when /tmp isn't a mount point. Oh yeah. Forgot that one. We could fix it this way: diff --git a/check b/check index ea92b0d62a..7196cfbfea 100755 --- a/check +++ b/check @@ -821,6 +821,20 @@ function run_section() echo "SECTION -- $section" fi + # If we're running in a private mount namespace, /tmp is a private + # directory. We /could/ just mkdir it, but we'd rather have people + # set those paths elsewhere. + if [ "$HAVE_PRIVATENS" = yes ] && [[ $TEST_DIR =~ ^\/tmp ]]; then + echo "$TEST_DIR: TEST_DIR must not be in /tmp" + status=1 + exit + fi + if [ "$HAVE_PRIVATENS" = yes ] && [[ $SCRATCH_MNT =~ ^\/tmp ]]; then + echo "$SCRATCH_MNT: SCRATCH_MNT must not be in /tmp" + status=1 + exit + fi + sect_start=`_wallclock` if $RECREATE_TEST_DEV || [ "$OLD_FSTYP" != "$FSTYP" ]; then echo "RECREATING -- $FSTYP on $TEST_DEV" Or we could just have _begin_fstest mkdir them if they don't exist. Unless it'd make more sense to have ./check define TEST_DIR and SCRATCH_MNT itself? Imagine if it instead did this: FSTESTS_RUN_DIR=/run/fstests-$$ mkdir -p $FSTESTS_RUN_DIR mount -t tmpfs none $FSTESTS_RUN_DIR test -n "$TEST_DEV" && mkdir -p $FSTESTS_RUN_DIR/test test -n "$SCRATCH_MNT" && $FSTESTS_RUN_DIR/scratch mount -o remount,ro $FSTESTS_RUN_DIR test -n "$TEST_DEV" && TEST_DIR=$FSTESTS_RUN_DIR/test test -n "$SCRATCH_MNT" && SCRATCH_MNT=$FSTESTS_RUN_DIR/scratch Now the user doesn't have to create the directories themselves, and since the underlying $FSTESTS_RUN_DIR is read only, mount failures will show up immediately as write failures instead of filling up the wrong filesystem. --D > > But then there's a bunch more complaints about "everything" being > > broken. If things are still broken for you, please send detailed dumps > > and let Zorro and I figure that out. > > > > Note that run_privatens is a lot cleaner than run_setsid -- as I've > > stated elsewhere, using a new unix process session id to run a test is > > very messy -- there's no longer a controlling tty, so SIGSTOP doesn't > > work and SIGINT is masked by default. > > > > As I suggested elsewhere today, maybe the solution is to have the tests > > that *require* the global pid namespace (anything calling the > > btrfs*read*mirror* helpers) turn themselves off with a > > "_require_non_privatens"; and then ./check can select setsid for any > > test containing that phrase. > > > > Thoughts on this part? > > More likes a trick, anyway I think we can talk about the way to fix btrfs > BASHPID related problems later. And I still don't know if it's the same > root cause with Dave Sterba hit. > > > > > -- > > > > I'm ok with taking the quickest route to something that makes everyone > > happy so that we can push to master again, and merge other peoples' > > changes because I bet those have been backing up for a while. > > > > It's clearly time to talk about reorganizing who's responsible for > > reviews and for pushing git tree changes, and to take pressure off > > Zorro. Anand has been signalling interest in taking on some of the > > btrfs workload, and I think that should happen in some form because > > Zorro and I are clearly outmatched. > > Acutally I've tried to test most of major fs which fstests supports, > before a release. Btrfs testing is supported by my test scripts now. > Currently only default btrfs be tested. If Anand would like to test > more for btrfs before each release, I think that helps btrfs more. > > > > > > Signed-off-by: Zorro Lang <zlang@kernel.org> > > > --- > > > > > > Hi, > > > > > > This patch aims to be talked. Refer to above commit log. This patch > > > is a workaround for 2 targets: > > > 1) Avoid the regressions of lastest xfstests release. > > > 2) Give us more time to improve the "privatens" method. > > > > > > And compare with revert that commit directly, this patch trys to > > > give "privatens" method a chance to be enabled and tested (by > > > export TRY_PRIVATENS=yes). > > > > Everyone else: do things work better if you manually patch out the > > run_privatens selection code so that run_setsid is always used? The > > setsid code itself is also new; before that fstests just used killall. > > If no one reports bugs to run_setsid, I'll think it's stable (temporarily), > then we'll pay more attention to "privatens". > > > > > > Thanks, > > > Zorro > > > > > > check | 22 +++++++--------------- > > > 1 file changed, 7 insertions(+), 15 deletions(-) > > > > > > diff --git a/check b/check > > > index ea92b0d62..33eb3e085 100755 > > > --- a/check > > > +++ b/check > > > @@ -674,10 +674,13 @@ _stash_test_status() { > > > esac > > > } > > > > > > -# Can we run in a private pid/mount namespace? > > > -HAVE_PRIVATENS= > > > -./tools/run_privatens bash -c "exit 77" > > > -test $? -eq 77 && HAVE_PRIVATENS=yes > > > +# Don't try "privatens" by default, it's experimental for now. > > > +if [ "$TRY_PRIVATENS" = "yes" ];then > > > + # Can we run in a private pid/mount namespace? > > > + HAVE_PRIVATENS= > > > + ./tools/run_privatens bash -c "exit 77" > > > + test $? -eq 77 && HAVE_PRIVATENS=yes > > > +fi > > > # Can we run systemd scopes? > > > HAVE_SYSTEMD_SCOPES= > > > @@ -692,15 +695,6 @@ function _adjust_oom_score() { > > > } > > > _adjust_oom_score -500 > > > > > > -warn_deprecated_sessionid() > > > -{ > > > - if [ -z "$WARNED_DEPRECATED_SESSIONID" ]; then > > > - echo "WARNING: Running fstests without private pid/mount namespace" > > > - echo "support is deprecated and will be removed in February 2026." > > > - WARNED_DEPRECATED_SESSIONID=1 > > > - fi > > > -} > > > - > > > # ...and make the tests themselves somewhat more attractive to it, so that if > > > # the system runs out of memory it'll be the test that gets killed and not the > > > # test framework. The test is run in a separate process without any of our > > > @@ -900,8 +894,6 @@ function run_section() > > > seqres="$check" > > > _check_test_fs > > > > > > - test -n "$HAVE_PRIVATENS" || warn_deprecated_sessionid > > > - > > > > Removal of the deprecation should be a separate patch, for which I will > > pre-offer a > > Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> > > Ok, I'll splite this patch and send a v2. Thanks your reviewing! > > Thanks, > Zorro > > > > > --D > > > > > loop_status=() # track rerun-on-failure state > > > local tc_status ix > > > local -a _list=( $list ) > > > -- > > > 2.47.1 > > > > > > ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] check: disable HAVE_PRIVATENS by default 2025-03-04 20:59 ` Darrick J. Wong @ 2025-03-06 8:05 ` Zorro Lang 0 siblings, 0 replies; 5+ messages in thread From: Zorro Lang @ 2025-03-06 8:05 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Zorro Lang, fstests On Tue, Mar 04, 2025 at 12:59:42PM -0800, Darrick J. Wong wrote: > On Wed, Mar 05, 2025 at 04:43:01AM +0800, Zorro Lang wrote: > > On Tue, Mar 04, 2025 at 12:04:58PM -0800, Darrick J. Wong wrote: > > > On Tue, Mar 04, 2025 at 04:50:46AM +0800, Zorro Lang wrote: > > > > Currently we have 3 ways to run a test case in _run_seq(): > > > > > > > > if [ -n "${HAVE_PRIVATENS}" ]; then > > > > ./tools/run_privatens "./$seq" > > > > ... > > > > elif [ -n "${HAVE_SYSTEMD_SCOPES}" ]; then > > > > systemd-run --quiet --unit "${unit}" --scope \ > > > > ./tools/run_setsid "./$seq" & > > > > ... > > > > else > > > > ./tools/run_setsid "./$seq" & > > > > ... > > > > fi > > > > > > > > The "privatens" way brings in some regressions. We need more time > > > > to develop and test this way, it's not time let it to be the > > > > first default choice, so isolate the HAVE_PRIVATENS initialization > > > > by a TRY_PRIVATENS parameter, and disable it by default. > > > > > > > > Set TRY_PRIVATENS=yes to give "privatens" a try, otherwise run in > > > > old ways. This patch can be removed after "privatens" way is stable. > > > > > > I think it's ok to turn off privatens for now, seeing as it's clearly > > > caused some regressions that I don't know how to fix. The ones I know > > > about are: > > > > > > 1. The btrfs/14[01] use of BASHPID (not fixed) > > > 2. The weird flock thing in generic/504 (fixed) > > > > And a bug report when /tmp isn't a mount point. > > Oh yeah. Forgot that one. We could fix it this way: > > diff --git a/check b/check > index ea92b0d62a..7196cfbfea 100755 > --- a/check > +++ b/check > @@ -821,6 +821,20 @@ function run_section() > echo "SECTION -- $section" > fi > > + # If we're running in a private mount namespace, /tmp is a private > + # directory. We /could/ just mkdir it, but we'd rather have people > + # set those paths elsewhere. > + if [ "$HAVE_PRIVATENS" = yes ] && [[ $TEST_DIR =~ ^\/tmp ]]; then > + echo "$TEST_DIR: TEST_DIR must not be in /tmp" > + status=1 > + exit > + fi > + if [ "$HAVE_PRIVATENS" = yes ] && [[ $SCRATCH_MNT =~ ^\/tmp ]]; then > + echo "$SCRATCH_MNT: SCRATCH_MNT must not be in /tmp" > + status=1 > + exit > + fi > + > sect_start=`_wallclock` > if $RECREATE_TEST_DEV || [ "$OLD_FSTYP" != "$FSTYP" ]; then > echo "RECREATING -- $FSTYP on $TEST_DEV" Oh, I didn't describe clearly, I mean this one bug: https://lore.kernel.org/fstests/20250303064209.602577-1-naohiro.aota@wdc.com/ some systems might not have a tmpfs on /tmp ^^ I'll leave more review points under that thread. > > Or we could just have _begin_fstest mkdir them if they don't exist. > > Unless it'd make more sense to have ./check define TEST_DIR and > SCRATCH_MNT itself? Imagine if it instead did this: > > FSTESTS_RUN_DIR=/run/fstests-$$ > mkdir -p $FSTESTS_RUN_DIR > mount -t tmpfs none $FSTESTS_RUN_DIR Do you hope to have TEST_DIR and SCRATCH_MNT under a tmpfs? > test -n "$TEST_DEV" && mkdir -p $FSTESTS_RUN_DIR/test > test -n "$SCRATCH_MNT" && $FSTESTS_RUN_DIR/scratch > mount -o remount,ro $FSTESTS_RUN_DIR > test -n "$TEST_DEV" && TEST_DIR=$FSTESTS_RUN_DIR/test > test -n "$SCRATCH_MNT" && SCRATCH_MNT=$FSTESTS_RUN_DIR/scratch > > Now the user doesn't have to create the directories themselves, and > since the underlying $FSTESTS_RUN_DIR is read only, mount failures will > show up immediately as write failures instead of filling up the wrong > filesystem. I think we can talk about more details in another thread. Thanks, Zorro > > --D > > > > But then there's a bunch more complaints about "everything" being > > > broken. If things are still broken for you, please send detailed dumps > > > and let Zorro and I figure that out. > > > > > > Note that run_privatens is a lot cleaner than run_setsid -- as I've > > > stated elsewhere, using a new unix process session id to run a test is > > > very messy -- there's no longer a controlling tty, so SIGSTOP doesn't > > > work and SIGINT is masked by default. > > > > > > As I suggested elsewhere today, maybe the solution is to have the tests > > > that *require* the global pid namespace (anything calling the > > > btrfs*read*mirror* helpers) turn themselves off with a > > > "_require_non_privatens"; and then ./check can select setsid for any > > > test containing that phrase. > > > > > > Thoughts on this part? > > > > More likes a trick, anyway I think we can talk about the way to fix btrfs > > BASHPID related problems later. And I still don't know if it's the same > > root cause with Dave Sterba hit. > > > > > > > > -- > > > > > > I'm ok with taking the quickest route to something that makes everyone > > > happy so that we can push to master again, and merge other peoples' > > > changes because I bet those have been backing up for a while. > > > > > > It's clearly time to talk about reorganizing who's responsible for > > > reviews and for pushing git tree changes, and to take pressure off > > > Zorro. Anand has been signalling interest in taking on some of the > > > btrfs workload, and I think that should happen in some form because > > > Zorro and I are clearly outmatched. > > > > Acutally I've tried to test most of major fs which fstests supports, > > before a release. Btrfs testing is supported by my test scripts now. > > Currently only default btrfs be tested. If Anand would like to test > > more for btrfs before each release, I think that helps btrfs more. > > > > > > > > > Signed-off-by: Zorro Lang <zlang@kernel.org> > > > > --- > > > > > > > > Hi, > > > > > > > > This patch aims to be talked. Refer to above commit log. This patch > > > > is a workaround for 2 targets: > > > > 1) Avoid the regressions of lastest xfstests release. > > > > 2) Give us more time to improve the "privatens" method. > > > > > > > > And compare with revert that commit directly, this patch trys to > > > > give "privatens" method a chance to be enabled and tested (by > > > > export TRY_PRIVATENS=yes). > > > > > > Everyone else: do things work better if you manually patch out the > > > run_privatens selection code so that run_setsid is always used? The > > > setsid code itself is also new; before that fstests just used killall. > > > > If no one reports bugs to run_setsid, I'll think it's stable (temporarily), > > then we'll pay more attention to "privatens". > > > > > > > > > Thanks, > > > > Zorro > > > > > > > > check | 22 +++++++--------------- > > > > 1 file changed, 7 insertions(+), 15 deletions(-) > > > > > > > > diff --git a/check b/check > > > > index ea92b0d62..33eb3e085 100755 > > > > --- a/check > > > > +++ b/check > > > > @@ -674,10 +674,13 @@ _stash_test_status() { > > > > esac > > > > } > > > > > > > > -# Can we run in a private pid/mount namespace? > > > > -HAVE_PRIVATENS= > > > > -./tools/run_privatens bash -c "exit 77" > > > > -test $? -eq 77 && HAVE_PRIVATENS=yes > > > > +# Don't try "privatens" by default, it's experimental for now. > > > > +if [ "$TRY_PRIVATENS" = "yes" ];then > > > > + # Can we run in a private pid/mount namespace? > > > > + HAVE_PRIVATENS= > > > > + ./tools/run_privatens bash -c "exit 77" > > > > + test $? -eq 77 && HAVE_PRIVATENS=yes > > > > +fi > > > > # Can we run systemd scopes? > > > > HAVE_SYSTEMD_SCOPES= > > > > @@ -692,15 +695,6 @@ function _adjust_oom_score() { > > > > } > > > > _adjust_oom_score -500 > > > > > > > > -warn_deprecated_sessionid() > > > > -{ > > > > - if [ -z "$WARNED_DEPRECATED_SESSIONID" ]; then > > > > - echo "WARNING: Running fstests without private pid/mount namespace" > > > > - echo "support is deprecated and will be removed in February 2026." > > > > - WARNED_DEPRECATED_SESSIONID=1 > > > > - fi > > > > -} > > > > - > > > > # ...and make the tests themselves somewhat more attractive to it, so that if > > > > # the system runs out of memory it'll be the test that gets killed and not the > > > > # test framework. The test is run in a separate process without any of our > > > > @@ -900,8 +894,6 @@ function run_section() > > > > seqres="$check" > > > > _check_test_fs > > > > > > > > - test -n "$HAVE_PRIVATENS" || warn_deprecated_sessionid > > > > - > > > > > > Removal of the deprecation should be a separate patch, for which I will > > > pre-offer a > > > Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> > > > > Ok, I'll splite this patch and send a v2. Thanks your reviewing! > > > > Thanks, > > Zorro > > > > > > > > --D > > > > > > > loop_status=() # track rerun-on-failure state > > > > local tc_status ix > > > > local -a _list=( $list ) > > > > -- > > > > 2.47.1 > > > > > > > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-03-06 8:06 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-03-03 20:50 [PATCH] check: disable HAVE_PRIVATENS by default Zorro Lang 2025-03-04 20:04 ` Darrick J. Wong 2025-03-04 20:43 ` Zorro Lang 2025-03-04 20:59 ` Darrick J. Wong 2025-03-06 8:05 ` Zorro Lang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox