On Thu, Nov 03, 2022 at 04:02:12PM +0100, Borislav Petkov wrote: > On Thu, Nov 03, 2022 at 01:59:45PM +0100, Andrew Jones wrote: > > The patch I'm proposing ensures cpumask_next()'s range, which is actually > > [-1, nr_cpus_ids - 1), > > Lemme make sure I understand it correctly: on the upper boundary, if you > supply for n the value nr_cpu_ids - 2, then it will return potentially > the last bit if the mask is set, i.e., the one at position (nr_cpu_ids - 1). > > If you supply nr_cpus_ids - 1, then it'll return nr_cpu_ids to signal no > further bits set. > > Yes, no? Yes > > > I'll send a v4 with another stab at the commit message. > > Yes, and it is still an unreadable mess: "A kernel compiled with commit > ... but not its revert... " Nope. > > First make sure cpumask_next()'s valid accepted range has been settled > upon, has been explicitly documented in a comment above it and then I'll > take a patch that fixes whatever is there to fix. That's fair, but I'll leave that to Yury. > > Callers should not have to filter values before passing them in - the > function either returns an error or returns the next bit in the mask. That's reasonable, but cpumask folk probably need to discuss it because not all cpumask functions have a return value where an error may be placed. > > This thing: > > if (*pos == nr_cpu_ids) > > but then to pass in pos - 1: > > *pos = cpumask_next(*pos - 1 > > looks to me like the interface needs more cooking. Indeed, but that's less of an issue with cpumask_next() than with the way cpuinfo implements its start and next seq ops (next unconditionally increments *pos and then calls start and start must use *pos - 1 since the first time its called it needs to use -1). Thanks, drew
