On Wed, 17 Mar 2021, Peter Xu wrote:

On Wed, Mar 17, 2021 at 11:08:31AM -0400, John Kacur wrote:

quoted

On Wed, 17 Mar 2021, Peter Xu wrote:

quoted

Hi, Daniel,

On Wed, Mar 17, 2021 at 08:49:03AM +0100, Daniel Wagner wrote:

quoted

On Tue, Mar 16, 2021 at 04:07:05PM -0400, Peter Xu wrote:

quoted

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

IIRC, the current behavior allows the process to be placed into a cgroup
with a subset of CPUs and you just can do 'cyclictest -a -t'. Process
should not ignore external configuration. That's my whole point here.

In that case again I think a sane solution is not to check the cpu list in
every single tool we use, because even if we do that for all tools in rt-teets
repo, we can't guarantee to have this check for the rest tools to not ignore
this restriction.

A simple example is: what if the user specified "taskset -c $CPU cyclictest -a
$CPU -t 1 ..." where $CPU is not in the allowed list of current bash?  As long
as the taskset would work the so-called "environment" will be changed before
even loading cyclictest.

If you see that's the point I said we should fail at the same check point of
sched_setaffinity() rather than checking it explicitly in the tool, because
if we want a real-world restriction that's the only place I think it's possible..

But I'm not a cgroup/container guy, please correct me if I understood.

-- 
Peter Xu

When cyclictest and friends were originally written, we had this view 
point that we "owned" the whole machine, and didn't have any restrictions 
on where to schedule. As machines grew in size, and we added numa 
awareness, and cgroups became more prominent we added this code that tried 
to schedule according to the ill-defined environment that we found 
ourselves in.

As Peter points out we may have restricted ourselves more than is 
necessary, and can rely a bit more on the operating system to restrict us. 
On the otherhand using taskset is an easy workaround if the current code 
is to restrictive.

Because we can use taskset and things are working well otherwise I don't 
see this as super urgent, but I am willing to revisit this code and make 
it less restrictive if that makes sense.

I also am not a cgroup / container person, and would like to play around 
with this a bit more before we make some decisions on which direction to 
go in.

Does that make sense to everyone?

Sure thing on my side.  No bug reported so far this time, so I'll wait at least
until then :) I just don't know why it's not hit just like oslat since I don't
see a difference.  When I fixed the oslat thing, I thought cyclictest didn't
have such issue for some reason so I didn't consider to touch it at all.  But
when yesterday I rerun some tests I see this issue on rhel8, hence this patch.

Thanks,

-- 
Peter Xu

Thanks Peter - I appreciate it!

John