From: Ingo Molnar <mingo@kernel.org>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: linux-kernel@vger.kernel.org,
Peter Zijlstra <peterz@infradead.org>,
Thomas Gleixner <tglx@linutronix.de>,
Mike Galbraith <efault@gmx.de>, Ingo Molnar <mingo@elte.hu>,
"Rafael J . Wysocki" <rjw@rjwysocki.net>
Subject: Re: [RFC PATCH] kernel: sched: Provide a pointer to the valid CPU mask
Date: Wed, 5 Apr 2017 09:39:43 +0200 [thread overview]
Message-ID: <20170405073943.GA17266@gmail.com> (raw)
In-Reply-To: <20170404184202.20376-1-bigeasy@linutronix.de>
* Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
> In commit 4b53a3412d66 ("sched/core: Remove the tsk_nr_cpus_allowed()
> wrapper") the tsk_nr_cpus_allowed() wrapper was removed. There was not
> much difference in !RT but in RT we used this to implement
> migrate_disable(). Within a migrate_disable() section the CPU mask is
> restricted to single CPU while the "normal" CPU mask remains untouched.
>
> As an alternative implementation Ingo suggested to use
> struct task_struct {
> const cpumask_t *cpus_ptr;
> cpumask_t cpus_mask;
> };
> with
> t->cpus_allowed_ptr = &t->cpus_allowed;
>
> In -RT we then can switch the cpus_ptr to
> t->cpus_allowed_ptr = &cpumask_of(task_cpu(p));
>
> in a migration disabled region. The rules are simple:
> - Code that 'uses' ->cpus_allowed would use the pointer.
> - Code that 'modifies' ->cpus_allowed would use the direct mask.
>
> While converting the existing users I tried to stick with the rules
> above however… well mostly CPUFREQ tries to temporary switch the CPU
> mask to do something on a certain CPU and then switches the mask back it
> its original value. So in theory `cpus_ptr' could or should be used.
> However if this is invoked in a migration disabled region (which is not
> the case because it would require something like preempt_disable() and
> set_cpus_allowed_ptr() might sleep so it can't be) then the "restore"
> part would restore the wrong mask. So it only looks strange and I go for
> the pointer…
So maybe we could add the following facility:
ptr = sched_migrate_to_cpu_save(cpu);
...
sched_migrate_to_cpu_restore(ptr);
... and use it in the cpufreq code. Then -rt could simply define migrate_disable()
to be:
ptr = sched_migrate_to_cpu_save(raw_smp_processor_id());
and define migrate_enable() as:
sched_migrate_to_cpu_restore(ptr);
... or such.
In the cpu == current_cpu case it would be super fast - otherwise it would migrate
over to the target CPU first. Also note that this facility is strictly a special
case for single-CPU masks and migrations - i.e. the constant pointer cpumask
optimization would always apply.
Note that due to the use of the 'ptr' local variable the interface nests
naturally, so this would be a legitimate use:
ptr = sched_migrate_to_cpu_save(cpu);
...
migrate_disable();
...
migrate_enable();
...
sched_migrate_to_cpu_restore(ptr);
I.e. my proposal would be to essentially upstream the -rt migrate_disable()
facility in a slightly more generic form that would fit the cpufreq usecase.
I bet a number of the current driver's mucking with cpumask would also fit this
new API.
Does this make sense?
Thanks,
Ingo
next prev parent reply other threads:[~2017-04-05 7:39 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-04 18:42 [RFC PATCH] kernel: sched: Provide a pointer to the valid CPU mask Sebastian Andrzej Siewior
2017-04-05 7:39 ` Ingo Molnar [this message]
2017-04-05 8:37 ` Sebastian Andrzej Siewior
2017-04-06 6:16 ` Ingo Molnar
2017-04-06 7:38 ` Sebastian Andrzej Siewior
2017-04-06 8:01 ` Ingo Molnar
2017-04-06 9:25 ` Sebastian Andrzej Siewior
2017-04-06 9:46 ` Peter Zijlstra
2017-04-06 10:58 ` Thomas Gleixner
2017-04-06 11:41 ` Peter Zijlstra
2017-04-06 9:35 ` Peter Zijlstra
2017-04-06 9:42 ` Peter Zijlstra
2017-04-06 10:36 ` Thomas Gleixner
2017-04-06 11:02 ` Ingo Molnar
2017-04-06 11:10 ` Thomas Gleixner
2017-04-07 7:13 ` Ingo Molnar
2017-04-06 9:34 ` Peter Zijlstra
2017-04-06 9:32 ` Peter Zijlstra
2017-04-06 9:46 ` Sebastian Andrzej Siewior
2017-04-06 10:35 ` Peter Zijlstra
2017-04-06 10:47 ` Thomas Gleixner
2017-04-06 10:57 ` Peter Zijlstra
2017-04-06 11:03 ` Thomas Gleixner
2017-04-06 11:50 ` Peter Zijlstra
2017-04-06 11:56 ` Thomas Gleixner
2017-04-06 12:31 ` Peter Zijlstra
2017-04-11 1:38 ` [lkp-robot] [kernel] c1f943ee40: kernel_BUG_at_kernel/smpboot.c kernel test robot
2017-04-11 1:38 ` kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170405073943.GA17266@gmail.com \
--to=mingo@kernel.org \
--cc=bigeasy@linutronix.de \
--cc=efault@gmx.de \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=peterz@infradead.org \
--cc=rjw@rjwysocki.net \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.