| Message ID | 20210811201354.1976839-3-valentin.schneider@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rcu, arm64: PREEMPT_RT fixlets | expand |
On 2021-08-11 21:13:52 [+0100], Valentin Schneider wrote: > diff --git a/include/linux/sched.h b/include/linux/sched.h > index debc960f41e3..8ba7b4a7ee69 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -1715,6 +1715,16 @@ static inline bool is_percpu_thread(void) > #endif > } > > +/* Is the current task guaranteed to stay on its current CPU? */ > +static inline bool migratable(void) > +{ > +#ifdef CONFIG_SMP > + return preemptible() && !current->migration_disabled; > +#else > + return true; shouldn't this be false in the UP case? > +#endif > +} > + > /* Per-process atomic flags. */ > #define PFA_NO_NEW_PRIVS 0 /* May not gain new privileges. */ > #define PFA_SPREAD_PAGE 1 /* Spread page cache over cpuset */ Sebastian
On 2021-08-11 21:13:52 [+0100], Valentin Schneider wrote: > diff --git a/include/linux/sched.h b/include/linux/sched.h > index debc960f41e3..8ba7b4a7ee69 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -1715,6 +1715,16 @@ static inline bool is_percpu_thread(void) > #endif > } > > +/* Is the current task guaranteed to stay on its current CPU? */ > +static inline bool migratable(void) I'm going to rename this in my tree to `is_migratable' because of |security/keys/trusted-keys/trusted_core.c:45:22: error: ‘migratable’ redeclared as different kind of symbol | 45 | static unsigned char migratable; | | ^~~~~~~~~~ |In file included from arch/arm64/include/asm/compat.h:16, | from arch/arm64/include/asm/stat.h:13, | from include/linux/stat.h:6, | from include/linux/sysfs.h:22, | from include/linux/kobject.h:20, | from include/linux/of.h:17, | from include/linux/irqdomain.h:35, | from include/linux/acpi.h:13, | from include/linux/tpm.h:21, | from include/keys/trusted-type.h:12, | from security/keys/trusted-keys/trusted_core.c:10: |include/linux/sched.h:1719:20: note: previous definition of ‘migratable’ was here | 1719 | static inline bool migratable(void) | | ^~~~~~~~~~ Sebastian
On Tue, Aug 17, 2021 at 07:09:25PM +0200 Sebastian Andrzej Siewior wrote: > On 2021-08-11 21:13:52 [+0100], Valentin Schneider wrote: > > diff --git a/include/linux/sched.h b/include/linux/sched.h > > index debc960f41e3..8ba7b4a7ee69 100644 > > --- a/include/linux/sched.h > > +++ b/include/linux/sched.h > > @@ -1715,6 +1715,16 @@ static inline bool is_percpu_thread(void) > > #endif > > } > > > > +/* Is the current task guaranteed to stay on its current CPU? */ > > +static inline bool migratable(void) > > I'm going to rename this in my tree to `is_migratable' because of It's better anyway. See is_percpu_thread() 5 lines above :) > |security/keys/trusted-keys/trusted_core.c:45:22: error: ‘migratable’ redeclared as different kind of symbol > | 45 | static unsigned char migratable; > | | ^~~~~~~~~~ > |In file included from arch/arm64/include/asm/compat.h:16, > | from arch/arm64/include/asm/stat.h:13, > | from include/linux/stat.h:6, > | from include/linux/sysfs.h:22, > | from include/linux/kobject.h:20, > | from include/linux/of.h:17, > | from include/linux/irqdomain.h:35, > | from include/linux/acpi.h:13, > | from include/linux/tpm.h:21, > | from include/keys/trusted-type.h:12, > | from security/keys/trusted-keys/trusted_core.c:10: > |include/linux/sched.h:1719:20: note: previous definition of ‘migratable’ was here > | 1719 | static inline bool migratable(void) > | | ^~~~~~~~~~ > > Sebastian >
On 17/08/21 16:43, Sebastian Andrzej Siewior wrote: > On 2021-08-11 21:13:52 [+0100], Valentin Schneider wrote: >> diff --git a/include/linux/sched.h b/include/linux/sched.h >> index debc960f41e3..8ba7b4a7ee69 100644 >> --- a/include/linux/sched.h >> +++ b/include/linux/sched.h >> @@ -1715,6 +1715,16 @@ static inline bool is_percpu_thread(void) >> #endif >> } >> >> +/* Is the current task guaranteed to stay on its current CPU? */ >> +static inline bool migratable(void) >> +{ >> +#ifdef CONFIG_SMP >> + return preemptible() && !current->migration_disabled; >> +#else >> + return true; > > shouldn't this be false in the UP case? > Yes indeed, forgot to flip that one when inverting the logic.
On 17/08/21 19:09, Sebastian Andrzej Siewior wrote: > On 2021-08-11 21:13:52 [+0100], Valentin Schneider wrote: >> diff --git a/include/linux/sched.h b/include/linux/sched.h >> index debc960f41e3..8ba7b4a7ee69 100644 >> --- a/include/linux/sched.h >> +++ b/include/linux/sched.h >> @@ -1715,6 +1715,16 @@ static inline bool is_percpu_thread(void) >> #endif >> } >> >> +/* Is the current task guaranteed to stay on its current CPU? */ >> +static inline bool migratable(void) > > I'm going to rename this in my tree to `is_migratable' because of <snip> Thanks for carrying it through, I'll keep that change for the next version. > Sebastian
On 2021-08-22 19:14:18 [+0100], Valentin Schneider wrote:
> Thanks for carrying it through, I'll keep that change for the next version.
Just a quick question. This series ended with 3 patches in my queue. It
got decimated further due to Frederic series which ended up in
v5.17-rc1. I still have
| 2021-08-11 21:13 +0100 Valentin Schneider ∙ sched: Introduce migratable()
| 2021-08-11 21:13 +0100 Valentin Schneider ∙ arm64: mm: Make arch_faults_on_old_pte() check for migratability
what do we do about these two? I could repost these two if there are no
objections…
Sebastian
On 26/01/22 17:56, Sebastian Andrzej Siewior wrote: > On 2021-08-22 19:14:18 [+0100], Valentin Schneider wrote: >> Thanks for carrying it through, I'll keep that change for the next version. > > Just a quick question. This series ended with 3 patches in my queue. It > got decimated further due to Frederic series which ended up in > v5.17-rc1. I still have > | 2021-08-11 21:13 +0100 Valentin Schneider ∙ sched: Introduce migratable() > | 2021-08-11 21:13 +0100 Valentin Schneider ∙ arm64: mm: Make arch_faults_on_old_pte() check for migratability > > what do we do about these two? I could repost these two if there are no > objections… > Heh, had forgotten about those - I'm happy to repost with the s/migratable/is_migratable/. I also need to go back to those splats I got on my emag and fix the PMU/GPIO warnings... > Sebastian
On 2022-01-26 18:10:51 [+0000], Valentin Schneider wrote: > > | 2021-08-11 21:13 +0100 Valentin Schneider ∙ sched: Introduce migratable() > > | 2021-08-11 21:13 +0100 Valentin Schneider ∙ arm64: mm: Make arch_faults_on_old_pte() check for migratability … > Heh, had forgotten about those - I'm happy to repost with the > s/migratable/is_migratable/. I also need to go back to those splats I got > on my emag and fix the PMU/GPIO warnings... Now that I look at it gain, you might want to drop #1 and then #2 would switch to cant_migrate(). This might work… Sebastian
On 27/01/22 11:07, Sebastian Andrzej Siewior wrote: > On 2022-01-26 18:10:51 [+0000], Valentin Schneider wrote: >> > | 2021-08-11 21:13 +0100 Valentin Schneider ∙ sched: Introduce migratable() >> > | 2021-08-11 21:13 +0100 Valentin Schneider ∙ arm64: mm: Make arch_faults_on_old_pte() check for migratability > … >> Heh, had forgotten about those - I'm happy to repost with the >> s/migratable/is_migratable/. I also need to go back to those splats I got >> on my emag and fix the PMU/GPIO warnings... > > Now that I look at it gain, you might want to drop #1 and then #2 would > switch to cant_migrate(). This might work… > Wasn't aware of cant_migrate(), that does look even better. Lemme give it a shot. > Sebastian
On 26/01/22 17:56, Sebastian Andrzej Siewior wrote: > On 2021-08-22 19:14:18 [+0100], Valentin Schneider wrote: >> Thanks for carrying it through, I'll keep that change for the next version. > > Just a quick question. This series ended with 3 patches in my queue. It > got decimated further due to Frederic series which ended up in > v5.17-rc1. I still have > | 2021-08-11 21:13 +0100 Valentin Schneider ∙ sched: Introduce migratable() > | 2021-08-11 21:13 +0100 Valentin Schneider ∙ arm64: mm: Make arch_faults_on_old_pte() check for migratability > > what do we do about these two? I could repost these two if there are no > objections… > While I'm at it, I see you're still carrying 6ab5bb09040d arm64/sve: Make kernel FPU protection RT friendly If you're OK with it, I'll repost that: https://lore.kernel.org/lkml/20210722175157.1367122-1-valentin.schneider@arm.com/ > Sebastian
On 2022-01-27 19:27:56 [+0000], Valentin Schneider wrote: Sorry, got distracted. > While I'm at it, I see you're still carrying > > 6ab5bb09040d arm64/sve: Make kernel FPU protection RT friendly > > If you're OK with it, I'll repost that: > https://lore.kernel.org/lkml/20210722175157.1367122-1-valentin.schneider@arm.com/ I'm not too keen about preempt_enable_bh(). I would prefer the current approach maybe with a comment why BH here and preemption there is correct. Sebastian
diff --git a/include/linux/sched.h b/include/linux/sched.h index debc960f41e3..8ba7b4a7ee69 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1715,6 +1715,16 @@ static inline bool is_percpu_thread(void) #endif } +/* Is the current task guaranteed to stay on its current CPU? */ +static inline bool migratable(void) +{ +#ifdef CONFIG_SMP + return preemptible() && !current->migration_disabled; +#else + return true; +#endif +} + /* Per-process atomic flags. */ #define PFA_NO_NEW_PRIVS 0 /* May not gain new privileges. */ #define PFA_SPREAD_PAGE 1 /* Spread page cache over cpuset */
Some areas use preempt_disable() + preempt_enable() to safely access per-CPU data. The PREEMPT_RT folks have shown this can also be done by keeping preemption enabled and instead disabling migration (and acquiring a sleepable lock, if relevant). Introduce a helper which checks whether the current task can be migrated elsewhere, IOW if it is pinned to its local CPU in the current context. This can help determining if per-CPU properties can be safely accessed. Note that CPU affinity is not checked here, as a preemptible task can have its affinity changed at any given time (including if it has PF_NO_SETAFFINITY, when hotplug gets involved). Signed-off-by: Valentin Schneider <valentin.schneider@arm.com> --- include/linux/sched.h | 10 ++++++++++ 1 file changed, 10 insertions(+)