Messages in this thread | | | Date | Sun, 19 Mar 2006 09:43:26 -0800 (PST) | From | Linus Torvalds <> | Subject | Re: OOPS: 2.6.16-rc6 cpufreq_conservative |
| |
On Sat, 18 Mar 2006, Paul Jackson wrote: > > If find_first_bit and find_next_bit suck so bad, then why just fix > their use in the for_each_cpu_mask(). How about the other uses?
find_first_bit/find_next_bit don't actually suck. They're perfectly efficient for large bitmaps, which is what they were designed for (ie they were written for doing the free-block/inode bitmaps for minixfs originally).
In other words, they are meant for bitmaps of _thousands_ of bits, and for potentially sparse bitmaps.
Now, the CPU masks _can_ be large too, but commonly they are not. In this example, the "bitmap" was exactly two bits: cpu 0 and cpu 1. Using a complex "find_next_bit" for something like that is just overkill.
> And if we fix the cpu loop to the API Linus suggests, we should do the > same with the node loops
Yes. Almost ever "for_each_xyzzy" tends to be more easily written as a macro if we also have a ending condition.
> And for those of us with too many CPUs, how about something like > (totally untested and probably totally bogus): > > #if NR_CPUS <= BITS_IN_LONG > ... as per Linus, shifting a mask right ... > #else > #define for_each_cpu_mask(cpu, mask) > { for (cpu = 0; cpu < NR_CPUS; cpu++) { > if (!(test_bit(cpu, mask)) > continue; > #endif > > #define end_for_each_cpu_mask } }
Yes, except you'd probably be better off with testing whole words at a time, since a large NR_CPUs can often be because somebody wanted to support 256 CPU machines, and then used the same binary on a 8-way setup.
That would most helpfully be done with a double loop (outer one iterating over the bitmask words, inner one over the bits), but then it's hard to make "break" do the right thing inside the loop.
Linus - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |