[PULL] Wrapper macros for struct task_struct and struct mm_struct cpumask transition

[PULL] Wrapper macros for struct task_struct and struct mm_struct cpumask transition

[Rusty Russell]

The following changes since commit 99adcd9d67aaf04e28f5ae96df280f236bde4b66:
  Linus Torvalds (1):
        Merge branch 'fixes' of git://git.kernel.org/.../davej/cpufreq

are available in the git repository at:

  ssh://master.kernel.org/pub/scm/linux/kernel/git/rusty/linux-2.6-for-linus.git master

Rusty Russell (2):
      cpumask: tsk_cpumask for accessing the struct task_struct's cpus_allowed.
      cpumask: mm_cpumask for accessing the struct mm_struct's cpu_vm_mask.

 include/linux/mm_types.h |    3 +++
 include/linux/sched.h    |    3 +++
 2 files changed, 6 insertions(+), 0 deletions(-)

Re: [PULL] Wrapper macros for struct task_struct and struct mm_struct cpumask transition

[Nick Piggin]

On Thursday 12 March 2009 15:06:52 Rusty Russell wrote:
> The following changes since commit
> 99adcd9d67aaf04e28f5ae96df280f236bde4b66: Linus Torvalds (1):
>         Merge branch 'fixes' of git://git.kernel.org/.../davej/cpufreq
>
> are available in the git repository at:
>
>  
> ssh://master.kernel.org/pub/scm/linux/kernel/git/rusty/linux-2.6-for-linus.
>git master
>
> Rusty Russell (2):
>       cpumask: tsk_cpumask for accessing the struct task_struct's
> cpus_allowed. cpumask: mm_cpumask for accessing the struct mm_struct's
> cpu_vm_mask.
>
>  include/linux/mm_types.h |    3 +++
>  include/linux/sched.h    |    3 +++

What's the transition?

Re: [PULL] Wrapper macros for struct task_struct and struct mm_struct cpumask transition

[Rusty Russell]

On Thursday 12 March 2009 14:50:11 Nick Piggin wrote:
> On Thursday 12 March 2009 15:06:52 Rusty Russell wrote:
> > Rusty Russell (2):
> >       cpumask: tsk_cpumask for accessing the struct task_struct's cpus_allowed.
> >       cpumask: mm_cpumask for accessing the struct mm_struct's cpu_vm_mask.
> >
> >  include/linux/mm_types.h |    3 +++
> >  include/linux/sched.h    |    3 +++
> 
> What's the transition?

Generally, cpumask_t to cpumask_var_t, ie. a struct cpumask [1] normally,
or a struct cpumask * for CONFIG_CPUMASK_OFFSTACK=y (currently x86 only).

In these cases though, we're allocating them anyway so it makes more sense to
do a dangling bitmap at the end of the struct (and only allocate nr_cpu_ids
bits when CONFIG_CPUMASK_OFFSTACK=y).

I had a patch which used a const struct cpumask * for task_struct: we only
ever replace the whole thing, so we can often use the standard cpu_mask_all
or cpumask_of() and only allocate when it's set to something else.  But
the code was pretty ugly.

Anyway, it's a trivial to change if people use the wrappers.  And if the
wrappers are in Linus' tree, it's easy to get the conversions into linux-next.

Cheers,
Rusty.

Re: [PULL] Wrapper macros for struct task_struct and struct mm_struct cpumask transition

[Andrew Morton]

On Thu, 12 Mar 2009 17:45:22 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote:

> On Thursday 12 March 2009 14:50:11 Nick Piggin wrote:
> > On Thursday 12 March 2009 15:06:52 Rusty Russell wrote:
> > > Rusty Russell (2):
> > >       cpumask: tsk_cpumask for accessing the struct task_struct's cpus_allowed.
> > >       cpumask: mm_cpumask for accessing the struct mm_struct's cpu_vm_mask.
> > >
> > >  include/linux/mm_types.h |    3 +++
> > >  include/linux/sched.h    |    3 +++

Please prefer to put the patches in the email if practical?  More
eyeballs and all that.

<goes fishing in linux-next>


