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

[Peter Xu <peterx@redhat.com>]

On Wed, Mar 17, 2021 at 11:08:31AM -0400, John Kacur wrote:
> 
> 
> On Wed, 17 Mar 2021, Peter Xu wrote:
> 
> > Hi, Daniel,
> > 
> > On Wed, Mar 17, 2021 at 08:49:03AM +0100, Daniel Wagner wrote:
> > > On Tue, Mar 16, 2021 at 04:07:05PM -0400, Peter Xu wrote:
> > > > 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