mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Aboorva Devarajan <aboorvad@linux.ibm.com>
To: Shrikanth Hegde <sshegde@linux.ibm.com>,
	Madhavan Srinivasan <maddy@linux.ibm.com>,
	linuxppc-dev@lists.ozlabs.org
Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>,
	Christophe Leroy <chleroy@kernel.org>,
	linux-kernel@vger.kernel.org,
	Sourabh Jain <sourabhjain@linux.ibm.com>,
	Ritesh Harjani <ritesh.list@gmail.com>
Subject: Re: [PATCH 2/3] powerpc/powernv: fix preempt count leak in pnv_kexec_wait_secondaries_down
Date: Wed, 03 Jun 2026 11:38:44 +0530	[thread overview]
Message-ID: <953a5a29dc71c5f3067f977b48c6da1ba36d58ac.camel@linux.ibm.com> (raw)
In-Reply-To: <99da8dfa-2624-44fb-96c5-ff7eddf717ac@linux.ibm.com>

On Mon, 2026-05-18 at 13:26 +0530, Shrikanth Hegde wrote:
> 
> Hi Aboorva.
> 
> On 5/18/26 10:38 AM, Aboorva Devarajan wrote:
> > pnv_kexec_wait_secondaries_down() calls get_cpu() to obtain the current
> > CPU id but never calls the matching put_cpu(), leaking one
> > preempt_disable() nesting level on every invocation.
> > 
> > In practice the imbalance does not trigger a visible splat because the
> > kexec teardown path is a one-way trip: IRQs are already disabled, no
> > schedule() occurs after the leak, and default_machine_kexec() overwrites
> > preempt_count with HARDIRQ_OFFSET before jumping into kexec_sequence()
> > which never returns. However the bookkeeping is still wrong.
> > 
> > In the kexec teardown path IRQs are already disabled and the CPU is
> > pinned, so get_cpu()'s preempt_disable() side-effect is unnecessary.
> > Replace get_cpu() with raw_smp_processor_id() which returns the CPU id
> > without touching preempt_count.
> > 
> > Fixes: 298b34d7d578 ("powerpc/powernv: Fix kexec races going back to OPAL")
> > Signed-off-by: Aboorva Devarajan <aboorvad@linux.ibm.com>
> > ---
> >   arch/powerpc/platforms/powernv/setup.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
> > index 4dbb47ddbdcc4..177da0defcb36 100644
> > --- a/arch/powerpc/platforms/powernv/setup.c
> > +++ b/arch/powerpc/platforms/powernv/setup.c
> > @@ -396,7 +396,7 @@ static void pnv_kexec_wait_secondaries_down(void)
> >   {
> >   	int my_cpu, i, notified = -1;
> >   
> > -	my_cpu = get_cpu();
> > +	my_cpu = raw_smp_processor_id();
> >   
> 
> Is it always with irq-disabled?
> How about !CONFIG_SMP and in kexec_prepare_cpus. I see it disables interrupt later.
> (though it is a less common config)
> 

IIUC, PPC_POWERNV does 'select FORCE_SMP' (-> selects SMP), so there is no
!CONFIG_SMP powernv build. The !SMP kexec_prepare_cpus() variant in
arch/powerpc/kexec/core_64.c, the one you spotted that calls
ppc_md.kexec_cpu_down() before local_irq_disable() is therefore
never compiled with powernv, so pnv_kexec_cpu_down() ->
pnv_kexec_wait_secondaries_down() can't be reached through it.
so, IRQs are disabled in every case that reaches this function.


> So use smp_processor_id()?? One could compile with CONFIG_DEBUG_PREEMPT=y and
> see any reports.
> 
> >   	for_each_online_cpu(i) {
> >   		uint8_t status;

sure, I'll switch to smp_processor_id() in v2 rather than raw_smp_processor_id().
It returns the cpu id without touching preempt_count (so the leak is
gone), and unlike the raw variant it keeps the CONFIG_DEBUG_PREEMPT
check which is a no-op here since IRQs are off, but will flag any
future caller that reaches this path while preemptible instead of
silently hiding it.


Thanks,
Aboorva

  reply	other threads:[~2026-06-03  6:09 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-18  5:08 [PATCH 0/3] powerpc: fix preempt_count imbalances in perf and kexec paths Aboorva Devarajan
2026-05-18  5:08 ` [PATCH 1/3] powerpc/perf: fix preempt count underflow in fsl_emb_pmu_del Aboorva Devarajan
2026-05-18  6:13   ` Shrikanth Hegde
2026-06-03  5:59     ` Aboorva Devarajan
2026-05-18  5:08 ` [PATCH 2/3] powerpc/powernv: fix preempt count leak in pnv_kexec_wait_secondaries_down Aboorva Devarajan
2026-05-18  7:56   ` Shrikanth Hegde
2026-06-03  6:08     ` Aboorva Devarajan [this message]
2026-05-18  5:08 ` [PATCH 3/3] powerpc/kexec: fix double get_cpu() imbalance in kexec_prepare_cpus Aboorva Devarajan
2026-05-18  6:02   ` Shrikanth Hegde
2026-06-03  6:14     ` Aboorva Devarajan
2026-06-03  6:16       ` Shrikanth Hegde
2026-05-18  8:08 ` [PATCH 0/3] powerpc: fix preempt_count imbalances in perf and kexec paths Shrikanth Hegde
2026-06-03  6:16   ` Aboorva Devarajan
2026-06-03  6:27 [PATCH v2 " Aboorva Devarajan
2026-06-03  6:27 ` [PATCH 2/3] powerpc/powernv: fix preempt count leak in pnv_kexec_wait_secondaries_down Aboorva Devarajan

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=953a5a29dc71c5f3067f977b48c6da1ba36d58ac.camel@linux.ibm.com \
    --to=aboorvad@linux.ibm.com \
    --cc=atrajeev@linux.vnet.ibm.com \
    --cc=chleroy@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maddy@linux.ibm.com \
    --cc=ritesh.list@gmail.com \
    --cc=sourabhjain@linux.ibm.com \
    --cc=sshegde@linux.ibm.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®