Re: [PATCH 3/6] cpumask: truncate task_struct.cpus_allowed for CONFIG_CPUMASK_OFFSTACK

[Ingo Molnar <mingo@elte.hu>]

* Rusty Russell <rusty@rustcorp.com.au> wrote:

> Turns cpus_allowed into a bitmap, and truncate it to nr_cpu_ids if
> CONFIG_CPUMASK_OFFSTACK is set.
> 
> I do this rather than the classic [0] dangling array trick, because of
> INIT_TASK and references to sizeof(struct task_struct).
> 
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> ---
>  include/linux/init_task.h |    2 +-
>  include/linux/sched.h     |    7 +++++--
>  kernel/fork.c             |   19 ++++++++++++++++++-
>  3 files changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/init_task.h b/include/linux/init_task.h
> --- a/include/linux/init_task.h
> +++ b/include/linux/init_task.h
> @@ -130,7 +130,7 @@ extern struct cred init_cred;
>  	.static_prio	= MAX_PRIO-20,					\
>  	.normal_prio	= MAX_PRIO-20,					\
>  	.policy		= SCHED_NORMAL,					\
> -	.cpus_allowed	= CPU_MASK_ALL,					\
> +	.cpus_allowed	= CPU_BITS_ALL,					\
>  	.mm		= NULL,						\
>  	.active_mm	= &init_mm,					\
>  	.se		= {						\
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1256,7 +1256,6 @@ struct task_struct {
>  #endif
>  
>  	unsigned int policy;
> -	cpumask_t cpus_allowed;
>  
>  #ifdef CONFIG_TREE_PREEMPT_RCU
>  	int rcu_read_lock_nesting;
> @@ -1544,10 +1543,14 @@ struct task_struct {
>  	unsigned long trace_recursion;
>  #endif /* CONFIG_TRACING */
>  	unsigned long stack_start;
> +
> +	/* This has to go at the end: if CONFIG_CPUMASK_OFFSTACK=y, only
> +	 * nr_cpu_ids bits will actually be allocated. */
> +	DECLARE_BITMAP(cpus_allowed, CONFIG_NR_CPUS);

(nit: please use the comment style you see elsewhere in the file.)

>  };
>  
>  /* Future-safe accessor for struct task_struct's cpus_allowed. */
> -#define tsk_cpumask(tsk) (&(tsk)->cpus_allowed)
> +#define tsk_cpumask(tsk) (to_cpumask((tsk)->cpus_allowed))

Please use tsk_cpus_allowed() throughout - so that people who knew what 
p->cpus_allowed did know what this new thing does.

Also, i'm still having second thoughts about it all - could we somehow 
avoid all this wrappery of commonly used fields? (My main and pretty 
much only worry is that struct field members look so much cleaner than 
some wrapper intermediary.)

	Ingo