Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1670086 > unrolled thread

[patch 32/55] x86/irq: Restructure fixup_irqs()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [patch 32/55] x86/irq: Restructure fixup_irqs() Thomas Gleixner <tglx@linutronix.de> - 2017-06-20 02:10 +0200
    Re: [patch 32/55] x86/irq: Restructure fixup_irqs() Thomas Gleixner <tglx@linutronix.de> - 2017-06-20 23:30 +0200
    Re: [patch 32/55] x86/irq: Restructure fixup_irqs() Keith Busch <keith.busch@intel.com> - 2017-06-20 23:30 +0200
    [tip:irq/core] x86/irq: Restructure fixup_irqs() tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2017-06-22 19:10 +0200

#1670086 — [patch 32/55] x86/irq: Restructure fixup_irqs()

Reorder fixup_irqs() so it matches the flow in the generic migration code.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/kernel/irq.c |   46 ++++++++++++++++++++--------------------------
 1 file changed, 20 insertions(+), 26 deletions(-)

--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -433,7 +433,6 @@ int check_irq_vectors_for_cpu_disable(vo
 void fixup_irqs(void)
 {
 	unsigned int irq, vector;
-	static int warned;
 	struct irq_desc *desc;
 	struct irq_data *data;
 	struct irq_chip *chip;
@@ -441,18 +440,27 @@ void fixup_irqs(void)
 
 	for_each_irq_desc(irq, desc) {
 		const struct cpumask *affinity;
-		int break_affinity = 0;
-		int set_affinity = 1;
+		bool break_affinity = false;
 
 		if (!desc)
 			continue;
-		if (irq == 2)
-			continue;
 
 		/* interrupt's are disabled at this point */
 		raw_spin_lock(&desc->lock);
 
 		data = irq_desc_get_irq_data(desc);
+		chip = irq_data_get_irq_chip(data);
+		/*
+		 * The interrupt descriptor might have been cleaned up
+		 * already, but it is not yet removed from the radix
+		 * tree. If the chip does not have an affinity setter,
+		 * nothing to do here.
+		 */
+		if (!chip !chip->irq_set_affinity) {
+			raw_spin_unlock(&desc->lock);
+			continue;
+		}
+
 		affinity = irq_data_get_affinity_mask(data);
 
 		if (!irq_has_action(irq) || irqd_is_per_cpu(data) ||
@@ -485,30 +493,18 @@ void fixup_irqs(void)
 		 * affinity and use cpu_online_mask as fall back.
 		 */
 		if (cpumask_any_and(affinity, cpu_online_mask) >= nr_cpu_ids) {
-			break_affinity = 1;
+			broke_affinity = true;
 			affinity = cpu_online_mask;
 		}
 
-		chip = irq_data_get_irq_chip(data);
-		/*
-		 * The interrupt descriptor might have been cleaned up
-		 * already, but it is not yet removed from the radix tree
-		 */
-		if (!chip) {
-			raw_spin_unlock(&desc->lock);
-			continue;
-		}
-
 		if (!irqd_can_move_in_process_context(data) && chip->irq_mask)
 			chip->irq_mask(data);
 
-		if (chip->irq_set_affinity) {
-			ret = chip->irq_set_affinity(data, affinity, true);
-			if (ret == -ENOSPC)
-				pr_crit("IRQ %d set affinity failed because there are no available vectors.  The device assigned to this IRQ is unstable.\n", irq);
-		} else {
-			if (!(warned++))
-				set_affinity = 0;
+		ret = chip->irq_set_affinity(data, affinity, true);
+		if (ret) {
+			pr_crit("IRQ %u: Force affinity failed (%d)\n",
+				d->irq, ret);
+			broke_affinity = false;
 		}
 
 		/*
@@ -522,10 +518,8 @@ void fixup_irqs(void)
 
 		raw_spin_unlock(&desc->lock);
 
-		if (break_affinity && set_affinity)
+		if (broke_affinity)
 			pr_notice("Broke affinity for irq %i\n", irq);
-		else if (!set_affinity)
-			pr_notice("Cannot set affinity for irq %i\n", irq);
 	}
 
 	/*

[toc] | [next] | [standalone]

#1671145

On Tue, 20 Jun 2017, Keith Busch wrote:
> On Tue, Jun 20, 2017 at 01:37:32AM +0200, Thomas Gleixner wrote:
> > @@ -441,18 +440,27 @@ void fixup_irqs(void)
> >  
> >  	for_each_irq_desc(irq, desc) {
> >  		const struct cpumask *affinity;
> > -		int break_affinity = 0;
> > -		int set_affinity = 1;
> > +		bool break_affinity = false;
> >  
> >  		if (!desc)
> >  			continue;
> > -		if (irq == 2)
> > -			continue;
> >  
> >  		/* interrupt's are disabled at this point */
> >  		raw_spin_lock(&desc->lock);
> >  
> >  		data = irq_desc_get_irq_data(desc);
> > +		chip = irq_data_get_irq_chip(data);
> > +		/*
> > +		 * The interrupt descriptor might have been cleaned up
> > +		 * already, but it is not yet removed from the radix
> > +		 * tree. If the chip does not have an affinity setter,
> > +		 * nothing to do here.
> > +		 */
> > +		if (!chip !chip->irq_set_affinity) {
> > +			raw_spin_unlock(&desc->lock);
> > +			continue;
> > +		}
> 
> A bit of a moot point since the very next patch deletes all of this,
> but found this broken 'if' condition when compiling one at a time,
> missing the '&&'.

Hmm, How did I fatfinger that one after booting it?

Yes, the patch is kinda moot, but I wanted to verify that shifting the
logic around does not break any of the hotplug stress tests, so that the
next step becomes less risky.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1671149

On Tue, Jun 20, 2017 at 01:37:32AM +0200, Thomas Gleixner wrote:
> @@ -441,18 +440,27 @@ void fixup_irqs(void)
>  
>  	for_each_irq_desc(irq, desc) {
>  		const struct cpumask *affinity;
> -		int break_affinity = 0;
> -		int set_affinity = 1;
> +		bool break_affinity = false;
>  
>  		if (!desc)
>  			continue;
> -		if (irq == 2)
> -			continue;
>  
>  		/* interrupt's are disabled at this point */
>  		raw_spin_lock(&desc->lock);
>  
>  		data = irq_desc_get_irq_data(desc);
> +		chip = irq_data_get_irq_chip(data);
> +		/*
> +		 * The interrupt descriptor might have been cleaned up
> +		 * already, but it is not yet removed from the radix
> +		 * tree. If the chip does not have an affinity setter,
> +		 * nothing to do here.
> +		 */
> +		if (!chip !chip->irq_set_affinity) {
> +			raw_spin_unlock(&desc->lock);
> +			continue;
> +		}

A bit of a moot point since the very next patch deletes all of this,
but found this broken 'if' condition when compiling one at a time,
missing the '&&'.

[toc] | [prev] | [next] | [standalone]

#1672859 — [tip:irq/core] x86/irq: Restructure fixup_irqs()

Commit-ID:  654abd0a7baf144998147787121da0f9422dafc8
Gitweb:     http://git.kernel.org/tip/654abd0a7baf144998147787121da0f9422dafc8
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 20 Jun 2017 01:37:32 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 22 Jun 2017 18:21:18 +0200

x86/irq: Restructure fixup_irqs()

Reorder fixup_irqs() so it matches the flow in the generic migration code.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Keith Busch <keith.busch@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Christoph Hellwig <hch@lst.de>
Link: http://lkml.kernel.org/r/20170619235445.774272454@linutronix.de

---
 arch/x86/kernel/irq.c | 46 ++++++++++++++++++++--------------------------
 1 file changed, 20 insertions(+), 26 deletions(-)

diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
index 9696007d..78bd2b8 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -433,7 +433,6 @@ int check_irq_vectors_for_cpu_disable(void)
 void fixup_irqs(void)
 {
 	unsigned int irq, vector;
-	static int warned;
 	struct irq_desc *desc;
 	struct irq_data *data;
 	struct irq_chip *chip;
@@ -441,18 +440,27 @@ void fixup_irqs(void)
 
 	for_each_irq_desc(irq, desc) {
 		const struct cpumask *affinity;
-		int break_affinity = 0;
-		int set_affinity = 1;
+		bool break_affinity = false;
 
 		if (!desc)
 			continue;
-		if (irq == 2)
-			continue;
 
 		/* interrupt's are disabled at this point */
 		raw_spin_lock(&desc->lock);
 
 		data = irq_desc_get_irq_data(desc);
+		chip = irq_data_get_irq_chip(data);
+		/*
+		 * The interrupt descriptor might have been cleaned up
+		 * already, but it is not yet removed from the radix
+		 * tree. If the chip does not have an affinity setter,
+		 * nothing to do here.
+		 */
+		if (!chip && !chip->irq_set_affinity) {
+			raw_spin_unlock(&desc->lock);
+			continue;
+		}
+
 		affinity = irq_data_get_affinity_mask(data);
 
 		if (!irq_has_action(irq) || irqd_is_per_cpu(data) ||
@@ -485,30 +493,18 @@ void fixup_irqs(void)
 		 * affinity and use cpu_online_mask as fall back.
 		 */
 		if (cpumask_any_and(affinity, cpu_online_mask) >= nr_cpu_ids) {
-			break_affinity = 1;
+			broke_affinity = true;
 			affinity = cpu_online_mask;
 		}
 
-		chip = irq_data_get_irq_chip(data);
-		/*
-		 * The interrupt descriptor might have been cleaned up
-		 * already, but it is not yet removed from the radix tree
-		 */
-		if (!chip) {
-			raw_spin_unlock(&desc->lock);
-			continue;
-		}
-
 		if (!irqd_can_move_in_process_context(data) && chip->irq_mask)
 			chip->irq_mask(data);
 
-		if (chip->irq_set_affinity) {
-			ret = chip->irq_set_affinity(data, affinity, true);
-			if (ret == -ENOSPC)
-				pr_crit("IRQ %d set affinity failed because there are no available vectors.  The device assigned to this IRQ is unstable.\n", irq);
-		} else {
-			if (!(warned++))
-				set_affinity = 0;
+		ret = chip->irq_set_affinity(data, affinity, true);
+		if (ret) {
+			pr_crit("IRQ %u: Force affinity failed (%d)\n",
+				d->irq, ret);
+			broke_affinity = false;
 		}
 
 		/*
@@ -522,10 +518,8 @@ void fixup_irqs(void)
 
 		raw_spin_unlock(&desc->lock);
 
-		if (break_affinity && set_affinity)
+		if (broke_affinity)
 			pr_notice("Broke affinity for irq %i\n", irq);
-		else if (!set_affinity)
-			pr_notice("Cannot set affinity for irq %i\n", irq);
 	}
 
 	/*

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web