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

Groups > linux.kernel > #1670102 > unrolled thread

[patch 19/55] genirq: Provide irq_fixup_move_pending()

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 19/55] genirq: Provide irq_fixup_move_pending() Thomas Gleixner <tglx@linutronix.de> - 2017-06-20 02:20 +0200
    Re: [patch 19/55] genirq: Provide irq_fixup_move_pending() Dou Liyang <douly.fnst@cn.fujitsu.com> - 2017-06-20 06:30 +0200
      Re: [patch 19/55] genirq: Provide irq_fixup_move_pending() Thomas Gleixner <tglx@linutronix.de> - 2017-06-20 09:00 +0200
    [tip:irq/core] genirq: Provide irq_fixup_move_pending() tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2017-06-22 19:00 +0200

#1670102 — [patch 19/55] genirq: Provide irq_fixup_move_pending()

If an CPU goes offline, the interrupts are migrated away, but a eventually
pending interrupt move, which has not yet been made effective is kept
pending even if the outgoing CPU is the sole target of the pending affinity
mask. What's worse is, that the pending affinity mask is discarded even if
it would contain a valid subset of the online CPUs.

Implement a helper function which allows to avoid these issues.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/irq.h    |    5 +++++
 kernel/irq/migration.c |   30 ++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -490,9 +490,14 @@ extern void irq_migrate_all_off_this_cpu
 #if defined(CONFIG_SMP) && defined(CONFIG_GENERIC_PENDING_IRQ)
 void irq_move_irq(struct irq_data *data);
 void irq_move_masked_irq(struct irq_data *data);
+bool irq_fixup_move_pending(struct irq_desc *desc, bool force_clear);
 #else
 static inline void irq_move_irq(struct irq_data *data) { }
 static inline void irq_move_masked_irq(struct irq_data *data) { }
+static inline bool irq_fixup_move_pending(struct irq_desc *desc, bool fclear)
+{
+	return false;
+}
 #endif
 
 extern int no_irq_affinity;
--- a/kernel/irq/migration.c
+++ b/kernel/irq/migration.c
@@ -4,6 +4,36 @@
 
 #include "internals.h"
 
