All of lore.kernel.org
 help / color / mirror / Atom feed
From: pheragu@codeaurora.org
To: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org, ckadabi@codeaurora.org,
	tsoni@codeaurora.org, bryanh@codeaurora.org,
	Prasad Sodagudi <psodagud@codeaurora.org>
Subject: Re: [PATCH] genirq: Avoid race between cpu hot plug and irq_desc() allocation paths
Date: Wed, 05 Sep 2018 16:27:09 -0700	[thread overview]
Message-ID: <819e8a811ebf49366d75676922903368@codeaurora.org> (raw)
In-Reply-To: <alpine.DEB.2.21.1809052022110.1416@nanos.tec.linutronix.de>

On 2018-09-05 11:23, Thomas Gleixner wrote:
> On Wed, 5 Sep 2018, Prakruthi Deepak Heragu wrote:
> 
>> One of the cores might have just allocated irq_desc() and other core
>> might be doing irq migration in the hot plug path. In the hot plug 
>> path
>> during the IRQ migration, for_each_active_irq macro is trying to get
>> irqs whose bits are set in allocated_irqs bit map but there is no 
>> return
>> value check after irq_to_desc for desc validity.
> 
> Confused. All parts involved, irq allocation/deallocation and the CPU
> hotplug code take sparse_irq_lock to prevent exavtly that.
> 
Removing the NULL pointer check and adding this sparse_irq_lock
that you suggested will solve this issue. The code looks like
this now. Is this okay?
diff --git a/kernel/irq/cpuhotplug.c b/kernel/irq/cpuhotplug.c
index 9409b55..f2ef76e 100644
--- a/kernel/irq/cpuhotplug.c
+++ b/kernel/irq/cpuhotplug.c
@@ -185,14 +185,10 @@ void irq_migrate_all_off_this_cpu(void)
  {
  	struct irq_desc *desc;
  	unsigned int irq;
-
+	irq_lock_sparse();
  	for_each_active_irq(irq) {
  		bool affinity_broken;
-
  		desc = irq_to_desc(irq);
-		if (!desc)
-			continue;
-
  		raw_spin_lock(&desc->lock);
  		affinity_broken = migrate_one_irq(desc);
  		raw_spin_unlock(&desc->lock);
@@ -202,6 +198,7 @@ void irq_migrate_all_off_this_cpu(void)
  					    irq, smp_processor_id());
  		}
  	}
+	irq_unlock_sparse();
  }

  static void irq_restore_affinity_of_irq(struct irq_desc *desc, unsigned 
int cpu)

> Thanks,
> 
> 	tglx

Thanks,
Prakruthi Deepak Heragu

  reply	other threads:[~2018-09-05 23:27 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-05 17:05 [PATCH] genirq: Avoid race between cpu hot plug and irq_desc() allocation paths Prakruthi Deepak Heragu
2018-09-05 18:23 ` Thomas Gleixner
2018-09-05 23:27   ` pheragu [this message]
2018-09-06  7:56     ` Thomas Gleixner

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=819e8a811ebf49366d75676922903368@codeaurora.org \
    --to=pheragu@codeaurora.org \
    --cc=bryanh@codeaurora.org \
    --cc=ckadabi@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=psodagud@codeaurora.org \
    --cc=tglx@linutronix.de \
    --cc=tsoni@codeaurora.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.