On Fri, 19 Feb 2021, Daniel Wagner wrote: > Hi Peter, > > On Thu, Feb 18, 2021 at 02:27:29PM -0500, Peter Xu wrote: > > parse_cpumask() is too strict for oslat, in that use_current_cpuset() will > > filter out all the cores that are not allowed for current process to run. This > > seems to be unnecessary at least for oslat. For example, the bash process that > > runs the oslat program may have a sched affinity of 0-2, however it's still > > legal to have it start a oslat thread running on the cores outside 0-2 as long > > as the follow up sched_setaffinity() will succeed. > > > > numa_parse_cpustring_all() suites exactly for this case, which should already > > have considered sysconf(_SC_NPROCESSORS_ONLN) limit. Use that instead. > > > > Since at it, also remove initialization of cpu_set variable otherwise it's > > leaked in previous parse_cpumask too: numa_parse_cpustring_all() will return a > > newly allocated buffer already. Quotting from manual: > > > > numa_parse_nodestring() parses a character string list of nodes into a bit > > mask. The bit mask is allocated by numa_allocate_nodemask(). > > > > numa_parse_nodestring_all() is similar to numa_parse_nodestring, but can > > parse all possible nodes, not only current nodeset. > > My 2 cents: If parse_cpumask() is to strict fix parse_cpumask() so all parse_cpumask() correctly examines the current environment and creates an intersection between the requested cpus and the ones it has permission to use. Using taskset with the same parameters as passed to the tools gets around that. I have a few comments 1. Make the programs in the suite work the same way where that makes sense, but let's not make a fetish of it, sometimes there are good reasons not to impose the conformity. 2. If the user intentionally wants to go outside of the constraints, we could use a --force option to make sure the user is aware they are doing so. In private chats with Peter though, he pointed out that if you're always using the --force option, then maybe it makes sense for that to be the default, and in addition, should calls to setaffinity() be the test? 3. We need to think a little bit about whether and how cyclictest and friends should run in containers. > tools do the same thing. Note parse_cpumask() contains a couple > of bugs: > > "rt-numa: Use mask size for iterator limit" > "rt-numa: Remove max_cpus argument from parse_cpusmask" Are those actually bug fixes or just a different way of doing things? > > Thanks, > Daniel >
