Re: OOPS: 2.6.16-rc6 cpufreq_conservative

[Willy Tarreau <willy@w.ods.org>]

On Mon, Mar 20, 2006 at 08:18:46AM +0100, Willy Tarreau wrote:
> On Sun, Mar 19, 2006 at 10:26:30PM -0800, Linus Torvalds wrote:
> > 
> > 
> > On Mon, 20 Mar 2006, Willy Tarreau wrote:
> > > 
> > > Now, does removing the macro completely change the output code ?
> > > I think that if something written like this produces the same
> > > code, it would be easier to read :
> > > 
> > > #define for_each_cpu_mask(cpu, mask)			\
> > > 	for ((cpu) = 0; (cpu) < NR_CPUS; (cpu)++) {	\
> > > 		unsigned long __bits = (mask).bits[0] >> (cpu); \
> > > 		if (!__bits)				\
> > > 			break;				\
> > > 		if (!__bits & 1)			\
> > > 			continue;			\
> > > 		else
> > 
> > Absolutely, but now it has a dangling "{" that didn't get closed. So the 
> > above would definitely be more readable, it just doesn't actually work.
> > 
> > Unless you'd do the "end_for_each_cpu" define (to close the statement), 
> > and update the 300+ places that use this. Which might well be worth it.
> > 
> > So the subtle "break from the middle of a statement expression" was just a 
> > rather hacky way to avoid having to change all the users of this macro.
> >
> > 			Linus
> 
> Oh, you're right, now I understand your motivation in doing this.
> Then perhaps using your trick but applying it to the whole for loop
> would make it easier to read ?
> 
> #define for_each_cpu_mask(cpu, mask)			\
> 	for ((cpu) = 0; (cpu) < NR_CPUS; (cpu)++) 	\
> 	    ({	unsigned long __bits = (mask).bits[0] >> (cpu); \
> 		if (!__bits)				\
> 			break;				\
> 		if (!__bits & 1)			\
> 			continue;			\
> 		else					\
>                 ...					\
>             })

Well, forget it !
I did not realize that the 'else' was what called the inner loop.
So this construct it not possible at all either for the same reason.

Sorry for not having read the code before posting.

Cheers,
Willy