On Mon, Mar 06, 2023 at 09:29:10AM -0800, Linus Torvalds wrote: > On Mon, Mar 6, 2023 at 8:07 AM Vernon Yang <vernon...@gmail.com> wrote: > > > > After commit 596ff4a09b89 ("cpumask: re-introduce constant-sized cpumask > > optimizations"), the cpumask size is divided into three different case, > > so fix comment of cpumask_xxx correctly. > > No no. > > Those three cases are meant to be entirely internal optimizations. > They are literally just "preferred sizes". > > The correct thing to do is always that > > * Returns >= nr_cpu_ids if no cpus set. > > because nr_cpu_ids is always the *smallest* of the access sizes. > > That's exactly why it's a ">=". The CPU mask stuff has always > historically potentially used a different size than the actual > nr_cpu_ids, in that it could do word-sized scans even when the machine > might only have a smaller set of CPUs. > > So the whole "small" vs "large" should be seen entirely internal to > cpumask.h. We should not expose it outside (sadly, that already > happened with "nr_cpumask_size", which also was that kind of thing.
I also just see nr_cpumask_size exposed to outside, so... Sorry. > > So no, this patch is wrong. If anything, the comments should be strengthened. > > Of course, right now Guenter seems to be reporting a problem with that > optimization, so unless I figure out what is going on I'll just need > to revert it anyway. Yes, cause is the cpumask_next() calls find_next_bit(..., size, ...), and find_next_bit(..., size, ...) if no bits are set, returns @size. @size was a nr_cpumask_bits variable before, now it is small_cpumask_bits, and when NR_CPUS < = BITS_PER_LONG, small_cpumask_bits is a macro, which is replaced with NR_CPUS at compile, so only the NR_CPUS is returned when it no further cpus set. But before nr_cpumask_bits variable, it was read while running, and it was mutable. The random.c try_to_generate_entropy() to get first cpu by `if (cpu == nr_cpumask_bits)`, but cpumask_next() alway return NR_CPUS, nr_cpumask_bits is nr_cpu_ids, so pass NR_CPUS to add_timer_on(), > > Linus > > Linus