Re: OOPS: 2.6.16-rc6 cpufreq_conservative

[Linus Torvalds <torvalds@osdl.org>]

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/