linux-kernel - Re: [PATCH 7/8] cpumask: Provide a generic set of CPUMASK

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <487DE8BB.5070407@sgi.com>
Date:	Wed, 16 Jul 2008 05:25:31 -0700
From:	Mike Travis <travis@....com>
To:	Bert Wesarg <bert.wesarg@...glemail.com>
CC:	Rusty Russell <rusty@...tcorp.com.au>, Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	Christoph Lameter <cl@...ux-foundation.org>,
	Jack Steiner <steiner@....com>, linux-kernel@...r.kernel.org,
	Paul Jackson <pj@....com>, LKML <linux-kernel@...r.kernel.org>
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@....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@...e.hu>
>>    Date:   Tue Jul 15 14:55:17 2008 +0200
>>
>> Signed-off-by: Mike Travis <travis@....com>
>> Cc: Paul Jackson <pj@....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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/