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

[John Kacur <jkacur@redhat.com>]

On Wed, 17 Mar 2021, Peter Xu wrote:

> 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
> 
> 

Thanks Peter - I appreciate it!

John