Re: [PATCH] check: disable HAVE_PRIVATENS by default

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

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
>