Re: [PATCH] rt-tests: Drop use_current_cpuset() check

From: Peter Xu <peterx@redhat.com>
To: Daniel Wagner <wagi@monom.org>
Cc: linux-rt-users@vger.kernel.org, John Kacur <jkacur@redhat.com>,
	Clark Williams <williams@redhat.com>,
	Daniel Wagner <dwagner@suse.de>
Subject: Re: [PATCH] rt-tests: Drop use_current_cpuset() check
Date: Tue, 16 Mar 2021 16:07:05 -0400	[thread overview]
Message-ID: <20210316200705.GC395976@xz-x1> (raw)
In-Reply-To: <20210316081826.bthxqja6shbcp36j@beryllium.lan>

On Tue, Mar 16, 2021 at 09:18:26AM +0100, Daniel Wagner wrote:
> Hi Peter,

Daniel,

> 
> On Mon, Mar 15, 2021 at 03:37:07PM -0400, Peter Xu wrote:
> > CPU list should allow to be any list rather than only the cores that are
> > allowed to schedule for the current process.
> 
> See commit aaa57168dfd3 ("rt-tests: cyclictest: Only run on runtime
> affinity and user supplied affinity")

(Thanks for the commit ID)

> 
> > Before this patch, cyclictest will fail with below condition:
> > 
> > $ taskset -pc $$
> > pid 2316's current affinity list: 0,2,4,6,8
> > $ sudo cyclictest -m -N -p 1 -a 1,3,5,7 -t 4
> > WARN: Couldn't setaffinity in main thread: Invalid argument
> > 
> > After this patch, it'll be allowed to run.
> 
> As John writes in the commit above message I think cyclictest should
> honor to the runtime settings. The warning above could be extended and
> telling you what's the problem is.
> 
> Your commit message should contain the argument why it's better not to
> check the environment settings. So the question is what is the least
> surprise for the user?

I think what I'm missing is why we had such a restriction.  Quotting from the
commit ID:

    Currently if the user passes the affinity to cyclictest, threads are run
    there even if they should be excluded according to the affinity of the
    runtime environment.

So I'm not sure I understand the word "runtime environment".

If it's defined as "the set of cores that this process is allowed to run", I
don't understand why it's not allowed to schedule things outside of this set of
cores, especially if sched_setaffinity() syscall would succeed.  IOW, I'm
afraid we got the idea slightly mixed up on "cores allowed to schedule this
process" with "cores allowed to be set as schedule target by this process", and
IMHO we should simply rely on sched_setaffinity() syscall to decide whether the
affinity setting is legal or not as a single checkpoint.

John and I had some discussion offlist about this last time on oslat, it should
be the same thing here I think. But I'd like John to help confirm too since I
could be missing something.

Thanks,

-- 
Peter Xu