Messages in this thread | | | Date | Thu, 16 Feb 2023 09:51:42 +0100 | Subject | Re: [PATCH v3] firmware: arm_sdei: Fix sleep from invalid context BUG | From | Pierre Gondois <> |
| |
On 2/15/23 19:13, James Morse wrote: > Hi Pierre, Will, > > On 15/02/2023 16:09, Pierre Gondois wrote: >> Running a preemp_rt kernel based on vv6.2-rc3-rt1 based kernel on an >> Ampere Altra triggers: >> BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:46 >> in_atomic(): 0, irqs_disabled(): 128, non_block: 0, pid: 24, name: cpuhp/0 >> preempt_count: 0, expected: 0 >> RCU nest depth: 0, expected: 0 >> 3 locks held by cpuhp/0/24: >> #0: ffffda30217c70d0 (cpu_hotplug_lock){++++}-{0:0}, at: cpuhp_thread_fun+0x5c/0x248 >> #1: ffffda30217c7120 (cpuhp_state-up){+.+.}-{0:0}, at: cpuhp_thread_fun+0x5c/0x248 >> #2: ffffda3021c711f0 (sdei_list_lock){....}-{3:3}, at: sdei_cpuhp_up+0x3c/0x130 >> irq event stamp: 36 >> hardirqs last enabled at (35): [<ffffda301e85b7bc>] finish_task_switch+0xb4/0x2b0 >> hardirqs last disabled at (36): [<ffffda301e812fec>] cpuhp_thread_fun+0x21c/0x248 >> softirqs last enabled at (0): [<ffffda301e80b184>] copy_process+0x63c/0x1ac0 >> softirqs last disabled at (0): [<0000000000000000>] 0x0 >> CPU: 0 PID: 24 Comm: cpuhp/0 Not tainted 5.19.0-rc3-rt5-[...] >> Hardware name: WIWYNN Mt.Jade Server [...] >> Call trace: >> dump_backtrace+0x114/0x120 >> show_stack+0x20/0x70 >> dump_stack_lvl+0x9c/0xd8 >> dump_stack+0x18/0x34 >> __might_resched+0x188/0x228 >> rt_spin_lock+0x70/0x120 >> sdei_cpuhp_up+0x3c/0x130 >> cpuhp_invoke_callback+0x250/0xf08 >> cpuhp_thread_fun+0x120/0x248 >> smpboot_thread_fn+0x280/0x320 >> kthread+0x130/0x140 >> ret_from_fork+0x10/0x20 >> >> sdei_cpuhp_up() is called in the STARTING hotplug section, >> which runs whith interrupts disabled. Use a CPUHP_AP_ONLINE_DYN entry > > Typo: with > >> instead to execute the cpuhp cb later, with preemption enabled. >> >> SDEI originaly got its own cpuhp slot because to allow interracting > > Typo: originally > Typo: interacting > >> with perf. It got superseded by pNMI and this early slot is not >> relevant anymore. [1] > > (Please add a spell checker to your workflow. 'codespell' knows which bits of > a diff it should check) > > "because to allow interacting" isn't easy to parse, "to allow an interaction with"? > > >> Some SDEI calls (e.g. SDEI_1_0_FN_SDEI_PE_MASK) take actions on the >> calling CPU. It is checked that preemption is disabled for them. >> _ONLINE cpuhp cb are executed in the 'per CPU hotplug thread'. >> Preemption is enabled in those threads, but their cpumask is limited >> to 1 CPU. >> Move 'WARN_ON_ONCE(preemptible())' statements so that SDEI cpuhp cb >> don't trigger them. >> >> Also add a check for the SDEI_1_0_FN_SDEI_PRIVATE_RESET SDEI call >> which acts on the calling CPU. > > Thanks for sticking with this, > > Reviewed-by: James Morse <james.morse@arm.com> > > > Will, are you happy to pick this up and fix the typos, or would you prefer a clean version > to be posted?
I re-posted the patch with the typos corrected at: https://lore.kernel.org/all/20230216084920.144064-1-pierre.gondois@arm.com/
sorry for those, Regards, Pierre
> > > Thanks, > > James
| |