Messages in this thread | | | From | Rusty Russell <> | Subject | Re: [Bug #11342] Linux 2.6.27-rc3: kernel BUG at mm/vmalloc.c - bisected | Date | Thu, 25 Sep 2008 11:50:25 +1000 |
| |
On Wednesday 27 August 2008 02:51:46 Linus Torvalds wrote: > On Tue, 26 Aug 2008, Yinghai Lu wrote: > > wonder if could use "unsigned long *" directly. > > I would actually suggest something like this: > > - we continue to have a magic "cpumask_t". > > - we do different cases for big and small NR_CPUS: > > #if NR_CPUS <= BITS_PER_LONG > > /* > * Make it an array - that way passing it as an argument will > * always pass it as a pointer! > */ > typedef unsigned long cpumask_t[1]; > > static inline void create_cpumask(cpumask_t *p) > { > *p = 0; > } > static inline void free_cpumask(cpumask_t *p) > { > } > > #else > > typedef unsigned long *cpumask_t; > > static inline void create_cpumask(cpumask_t *p) > { > *p = kcalloc(..); > } > > static inline void free_cpumask(cpumask_t *p) > { > kfree(*p); > } > > #endif > > and now after you do this, you can just do something like > > cpumask_t mycpu; > > create_cpumask(&mycpu); > .. > free_cpumask(&mycpu); > > and in between, you can use 'cpumask' as a pointer, because even when it > is an array directly allocated on the stack, the array can always > degenerate into a pointer by C type rules!
Hi Linus,
This turns out to be awful in practice, mainly due to const. Consider:
#ifdef CONFIG_CPUMASK_OFFSTACK typedef unsigned long *cpumask_t; #else typedef unsigned long cpumask_t[1]; #endif
cpumask_t returns_cpumask(void);
That's obviously illegal if cpumask_t is an array. So we need a typedef which says "really always a pointer".
typedef unsigned long *cpumask_return_t; cpumask_return_t returns_cpumask(void);
But we usually want it to return a const ptr, and this doesn't work:
const cpumask_return_t returns_cpumask(void); foo.c:12: warning: type qualifiers ignored on function return type
So now we need: typedef const unsigned long *cpumask_const_return_t; cpumask_const_return_t returns_cpumask(void);
OK, now consider a function which wants to take a const cpu bitmap:
void cpus_copy(cpumask_t dst, const cpumask_t src); ... cpus_copy(cpus, returns_cpumask()); foo.c:34: warning: passing argument 2 of ‘cpus_copy’ discards qualifiers from pointer target type
Oops, that didn't work with the pointer version. So we need another typedef: #ifdef CONFIG_CPUMASK_OFFSTACK typedef const unsigned long *cpumask_const_t; #else typedef const unsigned long cpumask_const_t[1]; #endif
void cpus_copy(cpumask_t dst, cpumask_const_t src);
We end up with this: #ifdef CONFIG_CPUMASK_OFFSTACK typedef unsigned long *cpumask_t; typedef const unsigned long *cpumask_const_t; #else typedef unsigned long cpumask_t[1]; typedef const unsigned long cpumask_const_t[1]; #endif typedef unsigned long *cpumask_return_t; typedef const unsigned long *cpumask_const_return_t; typedef unsigned long cpumask_data_t[1];
I can't see a neater way down this path, and I don't want to lose const.
I can see three alternatives: 1) An ONSTACK_CPUMASK(name) macro which declares "struct cpumask name[1]" or "struct cpumask *name". Same idea as yours, without the typedef. 2) Use a normal struct for cpumask, make everyone use pointers, but have an struct cpumask *alloc_stack_cpumask() which uses alloca() for small NR_CPUS. 3) Same, but just use kmalloc everywhere. Optimize important cases by hand.
Anyone see a better way? Rusty. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |