Messages in this thread | | | Date | Tue, 7 Mar 2023 02:13:25 +0800 | From | Vernon Yang <> | Subject | Re: [PATCH 5/5] cpumask: fix comment of cpumask_xxx |
| |
On Mon, Mar 06, 2023 at 09:29:10AM -0800, Linus Torvalds wrote: > On Mon, Mar 6, 2023 at 8:07 AM Vernon Yang <vernon2gm@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
| |