Messages in this thread | | | Date | Wed, 16 Jul 2008 05:25:31 -0700 | From | Mike Travis <> | Subject | Re: [PATCH 7/8] cpumask: Provide a generic set of CPUMASK_ALLOC macros |
| |
Bert Wesarg wrote: > On Tue, Jul 15, 2008 at 23:14, Mike Travis <travis@sgi.com> wrote: >> * Provide a generic set of CPUMASK_ALLOC macros patterned after the >> SCHED_CPUMASK_ALLOC macros. This is used where multiple cpumask_t >> variables are declared on the stack to reduce the amount of stack >> space required. >> >> Based on linux-2.6.tip/master at the following commit: >> >> commit 0a91813e16ebd5c2d9b5c2acd5b7c91742112c4f >> Merge: 9a635fa... 724dce0... >> Author: Ingo Molnar <mingo@elte.hu> >> Date: Tue Jul 15 14:55:17 2008 +0200 >> >> Signed-off-by: Mike Travis <travis@sgi.com> >> Cc: Paul Jackson <pj@sgi.com> >> --- >> include/linux/cpumask.h | 21 +++++++++++++++++++++ >> 1 file changed, 21 insertions(+) >> >> --- linux-2.6.tip.orig/include/linux/cpumask.h >> +++ linux-2.6.tip/include/linux/cpumask.h >> @@ -75,6 +75,17 @@ >> * CPU_MASK_NONE Initializer - no bits set >> * unsigned long *cpus_addr(mask) Array of unsigned long's in mask >> * >> + *if NR_CPUS > BITS_PER_LONG >> + * CPUMASK_ALLOC(m) Declares and allocates struct m *m = >> + * (struct m *)kmalloc(sizeof(*m), ...) > Shouldn't you mention the GFP_KERNEL flag? And the cast should not > necessarily be mentioned in a comment.
Hmm, yes. I encountered the checkpatch warning about casting kmalloc and changed the macro but not the comment. (And now there's room for adding GFP_KERNEL to the comment! ;-) > >> + * CPUMASK_FREE(m) Macro for kfree(v) > kfree(m) > >> + *else >> + * CPUMASK_ALLOC(m) Declares struct m _m, *m = &_m >> + * CPUMASK_FREE(m) Nop >> + *endif >> + * CPUMASK_VAR(v, m) Declares cpumask_t *v = >> + * m + offset(struct m, v) > offsetof > and why can't you use a &(m->v)?
I thought about this but what if kmalloc fails? It's ok to add the offset to a NULL pointer, but dereferencing a null pointer (even though it's just to get the address) may introduce a fault, yes?
I'll look into this further. There was one other remnant from the copy/paste that needed to be fixed:
-#define CPUMASK_ALLOC(m) struct allmasks _m, *m = &_m +#define CPUMASK_ALLOC(m) struct m _m, *m = &_m
Thanks! Mike
| |