* Re: [PATCH 2/3] ARM: exynos: Simplify code in Exynos3250 CPU core restart path [not found] ` <CAJKOXPeO7uS0mLejYo-oBEpMvBd0338LDogRk=hnU61rdUXCQw@mail.gmail.com> @ 2020-10-26 21:43 ` Marek Szyprowski 2020-10-27 10:39 ` Krzysztof Kozlowski 0 siblings, 1 reply; 2+ messages in thread From: Marek Szyprowski @ 2020-10-26 21:43 UTC (permalink / raw) To: Krzysztof Kozlowski; +Cc: linux-samsung-soc, Bartlomiej Zolnierkiewicz Hi Krzysztof, On 22.03.2019 14:10, Krzysztof Kozlowski wrote: > On Fri, 22 Mar 2019 at 12:48, Marek Szyprowski <m.szyprowski@samsung.com> wrote: >> exynos_core_restart() is called by secondary CPU boot procedure, used by >> CPU hotplug. Replace of_machine_is_compatible() call with a simple SoC >> revision check. >> >> of_machine_is_compatible() function performs a dozen of string comparisons >> during the full device tree walk, while soc_is_exynos3250() is a simple >> integer check on SoC revision variable. > Yes but it is against the effort of getting rid of all soc_is_() (see > https://patchwork.kernel.org/project/linux-samsung-soc/list/?series=43565&state=* > ). It also makes this code sticky to mach - we cannot move it to > drivers. See also Arnd's opinion: > https://marc.info/?l=devicetree&m=139291569126848&w=2 > > Did you measure the practical performance impact of this change? No, I didn't measure the performance, but recently I noticed the following warning: ============================= WARNING: suspicious RCU usage 5.10.0-rc1-00001-g6f65599d1f4f-dirty #1800 Not tainted ----------------------------- ./include/trace/events/lock.h:37 suspicious rcu_dereference_check() usage! other info that might help us debug this: rcu_scheduler_active = 2, debug_locks = 1 RCU used illegally from extended quiescent state! no locks held by swapper/0/0. stack backtrace: CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.10.0-rc1-00001-g6f65599d1f4f-dirty #1800 Hardware name: Samsung Exynos (Flattened Device Tree) [<c0111514>] (unwind_backtrace) from [<c010ceb8>] (show_stack+0x10/0x14) [<c010ceb8>] (show_stack) from [<c0b1d8dc>] (dump_stack+0xb4/0xd4) [<c0b1d8dc>] (dump_stack) from [<c0194acc>] (lock_acquire+0x418/0x584) [<c0194acc>] (lock_acquire) from [<c0b29e58>] (_raw_spin_lock_irqsave+0x4c/0x60) [<c0b29e58>] (_raw_spin_lock_irqsave) from [<c0897af4>] (of_device_is_compatible+0x1c/0x4c) [<c0897af4>] (of_device_is_compatible) from [<c01216d8>] (exynos_core_restart+0x14/0xb0) [<c01216d8>] (exynos_core_restart) from [<c0120a78>] (exynos_cpu0_enter_aftr+0x1d0/0x1dc) [<c0120a78>] (exynos_cpu0_enter_aftr) from [<c08575b0>] (exynos_enter_coupled_lowpower+0x44/0x74) [<c08575b0>] (exynos_enter_coupled_lowpower) from [<c085477c>] (cpuidle_enter_state+0x178/0x660) [<c085477c>] (cpuidle_enter_state) from [<c08572dc>] (cpuidle_enter_state_coupled+0x35c/0x378) [<c08572dc>] (cpuidle_enter_state_coupled) from [<c0854cc8>] (cpuidle_enter+0x50/0x54) [<c0854cc8>] (cpuidle_enter) from [<c0164854>] (do_idle+0x224/0x2a4) [<c0164854>] (do_idle) from [<c0164c88>] (cpu_startup_entry+0x18/0x1c) [<c0164c88>] (cpu_startup_entry) from [<c1100fa0>] (start_kernel+0x640/0x67c) [<c1100fa0>] (start_kernel) from [<00000000>] (0x0) I will add this to the commit message and resend. This looks like a good reason for this change. >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >> --- >> arch/arm/mach-exynos/platsmp.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c >> index 65da13923b8f..d5d48fbdab17 100644 >> --- a/arch/arm/mach-exynos/platsmp.c >> +++ b/arch/arm/mach-exynos/platsmp.c >> @@ -216,7 +216,7 @@ void exynos_core_restart(u32 core_id) >> { >> u32 val; >> >> - if (!of_machine_is_compatible("samsung,exynos3250")) >> + if (!soc_is_exynos3250()) >> return; >> >> while (!pmu_raw_readl(S5P_PMU_SPARE2)) >> -- >> 2.17.1 >> > Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH 2/3] ARM: exynos: Simplify code in Exynos3250 CPU core restart path 2020-10-26 21:43 ` [PATCH 2/3] ARM: exynos: Simplify code in Exynos3250 CPU core restart path Marek Szyprowski @ 2020-10-27 10:39 ` Krzysztof Kozlowski 0 siblings, 0 replies; 2+ messages in thread From: Krzysztof Kozlowski @ 2020-10-27 10:39 UTC (permalink / raw) To: Marek Szyprowski; +Cc: linux-samsung-soc, Bartlomiej Zolnierkiewicz On Mon, Oct 26, 2020 at 10:43:03PM +0100, Marek Szyprowski wrote: > Hi Krzysztof, > > On 22.03.2019 14:10, Krzysztof Kozlowski wrote: > > On Fri, 22 Mar 2019 at 12:48, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > >> exynos_core_restart() is called by secondary CPU boot procedure, used by > >> CPU hotplug. Replace of_machine_is_compatible() call with a simple SoC > >> revision check. > >> > >> of_machine_is_compatible() function performs a dozen of string comparisons > >> during the full device tree walk, while soc_is_exynos3250() is a simple > >> integer check on SoC revision variable. > > Yes but it is against the effort of getting rid of all soc_is_() (see > > https://patchwork.kernel.org/project/linux-samsung-soc/list/?series=43565&state=* > > ). It also makes this code sticky to mach - we cannot move it to > > drivers. See also Arnd's opinion: > > https://marc.info/?l=devicetree&m=139291569126848&w=2 > > > > Did you measure the practical performance impact of this change? > > No, I didn't measure the performance, but recently I noticed the > following warning: > > ============================= > WARNING: suspicious RCU usage > 5.10.0-rc1-00001-g6f65599d1f4f-dirty #1800 Not tainted > ----------------------------- > ./include/trace/events/lock.h:37 suspicious rcu_dereference_check() usage! > > other info that might help us debug this: > > > rcu_scheduler_active = 2, debug_locks = 1 > RCU used illegally from extended quiescent state! > no locks held by swapper/0/0. > > stack backtrace: > CPU: 0 PID: 0 Comm: swapper/0 Not tainted > 5.10.0-rc1-00001-g6f65599d1f4f-dirty #1800 > Hardware name: Samsung Exynos (Flattened Device Tree) > [<c0111514>] (unwind_backtrace) from [<c010ceb8>] (show_stack+0x10/0x14) > [<c010ceb8>] (show_stack) from [<c0b1d8dc>] (dump_stack+0xb4/0xd4) > [<c0b1d8dc>] (dump_stack) from [<c0194acc>] (lock_acquire+0x418/0x584) > [<c0194acc>] (lock_acquire) from [<c0b29e58>] > (_raw_spin_lock_irqsave+0x4c/0x60) > [<c0b29e58>] (_raw_spin_lock_irqsave) from [<c0897af4>] > (of_device_is_compatible+0x1c/0x4c) > [<c0897af4>] (of_device_is_compatible) from [<c01216d8>] > (exynos_core_restart+0x14/0xb0) > [<c01216d8>] (exynos_core_restart) from [<c0120a78>] > (exynos_cpu0_enter_aftr+0x1d0/0x1dc) > [<c0120a78>] (exynos_cpu0_enter_aftr) from [<c08575b0>] > (exynos_enter_coupled_lowpower+0x44/0x74) > [<c08575b0>] (exynos_enter_coupled_lowpower) from [<c085477c>] > (cpuidle_enter_state+0x178/0x660) > [<c085477c>] (cpuidle_enter_state) from [<c08572dc>] > (cpuidle_enter_state_coupled+0x35c/0x378) > [<c08572dc>] (cpuidle_enter_state_coupled) from [<c0854cc8>] > (cpuidle_enter+0x50/0x54) > [<c0854cc8>] (cpuidle_enter) from [<c0164854>] (do_idle+0x224/0x2a4) > [<c0164854>] (do_idle) from [<c0164c88>] (cpu_startup_entry+0x18/0x1c) > [<c0164c88>] (cpu_startup_entry) from [<c1100fa0>] > (start_kernel+0x640/0x67c) > [<c1100fa0>] (start_kernel) from [<00000000>] (0x0) > > I will add this to the commit message and resend. This looks like a good > reason for this change. Good point, thanks. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-10-27 10:40 UTC | newest] Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20190322114844eucas1p16095d6c26d9b6ef23c1168ce70194255@eucas1p1.samsung.com> [not found] ` <20190322114833.12686-1-m.szyprowski@samsung.com> [not found] ` <20190322114833.12686-3-m.szyprowski@samsung.com> [not found] ` <CAJKOXPeO7uS0mLejYo-oBEpMvBd0338LDogRk=hnU61rdUXCQw@mail.gmail.com> 2020-10-26 21:43 ` [PATCH 2/3] ARM: exynos: Simplify code in Exynos3250 CPU core restart path Marek Szyprowski 2020-10-27 10:39 ` Krzysztof Kozlowski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).