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