+/**
+ * irq_fixup_move_pending - Cleanup irq move pending from a dying CPU
+ * @desc:		Interrupt descpriptor to clean up
+ * @force_clear:	If set clear the move pending bit unconditionally.
+ *			If not set, clear it only when the dying CPU is the
+ *			last one in the pending mask.
+ *
+ * Returns true if the pending bit was set and the pending mask contains an
+ * online CPU other than the dying CPU.
+ */
+bool irq_fixup_move_pending(struct irq_desc *desc, bool force_clear)
+{
+	struct irq_data *data = irq_desc_get_irq_data(desc);
+
+	if (!irqd_is_setaffinity_pending(data))
+		return false;
+
+	/*
+	 * The outgoing CPU might be the last online target in a pending
+	 * interrupt move. If that's the case clear the pending move bit.
+	 */
+	if (cpumask_any_and(desc->pending_mask, cpu_online_mask) > nr_cpu_ids) {
+		irqd_clr_move_pending(data);
+		return false;
+	}
+	if (force_clear)
+		irqd_clr_move_pending(data);
+	return true;
+}
+
 void irq_move_masked_irq(struct irq_data *idata)
 {
 	struct irq_desc *desc = irq_data_to_desc(idata);

[toc] | [next] | [standalone]

#1670253

Hi Thomas,

At 06/20/2017 07:37 AM, Thomas Gleixner wrote:
[...]
>
> +/**
> + * irq_fixup_move_pending - Cleanup irq move pending from a dying CPU
> + * @desc:		Interrupt descpriptor to clean up
> + * @force_clear:	If set clear the move pending bit unconditionally.
> + *			If not set, clear it only when the dying CPU is the
> + *			last one in the pending mask.
> + *
> + * Returns true if the pending bit was set and the pending mask contains an
> + * online CPU other than the dying CPU.
> + */
> +bool irq_fixup_move_pending(struct irq_desc *desc, bool force_clear)
> +{
> +	struct irq_data *data = irq_desc_get_irq_data(desc);
> +
> +	if (!irqd_is_setaffinity_pending(data))
> +		return false;
> +
> +	/*
> +	 * The outgoing CPU might be the last online target in a pending
> +	 * interrupt move. If that's the case clear the pending move bit.
> +	 */
> +	if (cpumask_any_and(desc->pending_mask, cpu_online_mask) > nr_cpu_ids) {

Should we consider the case of "=nr_cpu_ids" here, like:

cpumask_any_and(desc->pending_mask, cpu_online_mask) >= nr_cpu_ids


Thanks.
	Dou

> +		irqd_clr_move_pending(data);
> +		return false;
> +	}
> +	if (force_clear)
> +		irqd_clr_move_pending(data);
> +	return true;
> +}
> +
>  void irq_move_masked_irq(struct irq_data *idata)
>  {
>  	struct irq_desc *desc = irq_data_to_desc(idata);

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

#1670341

On Tue, 20 Jun 2017, Dou Liyang wrote:
> At 06/20/2017 07:37 AM, Thomas Gleixner wrote:
> [...]
> > 
> > +/**
> > + * irq_fixup_move_pending - Cleanup irq move pending from a dying CPU
> > + * @desc:		Interrupt descpriptor to clean up
> > + * @force_clear:	If set clear the move pending bit unconditionally.
> > + *			If not set, clear it only when the dying CPU is the
> > + *			last one in the pending mask.
> > + *
> > + * Returns true if the pending bit was set and the pending mask contains an
> > + * online CPU other than the dying CPU.
> > + */
> > +bool irq_fixup_move_pending(struct irq_desc *desc, bool force_clear)
> > +{
> > +	struct irq_data *data = irq_desc_get_irq_data(desc);
> > +
> > +	if (!irqd_is_setaffinity_pending(data))
> > +		return false;
> > +
> > +	/*
> > +	 * The outgoing CPU might be the last online target in a pending
> > +	 * interrupt move. If that's the case clear the pending move bit.
> > +	 */
> > +	if (cpumask_any_and(desc->pending_mask, cpu_online_mask) > nr_cpu_ids)
> > {
> 
> Should we consider the case of "=nr_cpu_ids" here, like:
> 
> cpumask_any_and(desc->pending_mask, cpu_online_mask) >= nr_cpu_ids

Yes, indeed. > is wrong. Good catch!

Thanks,

	tglx

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

#1672835 — [tip:irq/core] genirq: Provide irq_fixup_move_pending()

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

genirq: Provide irq_fixup_move_pending()

If an CPU goes offline, the interrupts are migrated away, but a eventually
pending interrupt move, which has not yet been made effective is kept
pending even if the outgoing CPU is the sole target of the pending affinity
mask. What's worse is, that the pending affinity mask is discarded even if
it would contain a valid subset of the online CPUs.

Implement a helper function which allows to avoid these issues.

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/20170619235444.691345468@linutronix.de

---
 include/linux/irq.h    |  5 +++++
 kernel/irq/migration.c | 30 ++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 7e62e10..d008065 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -491,9 +491,14 @@ extern void irq_migrate_all_off_this_cpu(void);
 #if defined(CONFIG_SMP) && defined(CONFIG_GENERIC_PENDING_IRQ)
 void irq_move_irq(struct irq_data *data);
 void irq_move_masked_irq(struct irq_data *data);
+bool irq_fixup_move_pending(struct irq_desc *desc, bool force_clear);
 #else
 static inline void irq_move_irq(struct irq_data *data) { }
 static inline void irq_move_masked_irq(struct irq_data *data) { }
+static inline bool irq_fixup_move_pending(struct irq_desc *desc, bool fclear)
+{
+	return false;
+}
 #endif
 
 extern int no_irq_affinity;
diff --git a/kernel/irq/migration.c b/kernel/irq/migration.c
index 37ddb7b..6ca054a 100644
--- a/kernel/irq/migration.c
+++ b/kernel/irq/migration.c
@@ -4,6 +4,36 @@
 
 #include "internals.h"
 
+/**
+ * irq_fixup_move_pending - Cleanup irq move pending from a dying CPU
+ * @desc:		Interrupt descpriptor to clean up
+ * @force_clear:	If set clear the move pending bit unconditionally.
+ *			If not set, clear it only when the dying CPU is the
+ *			last one in the pending mask.
+ *
+ * Returns true if the pending bit was set and the pending mask contains an
+ * online CPU other than the dying CPU.
+ */
+bool irq_fixup_move_pending(struct irq_desc *desc, bool force_clear)
+{
+	struct irq_data *data = irq_desc_get_irq_data(desc);
+
+	if (!irqd_is_setaffinity_pending(data))
+		return false;
+
+	/*
+	 * The outgoing CPU might be the last online target in a pending
+	 * interrupt move. If that's the case clear the pending move bit.
+	 */
+	if (cpumask_any_and(desc->pending_mask, cpu_online_mask) >= nr_cpu_ids) {
+		irqd_clr_move_pending(data);
+		return false;
+	}
+	if (force_clear)
+		irqd_clr_move_pending(data);
+	return true;
+}
+
 void irq_move_masked_irq(struct irq_data *idata)
 {
 	struct irq_desc *desc = irq_data_to_desc(idata);

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web