| 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
| ||
|
Message-Id: <1221123630.4415.1136.camel@twins.programming.kicks-ass.net>
Date: Thu, 11 Sep 2008 11:00:30 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Mike Travis <travis@....com>
Cc: Ingo Molnar <mingo@...e.hu>,
Andrew Morton <akpm@...ux-foundation.org>,
davej@...emonkey.org.uk, David Miller <davem@...emloft.net>,
Eric Dumazet <dada1@...mosbay.com>,
"Eric W. Biederman" <ebiederm@...ssion.com>,
Jack Steiner <steiner@....com>,
Jeremy Fitzhardinge <jeremy@...p.org>,
Jes Sorensen <jes@....com>, "H. Peter Anvin" <hpa@...or.com>,
Thomas Gleixner <tglx@...utronix.de>,
linux-kernel@...r.kernel.org,
Christoph Lameter <cl@...ux-foundation.org>,
Andi Kleen <andi@...stfloor.org>
Subject: Re: [RFC] CPUMASK: proposal for replacing cpumask_t
On Wed, 2008-09-10 at 15:47 -0700, Mike Travis wrote:
> Ingo Molnar wrote:
> > * Peter Zijlstra <peterz@...radead.org> wrote:
> >
> >>>> NAK
> ....
> >
> > seconded. Mike, since none of this is v2.6.27 material, lets do it right
> > with a v2.6.28 target. You know all the cpumask_t using code sites
> > inside out already, so the know-how is all available already :-) Please
> > make it finegrained series of patches so that we can resolve conflicts
> > with other trees more easily.
> >
> > perhaps propose the new cpumask_t API early (in this thread?), so that
> > people can comment on it before NAKs come flying against a full patchset
> > ;-)
> >
> > Ingo
>
>
> Here's an initial proposal for abstracting cpumask_t to be either
> an array of 1 or a pointer to an array... Hopefully this will
> minimize the amount of code changes while providing the capabilities
> this change is attempting to do.
>
> Comments most welcome. ;-)
>
> Thanks,
> Mike
> --
>
> Basically, linux/cpumask.h has the following defines:
>
> typedef struct { DECLARE_BITMAP(bits, NR_CPUS); } cpumask_data;
>
> #if NR_CPUS > BITS_PER_LONG
> typedef const cpumask_data *cpumask_t;
> typedef cpumask_data *cpumask_var;
> typedef cpumask_t cpumask_val;
> #define _NR_CPUS nr_cpu_ids
> #else
> typedef const cpumask_data cpumask_t[1];
> typedef cpumask_data cpumask_var[1];
> typedef cpumask_data cpumask_val;
> #define _NR_CPUS NR_CPUS
> #endif
>
> So in function prototypes:
>
> cpumask_t function(const cpumask_t *A,
> cpumask_t *B,
> cpumask_t cpumask_C)
>
> becomes:
>
> cpumask_val function(cpumask_t A,
> cpumask_var B,
> cpumask_t cpumask_C)
I guess we have to stick the const into the typedef otherwise we get a
const pointer instead of a const array member, right?
In which case I much prefer the following names:
cpumask_data_t - value
const_cpumask_t - pointer to constant value
cpumask_t - pointer to value
> And in local variables:
>
> cpumask_t ==> cpumask_var IFF variable is to be written.
>
> This:
>
> cpumask_t mask = cpu_online_map
> <change mask>
>
> becomes:
>
> #include <linux/cpumask_alloc.h>
>
> cpumask_var mask;
>
> alloc_cpumask(&mask);
Don't you have to deal with allocation errors?
> *mask = *cpu_online_map;
> <change mask>
> free_cpumask(&mask);
>
> Currently, alloc_cpumask is:
>
> #define BYTES_PER_CPUMASK (BITS_TO_LONGS(nr_cpu_ids)/sizeof(long))
>
> static inline bool no_cpumask(cpumask_t *m)
> {
> return (*m == NULL);
> }
>
> static inline void alloc_cpumask(cpumask_t *m)
> {
> cpumask_t d = kmalloc(BYTES_PER_CPUMASK, GFP_KERNEL);
> if (no_cpumask(&d))
> BUG();
yuckery yuck yuck!
> *m = d;
> }
>
> static inline void alloc_cpumask_nopanic(cpumask_t *m)
> {
> cpumask_t d = kmalloc(BYTES_PER_CPUMASK, GFP_KERNEL);
>
> *m = d;
> }
gah - at the very least you got the naming wrong, methinks the one
panic-ing should have panic in its name - if you really want to persist
with that variant.
> static inline void free_cpumask(cpumask_t *m)
> {
> kfree(*m);
> }
>
> Other means of obtaining a temporary cpumask_t variable will be provided
> for those cases where kmalloc() is not available.
>
> Furthermore, system-wide maps become:
>
> extern cpumask_data _cpu_possible_map; /* read/write */
> extern cpumask_data _cpu_online_map;
> extern cpumask_data _cpu_present_map;
> extern cpumask_data _cpu_active_map;
> #define cpu_possible_map ((cpumask_t)&_cpu_possible_map) /* read only */
> #define cpu_online_map ((cpumask_t)&_cpu_online_map)
> #define cpu_present_map ((cpumask_t)&_cpu_present_map)
> #define cpu_active_map ((cpumask_t)&_cpu_active_map)
>
>
> So code to set these bits would be:
>
> cpu_set(cpu, &_cpu_online_map);
> cpu_set(cpu, &_cpu_present_map);
> cpu_set(cpu, &_cpu_possible_map);
>
> Arrays that contain a fixed cpumask would have:
>
> struct xxx {
> cpumask_data cpumask;
> };
>
> .... though we should probably encourage the map to be allocated:
>
> struct xxx {
> cpumask_t readonly_cpumask;
> cpumask_var readwrite_cpumask;
> };
>
> alloc_cpumask(&xxx->readonly_cpumask);
> alloc_cpumask(&xxx->readwrite_cpumask);
>
>
> All the cpu operators become:
>
> #define cpu_XXX(dst, src) _cpu_XXX(dst, src, _NR_CPUS)
> static inline void __cpu_XXX(cpumask_var dstp, cpumask_t srcp, int count)
> {
> XXX_bit(dstp->bits, srcp->bits, count);
> }
>
> (_NR_CPUS being defined to be nr_cpu_ids allows us to allocate variable lengthed arrays.)
>
>
> Cpumask initializers become:
>
>
> #if NR_CPUS <= BITS_PER_LONG
>
> #define INIT_CPU_MASK_ALL \
> (cpumask_t) { { \
> [BITS_TO_LONGS(NR_CPUS)-1] = CPU_MASK_LAST_WORD \
> } }
>
> #else
> #define INIT_CPU_MASK_ALL \
> (cpumask_data) { { \
> [0 ... BITS_TO_LONGS(NR_CPUS)-2] = ~0UL, \
> [BITS_TO_LONGS(NR_CPUS)-1] = CPU_MASK_LAST_WORD \
> } }
> #endif
>
> #define INIT_CPU_MASK_NONE \
> (cpumask_data) { { \
> [0 ... BITS_TO_LONGS(NR_CPUS)-1] = 0UL \
> } }
>
> #define INIT_CPU_MASK_CPU0 \
> (cpumask_data) { { \
> [0] = 1UL \
> } }
>
> #if NR_CPUS > BITS_PER_LONG
> extern cpumask_t cpu_mask_all, cpu_mask_none, cpu_mask_cpu0;
> #define CPU_MASK_ALL (cpu_mask_all)
> #define CPU_MASK_NONE (cpu_mask_none)
> #define CPU_MASK_CPU0 (cpu_mask_cpu0)
> #else
> #define CPU_MASK_ALL ((cpumask_t)&INIT_CPU_MASK_ALL)
> #define CPU_MASK_NONE ((cpumask_t)&INIT_CPU_MASK_NONE)
> #define CPU_MASK_CPU0 ((cpumask_t)&INIT_CPU_MASK_CPU0)
> #endif
>
> And in kernel/cpu.c:
>
> /*
> * provide const cpumask_t's
> */
> #if NR_CPUS > BITS_PER_LONG
> cpumask_data cpu_mask_all __read_mostly = INIT_CPU_MASK_ALL;
> EXPORT_SYMBOL(cpu_mask_all);
>
> cpumask_data cpu_mask_none __read_mostly = INIT_CPU_MASK_NONE;
> EXPORT_SYMBOL(cpu_mask_none);
>
> cpumask_data cpu_mask_cpu0 __read_mostly = INIT_CPU_MASK_CPU0;
> EXPORT_SYMBOL(cpu_mask_cpu0);
> #endif
>
--
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/
Powered by blists - more mailing lists