: commit ccbc7af2a675bce8a9d4d3ab8e9c8a71a39d9158
: Author:     Rusty Russell <rusty@rustcorp.com.au>
: AuthorDate: Tue Mar 10 10:03:44 2009 +1100
: Commit:     Stephen Rothwell <sfr@canb.auug.org.au>
: CommitDate: Tue Mar 10 10:03:44 2009 +1100
: 
:     cpumask:task_cpumask
:     
:     This allows us to change the representation (to a dangling bitmap)
:     without breaking all the callers: they can use tsk_cpumask() now and
:     won't see a difference as the changes roll into linux-next.
:     
:     Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
: 
: diff --git a/include/linux/sched.h b/include/linux/sched.h
: index 8c216e0..011db2f 100644
: --- a/include/linux/sched.h
: +++ b/include/linux/sched.h
: @@ -1419,6 +1419,9 @@ struct task_struct {
:  #endif
:  };
:  
: +/* Future-safe accessor for struct task_struct's cpus_allowed. */
: +#define tsk_cpumask(tsk) (&(tsk)->cpus_allowed)
: +
:  /*
:   * Priority of a process goes from 0..MAX_PRIO-1, valid RT
:   * priority is 0..MAX_RT_PRIO-1, and SCHED_NORMAL/SCHED_BATCH
: 
: 
: 
: commit 614e0176340928efb92cba676126048fdd466f0f
: Author:     Rusty Russell <rusty@rustcorp.com.au>
: AuthorDate: Tue Mar 10 10:03:44 2009 +1100
: Commit:     Stephen Rothwell <sfr@canb.auug.org.au>
: CommitDate: Tue Mar 10 10:03:44 2009 +1100
: 
:     cpumask:mm_cpumask
:     
:     This allows us to change the representation (to a dangling bitmap)
:     without breaking all the callers: they can use mm_cpumask() now and
:     won't see a difference as the changes roll into linux-next.
:     
:     Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
: 
: diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
: index 92915e8..d84feb7 100644
: --- a/include/linux/mm_types.h
: +++ b/include/linux/mm_types.h
: @@ -276,4 +276,7 @@ struct mm_struct {
:  #endif
:  };
:  
: +/* Future-safe accessor for struct mm_struct's cpu_vm_mask. */
: +#define mm_cpumask(mm) (&(mm)->cpu_vm_mask)
: +
:  #endif /* _LINUX_MM_TYPES_H */
: 


It would be pretty perverse to run tsk_cpumask() against a `struct
cpuset*', but your proposed implementation would merrily permit that
mistake.

Can we write the kernel in C please??


> And if the
> wrappers are in Linus' tree, it's easy to get the conversions into linux-next.

yup, the patches are quite safe and mergeable.  And if they'd been in the
email body, Linus might have seen that and pulled them ;)

Re: [PULL] Wrapper macros for struct task_struct and struct mm_struct cpumask transition

[Rusty Russell]

On Thursday 12 March 2009 18:07:09 Andrew Morton wrote:
> On Thu, 12 Mar 2009 17:45:22 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote:
...
> Please prefer to put the patches in the email if practical?  More
> eyeballs and all that.

Hmm, yes.  Is there a preferred way of doing that with git request-pull?
Or just append the diff?

> : +/* Future-safe accessor for struct mm_struct's cpu_vm_mask. */
> : +#define mm_cpumask(mm) (&(mm)->cpu_vm_mask)
> : +
...
> It would be pretty perverse to run tsk_cpumask() against a `struct
> cpuset*', but your proposed implementation would merrily permit that
> mistake.
> 
> Can we write the kernel in C please??

You mean use an inline?  No, it's a bad idea for two reasons.  The minor
reason is that this macro is a temporary so it makes more sense to fix the
final version.

But the major one is const correctness.  Macros give a const value for
const input.  We can enhance the macro to check, but that's even uglier:

  static inline int check_is_task_struct(const struct task_struct *t) { }
  #define tsk_cpumask(t) (sizeof(check_is_task_struct(t)), &(t)->cpu_vm_mask)

> yup, the patches are quite safe and mergeable.  And if they'd been in the
> email body, Linus might have seen that and pulled them ;)

Probably not, as that's not what he complained about last time.  He might have
been enlightened by the commit messages tho.

Cheers,
Rusty.

Re: [PULL] Wrapper macros for struct task_struct and struct mm_struct cpumask transition

[Andrew Morton]

On Thu, 12 Mar 2009 19:43:02 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote:

> On Thursday 12 March 2009 18:07:09 Andrew Morton wrote:
> > On Thu, 12 Mar 2009 17:45:22 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote:
> ...
> > Please prefer to put the patches in the email if practical?  More
> > eyeballs and all that.
> 
> Hmm, yes.  Is there a preferred way of doing that with git request-pull?
> Or just append the diff?

You're asking me about git?

> > : +/* Future-safe accessor for struct mm_struct's cpu_vm_mask. */
> > : +#define mm_cpumask(mm) (&(mm)->cpu_vm_mask)
> > : +
> ...
> > It would be pretty perverse to run tsk_cpumask() against a `struct
> > cpuset*', but your proposed implementation would merrily permit that
> > mistake.
> > 
> > Can we write the kernel in C please??
> 
> You mean use an inline?  No, it's a bad idea for two reasons.  The minor
> reason is that this macro is a temporary so it makes more sense to fix the
> final version.
> 
> But the major one is const correctness.  Macros give a const value for
> const input.

hm.  Sounds weaselly.

>  We can enhance the macro to check, but that's even uglier:
> 
>   static inline int check_is_task_struct(const struct task_struct *t) { }
>   #define tsk_cpumask(t) (sizeof(check_is_task_struct(t)), &(t)->cpu_vm_mask)

include/linux/typecheck.h has help?

> > yup, the patches are quite safe and mergeable.  And if they'd been in the
> > email body, Linus might have seen that and pulled them ;)
> 
> Probably not, as that's not what he complained about last time.

Linus?  Complain?

>  He might have
> been enlightened by the commit messages tho.