by Michael Ellerman

[permalink] [raw]

Subject: Re: [PATCH 4.19 056/191] powerpc: select ARCH_WANT_IRQS_OFF_ACTIVATE_MM

Michael Ellerman <[email protected]> writes:
> Greg Kroah-Hartman <[email protected]> writes:
>> From: Nicholas Piggin <[email protected]>
>>
>> [ Upstream commit 66acd46080bd9e5ad2be4b0eb1d498d5145d058e ]
>>
>> powerpc uses IPIs in some situations to switch a kernel thread away
>> from a lazy tlb mm, which is subject to the TLB flushing race
>> described in the changelog introducing ARCH_WANT_IRQS_OFF_ACTIVATE_MM.
>>
>> Signed-off-by: Nicholas Piggin <[email protected]>
>> Signed-off-by: Michael Ellerman <[email protected]>
>> Link: https://lore.kernel.org/r/[email protected]
>> Signed-off-by: Sasha Levin <[email protected]>
>> ---
>> arch/powerpc/Kconfig | 1 +
>> arch/powerpc/include/asm/mmu_context.h | 2 +-
>> 2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index f38d153d25861..0bc53f0e37c0f 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -152,6 +152,7 @@ config PPC
>> select ARCH_USE_BUILTIN_BSWAP
>> select ARCH_USE_CMPXCHG_LOCKREF if PPC64
>> select ARCH_WANT_IPC_PARSE_VERSION
>> + select ARCH_WANT_IRQS_OFF_ACTIVATE_MM
>
> This depends on upstream commit:
>
> d53c3dfb23c4 ("mm: fix exec activate_mm vs TLB shootdown and lazy tlb switching race")
>
>
> Which I don't see in 4.19 stable, or in the email thread here.
>
> So this shouldn't be backported to 4.19 unless that commit is also
> backported.

I just sent you a backport of d53c3dfb23c4 for 4.19.

cheers

2020-11-04 07:27:55

by Pavel Machek

[permalink] [raw]

Subject: Re: [PATCH 4.19 000/191] 4.19.155-rc1 review

Hi!

> This is the start of the stable review cycle for the 4.19.155 release.
> There are 191 patches in this series, all will be posted as a response
> to this one. If anyone has any issues with these being applied, please
> let me know.
>
> Responses should be made by Thu, 05 Nov 2020 20:29:58 +0000.
> Anything received after that time might be too late.

No errors detected in CIP testing:

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-4.19.y

Tested-by: Pavel Machek (CIP) <[email protected]>

Best regards,
Pavel

--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Attachments:

(No filename) (731.00 B)
signature.asc (188.00 B)
Digital signature Download all attachments

2020-11-04 07:56:29

On Thu, 5 Nov 2020 at 19:24, Pavel Machek <[email protected]> wrote:
>
> Hi!
>
> > From: Joel Stanley <[email protected]>
> >
> > commit a02f6d42357acf6e5de6ffc728e6e77faf3ad217 upstream.
> >
> > It's not done anything for a long time. Save the percpu variable, and
> > emit a warning to remind users to not expect it to do anything.
> >
> > This uses pr_warn_once instead of pr_warn_ratelimit as testing
> > 'ppc64_cpu --smt=off' on a 24 core / 4 SMT system showed the warning
> > to be noisy, as the online/offline loop is slow.
>
> I don't believe this is good idea for stable. It is in 5.9-rc2, and
> likely mainline users will get userspace fixed, but that warning is
> less useful for -stable users.

The warning is about the existing behaviour of the kernel. It does let
the user know that they won't see any difference in behaviour when
tweaking the smt_snooze_delay variable, which was a real issue that
Anton hit.

I agree that the future commit that removes smt_snooze_delay from the
kernel should not be backported.

Cheers,

Joel

>
> (And besides, it does not fix any serious bug).
>
> Best regards,
> Pavel
>
> --
> http://www.livejournal.com/~pavelmachek

2020-11-06 20:16:48

by Krzysztof Kozlowski

[permalink] [raw]

Subject: Re: [PATCH 4.19 107/191] ARM: dts: s5pv210: move PMU node out of clock controller

