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@xxxxxxxxxx>
> > ---
> >
> > 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@xxxxxxxxxx>
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
> >
>
