2017-06-20 00:00:12

by Thomas Gleixner

[permalink] [raw]
Subject: [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 <[email protected]>
---
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);
}

/*



2017-06-20 21:26:14

by Keith Busch

[permalink] [raw]
Subject: Re: [patch 32/55] x86/irq: Restructure fixup_irqs()

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 '&&'.

2017-06-20 21:28:45

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 32/55] x86/irq: Restructure fixup_irqs()

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





Subject: [tip:irq/core] x86/irq: Restructure fixup_irqs()

Commit-ID: 654abd0a7baf144998147787121da0f9422dafc8
Gitweb: http://git.kernel.org/tip/654abd0a7baf144998147787121da0f9422dafc8
Author: Thomas Gleixner <[email protected]>
AuthorDate: Tue, 20 Jun 2017 01:37:32 +0200
Committer: Thomas Gleixner <[email protected]>
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 <[email protected]>
Cc: Jens Axboe <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Keith Busch <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]

---
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);
}

/*