On Thu, Nov 05, 2020 at 08:55:08PM +0100, Pavel Machek wrote:
> Hi!
>
> > > > The Power Management Unit (PMU) is a separate device which has little
> > > > common with clock controller. Moving it to one level up (from clock
> > > > controller child to SoC) allows to remove fake simple-bus compatible and
> > > > dtbs_check warnings like:
> > > >
> > > > clock-controller@e0100000: $nodename:0:
> > > > 'clock-controller@e0100000' does not match '^([a-z][a-z0-9\\-]+-bus|bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$'
> > >
> > > > +++ b/arch/arm/boot/dts/s5pv210.dtsi
> > > > @@ -98,19 +98,16 @@
> > > > };
> > > >
> > > > clocks: clock-controller@e0100000 {
> > > > - compatible = "samsung,s5pv210-clock", "simple-bus";
> > > > + compatible = "samsung,s5pv210-clock";
> > > > reg = <0xe0100000 0x10000>;
> > > ...
> > > > + pmu_syscon: syscon@e0108000 {
> > > > + compatible = "samsung-s5pv210-pmu", "syscon";
> > > > + reg = <0xe0108000 0x8000>;
> > > > };
> > >
> > > Should clock-controller@e0100000's reg be shortened to 0x8000 so that
> > > the ranges do not overlap?
> > >
> > > Signed-off-by: Pavel Machek (CIP) <[email protected]>
> >
> > I don't think this commit should be backported to stable. It is simple
> > dtbs_check - checking whether Devicetree source matches device tree
> > schema. Neither the schema nor the warning existed in v4.19. I think
> > dtbs_check fixes should not be backported, unless a real issue is
> > pointed out.
>
> I agree with you about the backporting. Hopefully Greg drops the
> commit.
>
> But the other issue is: should mainline be fixed so that ranges do not overlap?

Yes, it should be. This should fail on mapping resources...

I'll take a look, thanks for the report.

Best regards,
Krzysztof

2020-11-06 21:12:43

by Krzysztof Kozlowski

On Tue, Nov 17, 2020 at 07:45:28PM -0500, Sasha Levin wrote:
> On Tue, Nov 17, 2020 at 10:51:16AM -0800, Kamal Mostafa wrote:
> > On Tue, Nov 3, 2020 at 4:22 PM Michael Ellerman <[email protected]> wrote:
> >
> > > Greg Kroah-Hartman <[email protected]> writes:
> > > > From: Nicholas Piggin <[email protected]>
> > > >
> > > > [ Upstream commit 66acd46080bd9e5ad2be4b0eb1d498d5145d058e ]
> > > >
> > > > powerpc uses IPIs in some situations to switch a kernel thread away
> > > > from a lazy tlb mm, which is subject to the TLB flushing race
> > > > described in the changelog introducing ARCH_WANT_IRQS_OFF_ACTIVATE_MM.
> > > >
> > > > Signed-off-by: Nicholas Piggin <[email protected]>
> > > > Signed-off-by: Michael Ellerman <[email protected]>
> > > > Link:
> > > https://lore.kernel.org/r/[email protected]
> > > > Signed-off-by: Sasha Levin <[email protected]>
> > > > ---
> > > > arch/powerpc/Kconfig | 1 +
> > > > arch/powerpc/include/asm/mmu_context.h | 2 +-
> > > > 2 files changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > > > index f38d153d25861..0bc53f0e37c0f 100644
> > > > --- a/arch/powerpc/Kconfig
> > > > +++ b/arch/powerpc/Kconfig
> > > > @@ -152,6 +152,7 @@ config PPC
> > > > select ARCH_USE_BUILTIN_BSWAP
> > > > select ARCH_USE_CMPXCHG_LOCKREF if PPC64
> > > > select ARCH_WANT_IPC_PARSE_VERSION
> > > > + select ARCH_WANT_IRQS_OFF_ACTIVATE_MM
> > >
> > > This depends on upstream commit:
> > >
> > > d53c3dfb23c4 ("mm: fix exec activate_mm vs TLB shootdown and lazy tlb
> > > switching race")
> > >
> > >
> > > Which I don't see in 4.19 stable, or in the email thread here.
> > >
> > > So this shouldn't be backported to 4.19 unless that commit is also
> > > backported.
> > >
> > > cheers
> > >
> >
> > Hi-
> >
> > This glitch has made its way into 4.14.y ...
> > [4.14.y] c2bca8712a19 powerpc: select ARCH_WANT_IRQS_OFF_ACTIVATE_MM
> > But 4.14.y does not carry the prereq that introduces that config.
>
> I'll queue up the 4.19 backport for 4.14 too, thanks!
>

Thanks Sasha.

And nevermind my other concern ...

> > It would seem that the intent is that it should be *only* enabled
> > (currently at least) for arches that will explicitly select it, but the
> > config advice does not make that very clear. Could that new config get
> > an explicit "default n" line?

... I see now that a 'default' isn't necessary; the config only appears
for arches which explicitly select it, as intended.

-Kamal