Re: [PATCH] cpumask 5/10 rewrite cpumask.h - single bitmap basedimplementation

[Rusty Russell]

On Fri, 2004-06-04 at 03:10, Paul Jackson wrote:
> cpumask 5/10 rewrite cpumask.h - single bitmap based implementation
> 
> 	Major rewrite of cpumask to use a single implementation,
> 	as a struct-wrapped bitmap.

Go Paul!

> + * 1) The 'type-checked' form of cpu_isset() causes gcc (3.3.2, anyway)
> + *    to generate slightly worse code.  Note for example the additional
> + *    40 lines of assembly code compiling the "for each possible cpu"
> + *    loops buried in the disk_stat_read() macros calls when compiling
> + *    drivers/block/genhd.c (arch i386, CONFIG_SMP=y).  So use a simple
> + *    one-line #define for cpu_isset(), instead of wrapping an inline
> + *    inside a macro, the way we do the other calls.

Hmm... 

> +/* No static inline type checking - see Subtlety (1) above. */
> +#define cpu_isset(cpu, cpumask) test_bit((cpu), (cpumask).bits)

How about something really grungy like:

#define cpu_isset(cpu, cpumask)				\
	({ __typeof__(cpumask) __cpumask;		\
	   (void)(&__cpumask) == (cpumask_t *)0);	\
	   test_bit((cpu), (cpumask).bits); })

> +#define cpus_addr(src) ((src).bits)

We've discussed this before when talking about whether it'd be easier to
just make people use raw bitop functions directly, so I know we have
philosophical differences here.

So, opinion alert: if I were doing this, I'd probably live without this
macro; in my mind it crosses the "too much abstraction" line.  I did
momentarily wonder what this macro did when I saw it used in the
succeeding patches.

But it's a minor nit; thanks for doing these.

Rusty.
-- 
Anyone who quotes me in their signature is an idiot -- Rusty Russell

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/