Re: tip: demise of tsk_cpus_allowed() and tsk_nr_cpus_allowed()
From: Mike Galbraith
Date: Mon Feb 06 2017 - 07:47:59 EST
On Mon, 2017-02-06 at 13:29 +0100, Ingo Molnar wrote:
> * Mike Galbraith <efault@xxxxxx> wrote:
>
> > On Mon, 2017-02-06 at 11:31 +0100, Ingo Molnar wrote:
> > > * Mike Galbraith <efault@xxxxxx> wrote:
> > >
> > > > Hi Ingo,
> > > >
> > > > Doing my ~daily tip merge of -rt, I couldn't help noticing $subject, as
> > > > they grow more functionality in -rt, which is allegedly slowly but
> > > > surely headed toward merge. I don't suppose they could be left intact?
> > > > I can easily restore them in my local tree, but it seems a bit of a
> > > > shame to whack these integration friendly bits.
> > >
> > > Oh, I missed that. How is tsk_cpus_allowed() wrapped in -rt right now?
> >
> > RT extends them to reflect whether migration is disabled or not.
> >
> > +/* Future-safe accessor for struct task_struct's cpus_allowed. */
> > +static inline const struct cpumask *tsk_cpus_allowed(struct task_struct *p)
> > +{
> > + if (__migrate_disabled(p))
> > + return cpumask_of(task_cpu(p));
> > +
> > + return &p->cpus_allowed;
> > +}
> > +
> > +static inline int tsk_nr_cpus_allowed(struct task_struct *p)
> > +{
> > + if (__migrate_disabled(p))
> > + return 1;
> > + return p->nr_cpus_allowed;
> > +}
>
> So ... I think the cleaner approach in -rt would be to introduce
> ->cpus_allowed_saved, and when disabling/enabling migration then saving the
> current mask there and changing ->cpus_allowed - and then restoring it when
> re-enabling migration.
>
> This means ->cpus_allowed could be used by the scheduler directly, no wrappery
> would be required, AFAICS.
>
> ( Some extra care would be required in places that change ->cpus_allowed because
> they'd now have to be aware of ->cpus_allowed_saved. )
>
> Am I missing something?
I suppose it's a matter of personal preference. I prefer the above,
looks nice and clean to me. Hohum, I'll just put them back locally for
the nonce. My trees are only place holders until official releases
catch up anyway.
-Mike