Re: [PATCH 7/8] cpumask: Provide a generic set of CPUMASK_ALLOC macros

[Mike Travis <travis@sgi.com>]

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