linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: Ryan Roberts <ryan.roberts@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Shuah Khan <shuah@kernel.org>,
	linux-mm@kvack.org, linux-kselftest@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] selftests/mm: Don't needlessly use sudo to obtain root in run_vmtests.sh
Date: Sat, 10 Feb 2024 12:35:17 +0000	[thread overview]
Message-ID: <ZcdthfAvzLQ9lzvd@finisterre.sirena.org.uk> (raw)
In-Reply-To: <17c0b7a1-6ec2-4504-8287-f0fa111b9748@arm.com>

[-- Attachment #1: Type: text/plain, Size: 1770 bytes --]

On Sat, Feb 10, 2024 at 07:40:16AM +0000, Ryan Roberts wrote:
> On 09/02/2024 20:21, Mark Brown wrote:

> > When opening yama/ptrace_scope we unconditionally use sudo to ensure we
> > are running as root, resulting in failures if running in a minimal root
> > filesystem where sudo is not installed. Since automated test systems will
> > typically just run all of kselftest as root (and many kselftests rely on
> > this for full functionality) add a check to see if we're already root and
> > only invoke sudo if not.

> I don't really see the point of this. run_vmtests.sh needs to be run as root;
> there are lots of operations that depend on it and most tests will fail if not
> root. So I think it would be much cleaner just to remove this instance sudo.

Ah, I was assuming that some of the suite ran usefully as non-root given
that the only point of that sudo was to acquire root.  If the whole
thing needs to be root then we should instead have a check for root at
the top of run_vmtests.sh and just skip the whole thing if we aren't
root, but then I'm unclear why it's invoking sudo in the first place.

> The problem that I was referring to yesterday, about needing sudo was for this case:

> CATEGORY="mlock" run_test sudo -u nobody ./on-fault-limit

> Here, we are using sudo to deprivilege ourselves from root and run
> on-fault-limit as nobody. This is required because the test is checking an
> rlimit that is only enforced for normal users.

> Somebody on list was talking about skipping this test if sudo wasn't present a
> couple of weeks back. Not sure if that happened.

Yes, there's a check:

	if command -v sudo &> /dev/null;
	then
	        CATEGORY="mlock" run_test sudo -u nobody ./on-fault-limit
	else
	        echo "# SKIP ./on-fault-limit"
	fi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2024-02-10 12:35 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-09 20:21 Mark Brown
2024-02-10  7:40 ` Ryan Roberts
2024-02-10 12:35   ` Mark Brown [this message]
2024-02-12  8:32     ` Ryan Roberts
2024-02-12 19:13       ` Mark Brown
2024-02-14  8:53         ` Ryan Roberts

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZcdthfAvzLQ9lzvd@finisterre.sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ryan.roberts@arm.com \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox