dri-devel Archive on lore.kernel.org
 help / color / Atom feed
* [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes
@ 2020-12-10 19:25 Thomas Gleixner
  2020-12-10 19:25 ` [patch 01/30] genirq: Move irq_has_action() into core code Thomas Gleixner
                   ` (29 more replies)
  0 siblings, 30 replies; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, linux-pci, Jakub Kicinski,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Juergen Gross,
	David Airlie, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller

A recent request to export kstat_irqs() pointed to a copy of the same in
the i915 code, which made me look for further usage of irq descriptors in
drivers.

The usage in drivers ranges from creative to broken in all colours.

irqdesc.h clearly says that this is core functionality and the fact C does
not allow full encapsulation is not a justification to fiddle with it just
because. It took us a lot of effort to make the core functionality provide
what drivers need.

If there is a shortcoming, it's not asked too much to talk to the relevant
maintainers instead of going off and fiddling with the guts of interrupt
descriptors and often enough without understanding lifetime and locking
rules.

As people insist on not respecting boundaries, this series cleans up the
(ab)use and at the end removes the export of irq_to_desc() to make it at
least harder. All legitimate users of this are built in.

While at it I stumbled over some other oddities related to interrupt
counting and cleaned them up as well.

The series applies on top of

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/core

and is also available from git:

  git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git genirq

Thanks,

	tglx
---
 arch/alpha/kernel/sys_jensen.c                       |    2 
 arch/arm/kernel/smp.c                                |    2 
 arch/parisc/kernel/irq.c                             |    7 
 arch/s390/kernel/irq.c                               |    2 
 arch/x86/kernel/topology.c                           |    1 
 arch/arm64/kernel/smp.c                              |    2 
 drivers/gpu/drm/i915/display/intel_lpe_audio.c       |    4 
 drivers/gpu/drm/i915/i915_irq.c                      |   34 +++
 drivers/gpu/drm/i915/i915_pmu.c                      |   18 -
 drivers/gpu/drm/i915/i915_pmu.h                      |    8 
 drivers/mfd/ab8500-debugfs.c                         |   16 -
 drivers/net/ethernet/mellanox/mlx4/en_cq.c           |    8 
 drivers/net/ethernet/mellanox/mlx4/en_rx.c           |    6 
 drivers/net/ethernet/mellanox/mlx4/mlx4_en.h         |    3 
 drivers/net/ethernet/mellanox/mlx5/core/en.h         |    2 
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c    |    2 
 drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c    |    6 
 drivers/ntb/msi.c                                    |    4 
 drivers/pci/controller/mobiveil/pcie-mobiveil-host.c |    8 
 drivers/pci/controller/pcie-xilinx-nwl.c             |    8 
 drivers/pinctrl/nomadik/pinctrl-nomadik.c            |    3 
 drivers/xen/events/events_base.c                     |  172 +++++++++++--------
 drivers/xen/evtchn.c                                 |   34 ---
 include/linux/interrupt.h                            |    1 
 include/linux/irq.h                                  |    7 
 include/linux/irqdesc.h                              |   40 +---
 include/linux/kernel_stat.h                          |    1 
 kernel/irq/irqdesc.c                                 |   42 ++--
 kernel/irq/manage.c                                  |   37 ++++
 kernel/irq/proc.c                                    |    5 
 30 files changed, 263 insertions(+), 222 deletions(-)


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 01/30] genirq: Move irq_has_action() into core code
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-10 19:25 ` [patch 02/30] genirq: Move status flag checks to core Thomas Gleixner
                   ` (28 subsequent siblings)
  29 siblings, 0 replies; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, linux-pci, Jakub Kicinski,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Juergen Gross,
	David Airlie, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller

This function uses irq_to_desc() and is going to be used by modules to
replace the open coded irq_to_desc() (ab)usage. The final goal is to remove
the export of irq_to_desc() so driver cannot fiddle with it anymore.

Move it into the core code and fixup the usage sites to include the proper
header.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/alpha/kernel/sys_jensen.c |    2 +-
 arch/x86/kernel/topology.c     |    1 +
 include/linux/interrupt.h      |    1 +
 include/linux/irqdesc.h        |    7 +------
 kernel/irq/manage.c            |   17 +++++++++++++++++
 5 files changed, 21 insertions(+), 7 deletions(-)

--- a/arch/alpha/kernel/sys_jensen.c
+++ b/arch/alpha/kernel/sys_jensen.c
@@ -7,7 +7,7 @@
  *
  * Code supporting the Jensen.
  */
-
+#include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/mm.h>
--- a/arch/x86/kernel/topology.c
+++ b/arch/x86/kernel/topology.c
@@ -25,6 +25,7 @@
  *
  * Send feedback to <colpatch@us.ibm.com>
  */
+#include <linux/interrupt.h>
 #include <linux/nodemask.h>
 #include <linux/export.h>
 #include <linux/mmzone.h>
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -232,6 +232,7 @@ extern void devm_free_irq(struct device
 # define local_irq_enable_in_hardirq()	local_irq_enable()
 #endif
 
+bool irq_has_action(unsigned int irq);
 extern void disable_irq_nosync(unsigned int irq);
 extern bool disable_hardirq(unsigned int irq);
 extern void disable_irq(unsigned int irq);
--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -179,12 +179,7 @@ int handle_domain_nmi(struct irq_domain
 /* Test to see if a driver has successfully requested an irq */
 static inline int irq_desc_has_action(struct irq_desc *desc)
 {
-	return desc->action != NULL;
-}
-
-static inline int irq_has_action(unsigned int irq)
-{
-	return irq_desc_has_action(irq_to_desc(irq));
+	return desc && desc->action != NULL;
 }
 
 /**
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -2752,3 +2752,20 @@ int irq_set_irqchip_state(unsigned int i
 	return err;
 }
 EXPORT_SYMBOL_GPL(irq_set_irqchip_state);
+
+/**
+ * irq_has_action - Check whether an interrupt is requested
+ * @irq:	The linux irq number
+ *
+ * Returns: A snapshot of the current state
+ */
+bool irq_has_action(unsigned int irq)
+{
+	bool res;
+
+	rcu_read_lock();
+	res = irq_desc_has_action(irq_to_desc(irq));
+	rcu_read_unlock();
+	return res;
+}
+EXPORT_SYMBOL_GPL(irq_has_action);

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 02/30] genirq: Move status flag checks to core
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
  2020-12-10 19:25 ` [patch 01/30] genirq: Move irq_has_action() into core code Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-27 19:20   ` Guenter Roeck
  2020-12-10 19:25 ` [patch 03/30] genirq: Move irq_set_lockdep_class() " Thomas Gleixner
                   ` (27 subsequent siblings)
  29 siblings, 1 reply; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, linux-pci, Jakub Kicinski,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Juergen Gross,
	David Airlie, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller

These checks are used by modules and prevent the removal of the export of
irq_to_desc(). Move the accessor into the core.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/irqdesc.h |   17 +++++------------
 kernel/irq/manage.c     |   17 +++++++++++++++++
 2 files changed, 22 insertions(+), 12 deletions(-)

--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -223,28 +223,21 @@ irq_set_chip_handler_name_locked(struct
 	data->chip = chip;
 }
 
+bool irq_check_status_bit(unsigned int irq, unsigned int bitmask);
+
 static inline bool irq_balancing_disabled(unsigned int irq)
 {
-	struct irq_desc *desc;
-
-	desc = irq_to_desc(irq);
-	return desc->status_use_accessors & IRQ_NO_BALANCING_MASK;
+	return irq_check_status_bit(irq, IRQ_NO_BALANCING_MASK);
 }
 
 static inline bool irq_is_percpu(unsigned int irq)
 {
-	struct irq_desc *desc;
-
-	desc = irq_to_desc(irq);
-	return desc->status_use_accessors & IRQ_PER_CPU;
+	return irq_check_status_bit(irq, IRQ_PER_CPU);
 }
 
 static inline bool irq_is_percpu_devid(unsigned int irq)
 {
-	struct irq_desc *desc;
-
-	desc = irq_to_desc(irq);
-	return desc->status_use_accessors & IRQ_PER_CPU_DEVID;
+	return irq_check_status_bit(irq, IRQ_PER_CPU_DEVID);
 }
 
 static inline void
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -2769,3 +2769,23 @@ bool irq_has_action(unsigned int irq)
 	return res;
 }
 EXPORT_SYMBOL_GPL(irq_has_action);
+
+/**
+ * irq_check_status_bit - Check whether bits in the irq descriptor status are set
+ * @irq:	The linux irq number
+ * @bitmask:	The bitmask to evaluate
+ *
+ * Returns: True if one of the bits in @bitmask is set
+ */
+bool irq_check_status_bit(unsigned int irq, unsigned int bitmask)
+{
+	struct irq_desc *desc;
+	bool res = false;
+
+	rcu_read_lock();
+	desc = irq_to_desc(irq);
+	if (desc)
+		res = !!(desc->status_use_accessors & bitmask);
+	rcu_read_unlock();
+	return res;
+}

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 03/30] genirq: Move irq_set_lockdep_class() to core
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
  2020-12-10 19:25 ` [patch 01/30] genirq: Move irq_has_action() into core code Thomas Gleixner
  2020-12-10 19:25 ` [patch 02/30] genirq: Move status flag checks to core Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-11 17:53   ` Andy Shevchenko
  2020-12-10 19:25 ` [patch 04/30] genirq: Provide irq_get_effective_affinity() Thomas Gleixner
                   ` (26 subsequent siblings)
  29 siblings, 1 reply; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, linux-pci, Jakub Kicinski,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Juergen Gross,
	David Airlie, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller

irq_set_lockdep_class() is used from modules and requires irq_to_desc() to
be exported. Move it into the core code which lifts another requirement for
the export.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/irqdesc.h |   10 ++++------
 kernel/irq/irqdesc.c    |   14 ++++++++++++++
 2 files changed, 18 insertions(+), 6 deletions(-)

--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -240,16 +240,14 @@ static inline bool irq_is_percpu_devid(u
 	return irq_check_status_bit(irq, IRQ_PER_CPU_DEVID);
 }
 
+void __irq_set_lockdep_class(unsigned int irq, struct lock_class_key *lock_class,
+			     struct lock_class_key *request_class);
 static inline void
 irq_set_lockdep_class(unsigned int irq, struct lock_class_key *lock_class,
 		      struct lock_class_key *request_class)
 {
-	struct irq_desc *desc = irq_to_desc(irq);
-
-	if (desc) {
-		lockdep_set_class(&desc->lock, lock_class);
-		lockdep_set_class(&desc->request_mutex, request_class);
-	}
+	if (IS_ENABLED(CONFIG_LOCKDEP))
+		__irq_set_lockdep_class(irq, lock_class, request_class);
 }
 
 #endif
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -968,3 +968,17 @@ unsigned int kstat_irqs_usr(unsigned int
 	rcu_read_unlock();
 	return sum;
 }
+
+#ifdef CONFIG_LOCKDEP
+void __irq_set_lockdep_class(unsigned int irq, struct lock_class_key *lock_class,
+			     struct lock_class_key *request_class)
+{
+	struct irq_desc *desc = irq_to_desc(irq);
+
+	if (desc) {
+		lockdep_set_class(&desc->lock, lock_class);
+		lockdep_set_class(&desc->request_mutex, request_class);
+	}
+}
+EXPORT_SYMBOL_GPL(irq_set_lockdep_class);
+#endif

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 04/30] genirq: Provide irq_get_effective_affinity()
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (2 preceding siblings ...)
  2020-12-10 19:25 ` [patch 03/30] genirq: Move irq_set_lockdep_class() " Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-10 19:25 ` [patch 05/30] genirq: Annotate irq stats data races Thomas Gleixner
                   ` (25 subsequent siblings)
  29 siblings, 0 replies; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, linux-pci, Jakub Kicinski,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Juergen Gross,
	David Airlie, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller

Provide an accessor to the effective interrupt affinity mask. Going to be
used to replace open coded fiddling with the irq descriptor.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/irq.h |    7 +++++++
 1 file changed, 7 insertions(+)

--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -907,6 +907,13 @@ struct cpumask *irq_data_get_effective_a
 }
 #endif
 
+static inline struct cpumask *irq_get_effective_affinity_mask(unsigned int irq)
+{
+	struct irq_data *d = irq_get_irq_data(irq);
+
+	return d ? irq_data_get_effective_affinity_mask(d) : NULL;
+}
+
 unsigned int arch_dynirq_lower_bound(unsigned int from);
 
 int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 05/30] genirq: Annotate irq stats data races
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (3 preceding siblings ...)
  2020-12-10 19:25 ` [patch 04/30] genirq: Provide irq_get_effective_affinity() Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-10 19:25 ` [patch 06/30] parisc/irq: Simplify irq count output for /proc/interrupts Thomas Gleixner
                   ` (24 subsequent siblings)
  29 siblings, 0 replies; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, linux-pci, Jakub Kicinski,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Juergen Gross,
	David Airlie, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller

Both the per cpu stats and the accumulated count are accessed lockless and
can be concurrently modified. That's intentional and the stats are a rough
estimate anyway. Annotate them with data_race().

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/irq/irqdesc.c |    4 ++--
 kernel/irq/proc.c    |    5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -943,10 +943,10 @@ unsigned int kstat_irqs(unsigned int irq
 	if (!irq_settings_is_per_cpu_devid(desc) &&
 	    !irq_settings_is_per_cpu(desc) &&
 	    !irq_is_nmi(desc))
-	    return desc->tot_count;
+		return data_race(desc->tot_count);
 
 	for_each_possible_cpu(cpu)
-		sum += *per_cpu_ptr(desc->kstat_irqs, cpu);
+		sum += data_race(*per_cpu_ptr(desc->kstat_irqs, cpu));
 	return sum;
 }
 
--- a/kernel/irq/proc.c
+++ b/kernel/irq/proc.c
@@ -488,9 +488,10 @@ int show_interrupts(struct seq_file *p,
 	if (!desc || irq_settings_is_hidden(desc))
 		goto outsparse;
 
-	if (desc->kstat_irqs)
+	if (desc->kstat_irqs) {
 		for_each_online_cpu(j)
-			any_count |= *per_cpu_ptr(desc->kstat_irqs, j);
+			any_count |= data_race(*per_cpu_ptr(desc->kstat_irqs, j));
+	}
 
 	if ((!desc->action || irq_desc_is_chained(desc)) && !any_count)
 		goto outsparse;

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 06/30] parisc/irq: Simplify irq count output for /proc/interrupts
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (4 preceding siblings ...)
  2020-12-10 19:25 ` [patch 05/30] genirq: Annotate irq stats data races Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-10 19:25 ` [patch 07/30] genirq: Make kstat_irqs() static Thomas Gleixner
                   ` (23 subsequent siblings)
  29 siblings, 0 replies; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, linux-pci, Jakub Kicinski,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Juergen Gross,
	David Airlie, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller

The SMP variant works perfectly fine on UP as well.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Helge Deller <deller@gmx.de>
Cc: afzal mohammed <afzal.mohd.ma@gmail.com>
Cc: linux-parisc@vger.kernel.org
---
 arch/parisc/kernel/irq.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

--- a/arch/parisc/kernel/irq.c
+++ b/arch/parisc/kernel/irq.c
@@ -216,12 +216,9 @@ int show_interrupts(struct seq_file *p,
 		if (!action)
 			goto skip;
 		seq_printf(p, "%3d: ", i);
-#ifdef CONFIG_SMP
+
 		for_each_online_cpu(j)
 			seq_printf(p, "%10u ", kstat_irqs_cpu(i, j));
-#else
-		seq_printf(p, "%10u ", kstat_irqs(i));
-#endif
 
 		seq_printf(p, " %14s", irq_desc_get_chip(desc)->name);
 #ifndef PARISC_IRQ_CR16_COUNTS

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 07/30] genirq: Make kstat_irqs() static
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (5 preceding siblings ...)
  2020-12-10 19:25 ` [patch 06/30] parisc/irq: Simplify irq count output for /proc/interrupts Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-10 19:25 ` [patch 08/30] genirq: Provide kstat_irqdesc_cpu() Thomas Gleixner
                   ` (22 subsequent siblings)
  29 siblings, 0 replies; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, linux-pci, Jakub Kicinski,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Juergen Gross,
	David Airlie, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller

No more users outside the core code.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/kernel_stat.h |    1 -
 kernel/irq/irqdesc.c        |   19 ++++++-------------
 2 files changed, 6 insertions(+), 14 deletions(-)

--- a/include/linux/kernel_stat.h
+++ b/include/linux/kernel_stat.h
@@ -67,7 +67,6 @@ static inline unsigned int kstat_softirq
 /*
  * Number of interrupts per specific IRQ source, since bootup
  */
-extern unsigned int kstat_irqs(unsigned int irq);
 extern unsigned int kstat_irqs_usr(unsigned int irq);
 
 /*
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -924,15 +924,7 @@ static bool irq_is_nmi(struct irq_desc *
 	return desc->istate & IRQS_NMI;
 }
 
-/**
- * kstat_irqs - Get the statistics for an interrupt
- * @irq:	The interrupt number
- *
- * Returns the sum of interrupt counts on all cpus since boot for
- * @irq. The caller must ensure that the interrupt is not removed
- * concurrently.
- */
-unsigned int kstat_irqs(unsigned int irq)
+static unsigned int kstat_irqs(unsigned int irq)
 {
 	struct irq_desc *desc = irq_to_desc(irq);
 	unsigned int sum = 0;
@@ -951,13 +943,14 @@ unsigned int kstat_irqs(unsigned int irq
 }
 
 /**
- * kstat_irqs_usr - Get the statistics for an interrupt
+ * kstat_irqs_usr - Get the statistics for an interrupt from thread context
  * @irq:	The interrupt number
  *
  * Returns the sum of interrupt counts on all cpus since boot for @irq.
- * Contrary to kstat_irqs() this can be called from any context.
- * It uses rcu since a concurrent removal of an interrupt descriptor is
- * observing an rcu grace period before delayed_free_desc()/irq_kobj_release().
+ *
+ * It uses rcu to protect the access since a concurrent removal of an
+ * interrupt descriptor is observing an rcu grace period before
+ * delayed_free_desc()/irq_kobj_release().
  */
 unsigned int kstat_irqs_usr(unsigned int irq)
 {

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 08/30] genirq: Provide kstat_irqdesc_cpu()
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (6 preceding siblings ...)
  2020-12-10 19:25 ` [patch 07/30] genirq: Make kstat_irqs() static Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-10 19:25 ` [patch 09/30] ARM: smp: Use irq_desc_kstat_cpu() in show_ipi_list() Thomas Gleixner
                   ` (21 subsequent siblings)
  29 siblings, 0 replies; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, linux-pci, Jakub Kicinski,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Juergen Gross,
	David Airlie, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller

Most users of kstat_irqs_cpu() have the irq descriptor already. No point in
calling into the core code and looking it up once more.

Use it in per_cpu_count_show() to start with.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/irqdesc.h |    6 ++++++
 kernel/irq/irqdesc.c    |    4 ++--
 2 files changed, 8 insertions(+), 2 deletions(-)

--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -113,6 +113,12 @@ static inline void irq_unlock_sparse(voi
 extern struct irq_desc irq_desc[NR_IRQS];
 #endif
 
+static inline unsigned int irq_desc_kstat_cpu(struct irq_desc *desc,
+					      unsigned int cpu)
+{
+	return desc->kstat_irqs ? *per_cpu_ptr(desc->kstat_irqs, cpu) : 0;
+}
+
 static inline struct irq_desc *irq_data_to_desc(struct irq_data *data)
 {
 	return container_of(data->common, struct irq_desc, irq_common_data);
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -147,12 +147,12 @@ static ssize_t per_cpu_count_show(struct
 				  struct kobj_attribute *attr, char *buf)
 {
 	struct irq_desc *desc = container_of(kobj, struct irq_desc, kobj);
-	int cpu, irq = desc->irq_data.irq;
 	ssize_t ret = 0;
 	char *p = "";
+	int cpu;
 
 	for_each_possible_cpu(cpu) {
-		unsigned int c = kstat_irqs_cpu(irq, cpu);
+		unsigned int c = irq_desc_kstat_cpu(desc, cpu);
 
 		ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%s%u", p, c);
 		p = ",";

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 09/30] ARM: smp: Use irq_desc_kstat_cpu() in show_ipi_list()
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (7 preceding siblings ...)
  2020-12-10 19:25 ` [patch 08/30] genirq: Provide kstat_irqdesc_cpu() Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-11 18:08   ` Marc Zyngier
  2020-12-10 19:25 ` [patch 10/30] arm64/smp: Use irq_desc_kstat_cpu() in arch_show_interrupts() Thomas Gleixner
                   ` (20 subsequent siblings)
  29 siblings, 1 reply; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, linux-pci, Jakub Kicinski,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Juergen Gross,
	David Airlie, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller

The irq descriptor is already there, no need to look it up again.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Russell King <linux@armlinux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
---
 arch/arm/kernel/smp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -550,7 +550,7 @@ void show_ipi_list(struct seq_file *p, i
 		seq_printf(p, "%*s%u: ", prec - 1, "IPI", i);
 
 		for_each_online_cpu(cpu)
-			seq_printf(p, "%10u ", kstat_irqs_cpu(irq, cpu));
+			seq_printf(p, "%10u ", irq_desc_kstat_cpu(ipi_desc[i], cpu));
 
 		seq_printf(p, " %s\n", ipi_types[i]);
 	}

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 10/30] arm64/smp: Use irq_desc_kstat_cpu() in arch_show_interrupts()
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (8 preceding siblings ...)
  2020-12-10 19:25 ` [patch 09/30] ARM: smp: Use irq_desc_kstat_cpu() in show_ipi_list() Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-11 18:08   ` Marc Zyngier
  2020-12-10 19:25 ` [patch 11/30] parisc/irq: Use irq_desc_kstat_cpu() in show_interrupts() Thomas Gleixner
                   ` (19 subsequent siblings)
  29 siblings, 1 reply; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, afzal mohammed, Michal Simek, linux-s390,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Marc Zyngier, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, Heiko Carstens,
	Wambui Karuga, Allen Hubbe, Juergen Gross, Will Deacon,
	David Airlie, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller

The irq descriptor is already there, no need to look it up again.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org
---
 arch/arm64/kernel/smp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -809,7 +809,7 @@ int arch_show_interrupts(struct seq_file
 		seq_printf(p, "%*s%u:%s", prec - 1, "IPI", i,
 			   prec >= 4 ? " " : "");
 		for_each_online_cpu(cpu)
-			seq_printf(p, "%10u ", kstat_irqs_cpu(irq, cpu));
+			seq_printf(p, "%10u ", irq_desc_kstat_cpu(ipi_desc[i], cpu));
 		seq_printf(p, "      %s\n", ipi_types[i]);
 	}
 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 11/30] parisc/irq: Use irq_desc_kstat_cpu() in show_interrupts()
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (9 preceding siblings ...)
  2020-12-10 19:25 ` [patch 10/30] arm64/smp: Use irq_desc_kstat_cpu() in arch_show_interrupts() Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-10 19:25 ` [patch 12/30] s390/irq: Use irq_desc_kstat_cpu() in show_msi_interrupt() Thomas Gleixner
                   ` (18 subsequent siblings)
  29 siblings, 0 replies; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, linux-pci, Jakub Kicinski,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Juergen Gross,
	David Airlie, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller

The irq descriptor is already there, no need to look it up again.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Helge Deller <deller@gmx.de>
Cc: afzal mohammed <afzal.mohd.ma@gmail.com>
Cc: linux-parisc@vger.kernel.org
---
 arch/parisc/kernel/irq.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/parisc/kernel/irq.c
+++ b/arch/parisc/kernel/irq.c
@@ -218,7 +218,7 @@ int show_interrupts(struct seq_file *p,
 		seq_printf(p, "%3d: ", i);
 
 		for_each_online_cpu(j)
-			seq_printf(p, "%10u ", kstat_irqs_cpu(i, j));
+			seq_printf(p, "%10u ", irq_desc_kstat_cpu(desc, j));
 
 		seq_printf(p, " %14s", irq_desc_get_chip(desc)->name);
 #ifndef PARISC_IRQ_CR16_COUNTS

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 12/30] s390/irq: Use irq_desc_kstat_cpu() in show_msi_interrupt()
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (10 preceding siblings ...)
  2020-12-10 19:25 ` [patch 11/30] parisc/irq: Use irq_desc_kstat_cpu() in show_interrupts() Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-10 20:31   ` Heiko Carstens
  2020-12-10 19:25 ` [patch 13/30] drm/i915/lpe_audio: Remove pointless irq_to_desc() usage Thomas Gleixner
                   ` (17 subsequent siblings)
  29 siblings, 1 reply; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	netdev, Will Deacon, Michal Simek, linux-s390, afzal mohammed,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Marc Zyngier, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, intel-gfx,
	Wambui Karuga, Allen Hubbe, Juergen Gross, Tvrtko Ursulin,
	Heiko Carstens, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	David Airlie, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, Saeed Mahameed,
	David S. Miller

The irq descriptor is already there, no need to look it up again.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: linux-s390@vger.kernel.org
---
 arch/s390/kernel/irq.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/s390/kernel/irq.c
+++ b/arch/s390/kernel/irq.c
@@ -124,7 +124,7 @@ static void show_msi_interrupt(struct se
 	raw_spin_lock_irqsave(&desc->lock, flags);
 	seq_printf(p, "%3d: ", irq);
 	for_each_online_cpu(cpu)
-		seq_printf(p, "%10u ", kstat_irqs_cpu(irq, cpu));
+		seq_printf(p, "%10u ", irq_desc_kstat_irq(desc, cpu));
 
 	if (desc->irq_data.chip)
 		seq_printf(p, " %8s", desc->irq_data.chip->name);

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 13/30] drm/i915/lpe_audio: Remove pointless irq_to_desc() usage
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (11 preceding siblings ...)
  2020-12-10 19:25 ` [patch 12/30] s390/irq: Use irq_desc_kstat_cpu() in show_msi_interrupt() Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-10 19:48   ` [Intel-gfx] " Ville Syrjälä
  2020-12-10 19:25 ` [patch 14/30] drm/i915/pmu: Replace open coded kstat_irqs() copy Thomas Gleixner
                   ` (16 subsequent siblings)
  29 siblings, 1 reply; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	netdev, Will Deacon, Michal Simek, linux-s390, afzal mohammed,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Marc Zyngier, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, Heiko Carstens,
	Wambui Karuga, Allen Hubbe, Juergen Gross, Tvrtko Ursulin,
	intel-gfx, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	David Airlie, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, Saeed Mahameed,
	David S. Miller

Nothing uses the result and nothing should ever use it in driver code.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Wambui Karuga <wambui.karugax@gmail.com>
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/i915/display/intel_lpe_audio.c |    4 ----
 1 file changed, 4 deletions(-)

--- a/drivers/gpu/drm/i915/display/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_lpe_audio.c
@@ -297,13 +297,9 @@ int intel_lpe_audio_init(struct drm_i915
  */
 void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv)
 {
-	struct irq_desc *desc;
-
 	if (!HAS_LPE_AUDIO(dev_priv))
 		return;
 
-	desc = irq_to_desc(dev_priv->lpe_audio.irq);
-
 	lpe_audio_platdev_destroy(dev_priv);
 
 	irq_free_desc(dev_priv->lpe_audio.irq);

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 14/30] drm/i915/pmu: Replace open coded kstat_irqs() copy
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (12 preceding siblings ...)
  2020-12-10 19:25 ` [patch 13/30] drm/i915/lpe_audio: Remove pointless irq_to_desc() usage Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-11  9:54   ` Jani Nikula
  2020-12-11 10:13   ` Tvrtko Ursulin
  2020-12-10 19:25 ` [patch 15/30] pinctrl: nomadik: Use irq_has_action() Thomas Gleixner
                   ` (15 subsequent siblings)
  29 siblings, 2 replies; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	netdev, David Airlie, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, linux-pci, Jakub Kicinski,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Juergen Gross,
	intel-gfx, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, Saeed Mahameed,
	David S. Miller

Driver code has no business with the internals of the irq descriptor.

Aside of that the count is per interrupt line and therefore takes
interrupts from other devices into account which share the interrupt line
and are not handled by the graphics driver.

Replace it with a pmu private count which only counts interrupts which
originate from the graphics card.

To avoid atomics or heuristics of some sort make the counter field
'unsigned long'. That limits the count to 4e9 on 32bit which is a lot and
postprocessing can easily deal with the occasional wraparound.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/i915/i915_irq.c |   34 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_pmu.c |   18 +-----------------
 drivers/gpu/drm/i915/i915_pmu.h |    8 ++++++++
 3 files changed, 43 insertions(+), 17 deletions(-)

--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -60,6 +60,24 @@
  * and related files, but that will be described in separate chapters.
  */
 
+/*
+ * Interrupt statistic for PMU. Increments the counter only if the
+ * interrupt originated from the the GPU so interrupts from a device which
+ * shares the interrupt line are not accounted.
+ */
+static inline void pmu_irq_stats(struct drm_i915_private *priv,
+				 irqreturn_t res)
+{
+	if (unlikely(res != IRQ_HANDLED))
+		return;
+
+	/*
+	 * A clever compiler translates that into INC. A not so clever one
+	 * should at least prevent store tearing.
+	 */
+	WRITE_ONCE(priv->pmu.irq_count, priv->pmu.irq_count + 1);
+}
+
 typedef bool (*long_pulse_detect_func)(enum hpd_pin pin, u32 val);
 
 static const u32 hpd_ilk[HPD_NUM_PINS] = {
@@ -1599,6 +1617,8 @@ static irqreturn_t valleyview_irq_handle
 		valleyview_pipestat_irq_handler(dev_priv, pipe_stats);
 	} while (0);
 
+	pmu_irq_stats(dev_priv, ret);
+
 	enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
 
 	return ret;
@@ -1676,6 +1696,8 @@ static irqreturn_t cherryview_irq_handle
 		valleyview_pipestat_irq_handler(dev_priv, pipe_stats);
 	} while (0);
 
+	pmu_irq_stats(dev_priv, ret);
+
 	enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
 
 	return ret;
@@ -2103,6 +2125,8 @@ static irqreturn_t ilk_irq_handler(int i
 	if (sde_ier)
 		raw_reg_write(regs, SDEIER, sde_ier);
 
+	pmu_irq_stats(i915, ret);
+
 	/* IRQs are synced during runtime_suspend, we don't require a wakeref */
 	enable_rpm_wakeref_asserts(&i915->runtime_pm);
 
@@ -2419,6 +2443,8 @@ static irqreturn_t gen8_irq_handler(int
 
 	gen8_master_intr_enable(regs);
 
+	pmu_irq_stats(dev_priv, IRQ_HANDLED);
+
 	return IRQ_HANDLED;
 }
 
@@ -2514,6 +2540,8 @@ static __always_inline irqreturn_t
 
 	gen11_gu_misc_irq_handler(gt, gu_misc_iir);
 
+	pmu_irq_stats(i915, IRQ_HANDLED);
+
 	return IRQ_HANDLED;
 }
 
@@ -3688,6 +3716,8 @@ static irqreturn_t i8xx_irq_handler(int
 		i8xx_pipestat_irq_handler(dev_priv, iir, pipe_stats);
 	} while (0);
 
+	pmu_irq_stats(dev_priv, ret);
+
 	enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
 
 	return ret;
@@ -3796,6 +3826,8 @@ static irqreturn_t i915_irq_handler(int
 		i915_pipestat_irq_handler(dev_priv, iir, pipe_stats);
 	} while (0);
 
+	pmu_irq_stats(dev_priv, ret);
+
 	enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
 
 	return ret;
@@ -3941,6 +3973,8 @@ static irqreturn_t i965_irq_handler(int
 		i965_pipestat_irq_handler(dev_priv, iir, pipe_stats);
 	} while (0);
 
+	pmu_irq_stats(dev_priv, IRQ_HANDLED);
+
 	enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
 
 	return ret;
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -423,22 +423,6 @@ static enum hrtimer_restart i915_sample(
 	return HRTIMER_RESTART;
 }
 
-static u64 count_interrupts(struct drm_i915_private *i915)
-{
-	/* open-coded kstat_irqs() */
-	struct irq_desc *desc = irq_to_desc(i915->drm.pdev->irq);
-	u64 sum = 0;
-	int cpu;
-
-	if (!desc || !desc->kstat_irqs)
-		return 0;
-
-	for_each_possible_cpu(cpu)
-		sum += *per_cpu_ptr(desc->kstat_irqs, cpu);
-
-	return sum;
-}
-
 static void i915_pmu_event_destroy(struct perf_event *event)
 {
 	struct drm_i915_private *i915 =
@@ -581,7 +565,7 @@ static u64 __i915_pmu_event_read(struct
 				   USEC_PER_SEC /* to MHz */);
 			break;
 		case I915_PMU_INTERRUPTS:
-			val = count_interrupts(i915);
+			val = READ_ONCE(pmu->irq_count);
 			break;
 		case I915_PMU_RC6_RESIDENCY:
 			val = get_rc6(&i915->gt);
--- a/drivers/gpu/drm/i915/i915_pmu.h
+++ b/drivers/gpu/drm/i915/i915_pmu.h
@@ -108,6 +108,14 @@ struct i915_pmu {
 	 */
 	ktime_t sleep_last;
 	/**
+	 * @irq_count: Number of interrupts
+	 *
+	 * Intentionally unsigned long to avoid atomics or heuristics on 32bit.
+	 * 4e9 interrupts are a lot and postprocessing can really deal with an
+	 * occasional wraparound easily. It's 32bit after all.
+	 */
+	unsigned long irq_count;
+	/**
 	 * @events_attr_group: Device events attribute group.
 	 */
 	struct attribute_group events_attr_group;

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 15/30] pinctrl: nomadik: Use irq_has_action()
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (13 preceding siblings ...)
  2020-12-10 19:25 ` [patch 14/30] drm/i915/pmu: Replace open coded kstat_irqs() copy Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-12  0:45   ` Linus Walleij
  2020-12-10 19:25 ` [patch 16/30] mfd: ab8500-debugfs: Remove the racy fiddling with irq_desc Thomas Gleixner
                   ` (14 subsequent siblings)
  29 siblings, 1 reply; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	netdev, Will Deacon, Michal Simek, linux-s390, afzal mohammed,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Marc Zyngier, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, intel-gfx,
	Wambui Karuga, Allen Hubbe, Juergen Gross, Tvrtko Ursulin,
	Heiko Carstens, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	David Airlie, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, Saeed Mahameed,
	David S. Miller

Let the core code do the fiddling with irq_desc.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-gpio@vger.kernel.org
---
 drivers/pinctrl/nomadik/pinctrl-nomadik.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- a/drivers/pinctrl/nomadik/pinctrl-nomadik.c
+++ b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
@@ -948,7 +948,6 @@ static void nmk_gpio_dbg_show_one(struct
 			   (mode < 0) ? "unknown" : modes[mode]);
 	} else {
 		int irq = chip->to_irq(chip, offset);
-		struct irq_desc	*desc = irq_to_desc(irq);
 		const int pullidx = pull ? 1 : 0;
 		int val;
 		static const char * const pulls[] = {
@@ -969,7 +968,7 @@ static void nmk_gpio_dbg_show_one(struct
 		 * This races with request_irq(), set_irq_type(),
 		 * and set_irq_wake() ... but those are "rare".
 		 */
-		if (irq > 0 && desc && desc->action) {
+		if (irq > 0 && irq_has_action(irq)) {
 			char *trigger;
 
 			if (nmk_chip->edge_rising & BIT(offset))

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 16/30] mfd: ab8500-debugfs: Remove the racy fiddling with irq_desc
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (14 preceding siblings ...)
  2020-12-10 19:25 ` [patch 15/30] pinctrl: nomadik: Use irq_has_action() Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-11  8:22   ` Linus Walleij
                     ` (2 more replies)
  2020-12-10 19:25 ` [patch 17/30] NTB/msi: Use irq_has_action() Thomas Gleixner
                   ` (13 subsequent siblings)
  29 siblings, 3 replies; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	netdev, afzal mohammed, Michal Simek, linux-s390,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Marc Zyngier, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, intel-gfx,
	Wambui Karuga, Allen Hubbe, Juergen Gross, Will Deacon,
	Tvrtko Ursulin, Heiko Carstens, linux-gpio, Stefano Stabellini,
	Rodrigo Vivi, Bjorn Helgaas, Lee Jones, linux-arm-kernel,
	Boris Ostrovsky, David Airlie, linux-parisc, Pankaj Bharadiya,
	Hou Zhiqiang, Tariq Toukan, Jon Mason, linux-ntb, Saeed Mahameed,
	David S. Miller

First of all drivers have absolutely no business to dig into the internals
of an irq descriptor. That's core code and subject to change. All of this
information is readily available to /proc/interrupts in a safe and race
free way.

Remove the inspection code which is a blatant violation of subsystem
boundaries and racy against concurrent modifications of the interrupt
descriptor.

Print the irq line instead so the information can be looked up in a sane
way in /proc/interrupts.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org
---
 drivers/mfd/ab8500-debugfs.c |   16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

--- a/drivers/mfd/ab8500-debugfs.c
+++ b/drivers/mfd/ab8500-debugfs.c
@@ -1513,24 +1513,14 @@ static int ab8500_interrupts_show(struct
 {
 	int line;
 
-	seq_puts(s, "name: number:  number of: wake:\n");
+	seq_puts(s, "name: number: irq: number of: wake:\n");
 
 	for (line = 0; line < num_interrupt_lines; line++) {
-		struct irq_desc *desc = irq_to_desc(line + irq_first);
-
-		seq_printf(s, "%3i:  %6i %4i",
+		seq_printf(s, "%3i:  %6i %4i %4i\n",
 			   line,
+			   line + irq_first,
 			   num_interrupts[line],
 			   num_wake_interrupts[line]);
-
-		if (desc && desc->name)
-			seq_printf(s, "-%-8s", desc->name);
-		if (desc && desc->action) {
-			struct irqaction *action = desc->action;
-
-			seq_printf(s, "  %s", action->name);
-			while ((action = action->next) != NULL)
-				seq_printf(s, ", %s", action->name);
 		}
 		seq_putc(s, '\n');
 	}

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 17/30] NTB/msi: Use irq_has_action()
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (15 preceding siblings ...)
  2020-12-10 19:25 ` [patch 16/30] mfd: ab8500-debugfs: Remove the racy fiddling with irq_desc Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-10 20:33   ` Logan Gunthorpe
  2020-12-10 19:25 ` [patch 18/30] PCI: xilinx-nwl: Use irq_data_get_irq_chip_data() Thomas Gleixner
                   ` (12 subsequent siblings)
  29 siblings, 1 reply; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, linux-pci, Jakub Kicinski,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Juergen Gross,
	David Airlie, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller

Use the proper core function.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Jon Mason <jdmason@kudzu.us>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Allen Hubbe <allenbh@gmail.com>
Cc: linux-ntb@googlegroups.com
---
 drivers/ntb/msi.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

--- a/drivers/ntb/msi.c
+++ b/drivers/ntb/msi.c
@@ -282,15 +282,13 @@ int ntbm_msi_request_threaded_irq(struct
 				  struct ntb_msi_desc *msi_desc)
 {
 	struct msi_desc *entry;
-	struct irq_desc *desc;
 	int ret;
 
 	if (!ntb->msi)
 		return -EINVAL;
 
 	for_each_pci_msi_entry(entry, ntb->pdev) {
-		desc = irq_to_desc(entry->irq);
-		if (desc->action)
+		if (irq_has_action(entry->irq))
 			continue;
 
 		ret = devm_request_threaded_irq(&ntb->dev, entry->irq, handler,

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 18/30] PCI: xilinx-nwl: Use irq_data_get_irq_chip_data()
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (16 preceding siblings ...)
  2020-12-10 19:25 ` [patch 17/30] NTB/msi: Use irq_has_action() Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-10 22:56   ` Rob Herring
  2020-12-10 19:25 ` [patch 19/30] PCI: mobiveil: " Thomas Gleixner
                   ` (11 subsequent siblings)
  29 siblings, 1 reply; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Russell King, James E.J. Bottomley,
	netdev, Will Deacon, Boris Ostrovsky, afzal mohammed,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Marc Zyngier, Helge Deller, Michal Simek,
	Christian Borntraeger, linux-pci, Jakub Kicinski, intel-gfx,
	Wambui Karuga, Allen Hubbe, linux-s390, Heiko Carstens,
	Tvrtko Ursulin, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, Chris Wilson, linux-arm-kernel,
	Juergen Gross, David Airlie, linux-parisc, Pankaj Bharadiya,
	Hou Zhiqiang, Tariq Toukan, Jon Mason, linux-ntb, Saeed Mahameed,
	David S. Miller

Going through a full irq descriptor lookup instead of just using the proper
helper function which provides direct access is suboptimal.

In fact it _is_ wrong because the chip callback needs to get the chip data
which is relevant for the chip while using the irq descriptor variant
returns the irq chip data of the top level chip of a hierarchy. It does not
matter in this case because the chip is the top level chip, but that
doesn't make it more correct.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: linux-pci@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
---
 drivers/pci/controller/pcie-xilinx-nwl.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

--- a/drivers/pci/controller/pcie-xilinx-nwl.c
+++ b/drivers/pci/controller/pcie-xilinx-nwl.c
@@ -379,13 +379,11 @@ static void nwl_pcie_msi_handler_low(str
 
 static void nwl_mask_leg_irq(struct irq_data *data)
 {
-	struct irq_desc *desc = irq_to_desc(data->irq);
-	struct nwl_pcie *pcie;
+	struct nwl_pcie *pcie = irq_data_get_irq_chip_data(data);
 	unsigned long flags;
 	u32 mask;
 	u32 val;
 
-	pcie = irq_desc_get_chip_data(desc);
 	mask = 1 << (data->hwirq - 1);
 	raw_spin_lock_irqsave(&pcie->leg_mask_lock, flags);
 	val = nwl_bridge_readl(pcie, MSGF_LEG_MASK);
@@ -395,13 +393,11 @@ static void nwl_mask_leg_irq(struct irq_
 
 static void nwl_unmask_leg_irq(struct irq_data *data)
 {
-	struct irq_desc *desc = irq_to_desc(data->irq);
-	struct nwl_pcie *pcie;
+	struct nwl_pcie *pcie = irq_data_get_irq_chip_data(data);
 	unsigned long flags;
 	u32 mask;
 	u32 val;
 
-	pcie = irq_desc_get_chip_data(desc);
 	mask = 1 << (data->hwirq - 1);
 	raw_spin_lock_irqsave(&pcie->leg_mask_lock, flags);
 	val = nwl_bridge_readl(pcie, MSGF_LEG_MASK);

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 19/30] PCI: mobiveil: Use irq_data_get_irq_chip_data()
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (17 preceding siblings ...)
  2020-12-10 19:25 ` [patch 18/30] PCI: xilinx-nwl: Use irq_data_get_irq_chip_data() Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-10 22:56   ` Rob Herring
  2020-12-10 19:25 ` [patch 20/30] net/mlx4: Replace irq_to_desc() abuse Thomas Gleixner
                   ` (10 subsequent siblings)
  29 siblings, 1 reply; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, linux-pci, Jakub Kicinski,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Juergen Gross,
	linux-s390, Hou Zhiqiang, Tvrtko Ursulin, linux-gpio,
	Stefano Stabellini, Rodrigo Vivi, Bjorn Helgaas, Lee Jones,
	linux-arm-kernel, Boris Ostrovsky, David Airlie, linux-parisc,
	Pankaj Bharadiya, Tariq Toukan, Jon Mason, linux-ntb, intel-gfx,
	David S. Miller

Going through a full irq descriptor lookup instead of just using the proper
helper function which provides direct access is suboptimal.

In fact it _is_ wrong because the chip callback needs to get the chip data
which is relevant for the chip while using the irq descriptor variant
returns the irq chip data of the top level chip of a hierarchy. It does not
matter in this case because the chip is the top level chip, but that
doesn't make it more correct.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Karthikeyan Mitran <m.karthikeyan@mobiveil.co.in>
Cc: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
---
 drivers/pci/controller/mobiveil/pcie-mobiveil-host.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

--- a/drivers/pci/controller/mobiveil/pcie-mobiveil-host.c
+++ b/drivers/pci/controller/mobiveil/pcie-mobiveil-host.c
@@ -306,13 +306,11 @@ int mobiveil_host_init(struct mobiveil_p
 
 static void mobiveil_mask_intx_irq(struct irq_data *data)
 {
-	struct irq_desc *desc = irq_to_desc(data->irq);
-	struct mobiveil_pcie *pcie;
+	struct mobiveil_pcie *pcie = irq_data_get_irq_chip_data(data);
 	struct mobiveil_root_port *rp;
 	unsigned long flags;
 	u32 mask, shifted_val;
 
-	pcie = irq_desc_get_chip_data(desc);
 	rp = &pcie->rp;
 	mask = 1 << ((data->hwirq + PAB_INTX_START) - 1);
 	raw_spin_lock_irqsave(&rp->intx_mask_lock, flags);
@@ -324,13 +322,11 @@ static void mobiveil_mask_intx_irq(struc
 
 static void mobiveil_unmask_intx_irq(struct irq_data *data)
 {
-	struct irq_desc *desc = irq_to_desc(data->irq);
-	struct mobiveil_pcie *pcie;
+	struct mobiveil_pcie *pcie = irq_data_get_irq_chip_data(data);
 	struct mobiveil_root_port *rp;
 	unsigned long flags;
 	u32 shifted_val, mask;
 
-	pcie = irq_desc_get_chip_data(desc);
 	rp = &pcie->rp;
 	mask = 1 << ((data->hwirq + PAB_INTX_START) - 1);
 	raw_spin_lock_irqsave(&rp->intx_mask_lock, flags);

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 20/30] net/mlx4: Replace irq_to_desc() abuse
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (18 preceding siblings ...)
  2020-12-10 19:25 ` [patch 19/30] PCI: mobiveil: " Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-13 11:24   ` Tariq Toukan
  2020-12-10 19:25 ` [patch 21/30] net/mlx4: Use effective interrupt affinity Thomas Gleixner
                   ` (9 subsequent siblings)
  29 siblings, 1 reply; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Will Deacon, Michal Simek, linux-s390, afzal mohammed,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Marc Zyngier, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, intel-gfx,
	Wambui Karuga, Allen Hubbe, Juergen Gross, Tvrtko Ursulin,
	Heiko Carstens, Jon Mason, linux-gpio, Stefano Stabellini,
	Rodrigo Vivi, Bjorn Helgaas, Lee Jones, linux-arm-kernel,
	Boris Ostrovsky, David Airlie, linux-parisc, netdev,
	Hou Zhiqiang, Tariq Toukan, Pankaj Bharadiya, linux-ntb,
	Saeed Mahameed, David S. Miller

No driver has any business with the internals of an interrupt
descriptor. Storing a pointer to it just to use yet another helper at the
actual usage site to retrieve the affinity mask is creative at best. Just
because C does not allow encapsulation does not mean that the kernel has no
limits.

Retrieve a pointer to the affinity mask itself and use that. It's still
using an interface which is usually not for random drivers, but definitely
less hideous than the previous hack.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Tariq Toukan <tariqt@nvidia.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
Cc: linux-rdma@vger.kernel.org
---
 drivers/net/ethernet/mellanox/mlx4/en_cq.c   |    8 +++-----
 drivers/net/ethernet/mellanox/mlx4/en_rx.c   |    6 +-----
 drivers/net/ethernet/mellanox/mlx4/mlx4_en.h |    3 ++-
 3 files changed, 6 insertions(+), 11 deletions(-)

--- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
@@ -90,7 +90,7 @@ int mlx4_en_activate_cq(struct mlx4_en_p
 			int cq_idx)
 {
 	struct mlx4_en_dev *mdev = priv->mdev;
-	int err = 0;
+	int irq, err = 0;
 	int timestamp_en = 0;
 	bool assigned_eq = false;
 
@@ -116,10 +116,8 @@ int mlx4_en_activate_cq(struct mlx4_en_p
 
 			assigned_eq = true;
 		}
-
-		cq->irq_desc =
-			irq_to_desc(mlx4_eq_get_irq(mdev->dev,
-						    cq->vector));
+		irq = mlx4_eq_get_irq(mdev->dev, cq->vector);
+		cq->aff_mask = irq_get_affinity_mask(irq);
 	} else {
 		/* For TX we use the same irq per
 		ring we assigned for the RX    */
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -959,8 +959,6 @@ int mlx4_en_poll_rx_cq(struct napi_struc
 
 	/* If we used up all the quota - we're probably not done yet... */
 	if (done == budget || !clean_complete) {
-		const struct cpumask *aff;
-		struct irq_data *idata;
 		int cpu_curr;
 
 		/* in case we got here because of !clean_complete */
@@ -969,10 +967,8 @@ int mlx4_en_poll_rx_cq(struct napi_struc
 		INC_PERF_COUNTER(priv->pstats.napi_quota);
 
 		cpu_curr = smp_processor_id();
-		idata = irq_desc_get_irq_data(cq->irq_desc);
-		aff = irq_data_get_affinity_mask(idata);
 
-		if (likely(cpumask_test_cpu(cpu_curr, aff)))
+		if (likely(cpumask_test_cpu(cpu_curr, cq->aff_mask)))
 			return budget;
 
 		/* Current cpu is not according to smp_irq_affinity -
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -46,6 +46,7 @@
 #endif
 #include <linux/cpu_rmap.h>
 #include <linux/ptp_clock_kernel.h>
+#include <linux/irq.h>
 #include <net/xdp.h>
 
 #include <linux/mlx4/device.h>
@@ -380,7 +381,7 @@ struct mlx4_en_cq {
 	struct mlx4_cqe *buf;
 #define MLX4_EN_OPCODE_ERROR	0x1e
 
-	struct irq_desc *irq_desc;
+	const struct cpumask *aff_mask;
 };
 
 struct mlx4_en_port_profile {

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 21/30] net/mlx4: Use effective interrupt affinity
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (19 preceding siblings ...)
  2020-12-10 19:25 ` [patch 20/30] net/mlx4: Replace irq_to_desc() abuse Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-13 11:31   ` Tariq Toukan
  2020-12-10 19:25 ` [patch 22/30] net/mlx5: Replace irq_to_desc() abuse Thomas Gleixner
                   ` (8 subsequent siblings)
  29 siblings, 1 reply; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Will Deacon, Michal Simek, linux-s390, afzal mohammed,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Marc Zyngier, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, intel-gfx,
	Wambui Karuga, Allen Hubbe, Juergen Gross, Tvrtko Ursulin,
	Heiko Carstens, Jon Mason, linux-gpio, Stefano Stabellini,
	Rodrigo Vivi, Bjorn Helgaas, Lee Jones, linux-arm-kernel,
	Boris Ostrovsky, David Airlie, linux-parisc, netdev,
	Hou Zhiqiang, Tariq Toukan, Pankaj Bharadiya, linux-ntb,
	Saeed Mahameed, David S. Miller

Using the interrupt affinity mask for checking locality is not really
working well on architectures which support effective affinity masks.

The affinity mask is either the system wide default or set by user space,
but the architecture can or even must reduce the mask to the effective set,
which means that checking the affinity mask itself does not really tell
about the actual target CPUs.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Tariq Toukan <tariqt@nvidia.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
Cc: linux-rdma@vger.kernel.org
---
 drivers/net/ethernet/mellanox/mlx4/en_cq.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
@@ -117,7 +117,7 @@ int mlx4_en_activate_cq(struct mlx4_en_p
 			assigned_eq = true;
 		}
 		irq = mlx4_eq_get_irq(mdev->dev, cq->vector);
-		cq->aff_mask = irq_get_affinity_mask(irq);
+		cq->aff_mask = irq_get_effective_affinity_mask(irq);
 	} else {
 		/* For TX we use the same irq per
 		ring we assigned for the RX    */

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 22/30] net/mlx5: Replace irq_to_desc() abuse
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (20 preceding siblings ...)
  2020-12-10 19:25 ` [patch 21/30] net/mlx4: Use effective interrupt affinity Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-13 11:34   ` Tariq Toukan
  2020-12-14 21:13   ` Saeed Mahameed
  2020-12-10 19:25 ` [patch 23/30] net/mlx5: Use effective interrupt affinity Thomas Gleixner
                   ` (7 subsequent siblings)
  29 siblings, 2 replies; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, linux-pci, Jakub Kicinski,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Juergen Gross,
	David Airlie, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller

No driver has any business with the internals of an interrupt
descriptor. Storing a pointer to it just to use yet another helper at the
actual usage site to retrieve the affinity mask is creative at best. Just
because C does not allow encapsulation does not mean that the kernel has no
limits.

Retrieve a pointer to the affinity mask itself and use that. It's still
using an interface which is usually not for random drivers, but definitely
less hideous than the previous hack.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/net/ethernet/mellanox/mlx5/core/en.h      |    2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c |    2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c |    6 +-----
 3 files changed, 3 insertions(+), 7 deletions(-)

--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -669,7 +669,7 @@ struct mlx5e_channel {
 	spinlock_t                 async_icosq_lock;
 
 	/* data path - accessed per napi poll */
-	struct irq_desc *irq_desc;
+	const struct cpumask	  *aff_mask;
 	struct mlx5e_ch_stats     *stats;
 
 	/* control */
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -1998,7 +1998,7 @@ static int mlx5e_open_channel(struct mlx
 	c->num_tc   = params->num_tc;
 	c->xdp      = !!params->xdp_prog;
 	c->stats    = &priv->channel_stats[ix].ch;
-	c->irq_desc = irq_to_desc(irq);
+	c->aff_mask = irq_get_affinity_mask(irq);
 	c->lag_port = mlx5e_enumerate_lag_port(priv->mdev, ix);
 
 	netif_napi_add(netdev, &c->napi, mlx5e_napi_poll, 64);
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
@@ -40,12 +40,8 @@
 static inline bool mlx5e_channel_no_affinity_change(struct mlx5e_channel *c)
 {
 	int current_cpu = smp_processor_id();
-	const struct cpumask *aff;
-	struct irq_data *idata;
 
-	idata = irq_desc_get_irq_data(c->irq_desc);
-	aff = irq_data_get_affinity_mask(idata);
-	return cpumask_test_cpu(current_cpu, aff);
+	return cpumask_test_cpu(current_cpu, c->aff_mask);
 }
 
 static void mlx5e_handle_tx_dim(struct mlx5e_txqsq *sq)

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 23/30] net/mlx5: Use effective interrupt affinity
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (21 preceding siblings ...)
  2020-12-10 19:25 ` [patch 22/30] net/mlx5: Replace irq_to_desc() abuse Thomas Gleixner
@ 2020-12-10 19:25 ` Thomas Gleixner
  2020-12-13 11:35   ` Tariq Toukan
  2020-12-14 20:58   ` Saeed Mahameed
  2020-12-10 19:26 ` [patch 24/30] xen/events: Remove unused bind_evtchn_to_irq_lateeoi() Thomas Gleixner
                   ` (6 subsequent siblings)
  29 siblings, 2 replies; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:25 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	afzal mohammed, Michal Simek, linux-s390, Lorenzo Pieralisi,
	Dave Jiang, xen-devel, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	Jakub Kicinski, intel-gfx, Wambui Karuga, Allen Hubbe,
	Juergen Gross, Will Deacon, Tvrtko Ursulin, Heiko Carstens,
	Jon Mason, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	David Airlie, linux-parisc, netdev, Hou Zhiqiang, Tariq Toukan,
	Pankaj Bharadiya, linux-ntb, Saeed Mahameed, David S. Miller

Using the interrupt affinity mask for checking locality is not really
working well on architectures which support effective affinity masks.

The affinity mask is either the system wide default or set by user space,
but the architecture can or even must reduce the mask to the effective set,
which means that checking the affinity mask itself does not really tell
about the actual target CPUs.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Saeed Mahameed <saeedm@nvidia.com>
Cc: Leon Romanovsky <leon@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
Cc: linux-rdma@vger.kernel.org
---
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -1998,7 +1998,7 @@ static int mlx5e_open_channel(struct mlx
 	c->num_tc   = params->num_tc;
 	c->xdp      = !!params->xdp_prog;
 	c->stats    = &priv->channel_stats[ix].ch;
-	c->aff_mask = irq_get_affinity_mask(irq);
+	c->aff_mask = irq_get_effective_affinity_mask(irq);
 	c->lag_port = mlx5e_enumerate_lag_port(priv->mdev, ix);
 
 	netif_napi_add(netdev, &c->napi, mlx5e_napi_poll, 64);

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 24/30] xen/events: Remove unused bind_evtchn_to_irq_lateeoi()
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (22 preceding siblings ...)
  2020-12-10 19:25 ` [patch 23/30] net/mlx5: Use effective interrupt affinity Thomas Gleixner
@ 2020-12-10 19:26 ` Thomas Gleixner
  2020-12-10 23:19   ` boris.ostrovsky
  2020-12-10 19:26 ` [patch 25/30] xen/events: Remove disfunct affinity spreading Thomas Gleixner
                   ` (5 subsequent siblings)
  29 siblings, 1 reply; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:26 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Jakub Kicinski, Will Deacon,
	Michal Simek, linux-s390, afzal mohammed, Stefano Stabellini,
	Dave Jiang, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	xen-devel, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	David Airlie, linux-gpio, Lorenzo Pieralisi, Rodrigo Vivi,
	Bjorn Helgaas, Boris Ostrovsky, linux-arm-kernel, Juergen Gross,
	Tvrtko Ursulin, Lee Jones, linux-parisc, Pankaj Bharadiya,
	Hou Zhiqiang, Tariq Toukan, Jon Mason, linux-ntb, intel-gfx,
	David S. Miller

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: xen-devel@lists.xenproject.org
---
 drivers/xen/events/events_base.c |    6 ------
 1 file changed, 6 deletions(-)

--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -1132,12 +1132,6 @@ int bind_evtchn_to_irq(evtchn_port_t evt
 }
 EXPORT_SYMBOL_GPL(bind_evtchn_to_irq);
 
-int bind_evtchn_to_irq_lateeoi(evtchn_port_t evtchn)
-{
-	return bind_evtchn_to_irq_chip(evtchn, &xen_lateeoi_chip);
-}
-EXPORT_SYMBOL_GPL(bind_evtchn_to_irq_lateeoi);
-
 static int bind_ipi_to_irq(unsigned int ipi, unsigned int cpu)
 {
 	struct evtchn_bind_ipi bind_ipi;

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 25/30] xen/events: Remove disfunct affinity spreading
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (23 preceding siblings ...)
  2020-12-10 19:26 ` [patch 24/30] xen/events: Remove unused bind_evtchn_to_irq_lateeoi() Thomas Gleixner
@ 2020-12-10 19:26 ` Thomas Gleixner
  2020-12-10 19:26 ` [patch 26/30] xen/events: Use immediate affinity setting Thomas Gleixner
                   ` (4 subsequent siblings)
  29 siblings, 0 replies; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:26 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Jakub Kicinski, Will Deacon,
	Michal Simek, linux-s390, afzal mohammed, Stefano Stabellini,
	Dave Jiang, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	xen-devel, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	David Airlie, linux-gpio, Lorenzo Pieralisi, Rodrigo Vivi,
	Bjorn Helgaas, Boris Ostrovsky, linux-arm-kernel, Juergen Gross,
	Tvrtko Ursulin, Lee Jones, linux-parisc, Pankaj Bharadiya,
	Hou Zhiqiang, Tariq Toukan, Jon Mason, linux-ntb, intel-gfx,
	David S. Miller

This function can only ever work when the event channels:

  - are already established
  - interrupts assigned to them
  - the affinity has been set by user space already

because any newly set up event channel is forced to be bound to CPU0 and
the affinity mask of the interrupt is forced to contain cpumask_of(0).

As the CPU0 enforcement was in place _before_ this was implemented it's
entirely unclear how that can ever have worked at all.

Remove it as preparation for doing it proper.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: xen-devel@lists.xenproject.org
---
 drivers/xen/events/events_base.c |    9 ---------
 drivers/xen/evtchn.c             |   34 +---------------------------------
 2 files changed, 1 insertion(+), 42 deletions(-)

--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -1696,15 +1696,6 @@ static int set_affinity_irq(struct irq_d
 	return ret;
 }
 
-/* To be called with desc->lock held. */
-int xen_set_affinity_evtchn(struct irq_desc *desc, unsigned int tcpu)
-{
-	struct irq_data *d = irq_desc_get_irq_data(desc);
-
-	return set_affinity_irq(d, cpumask_of(tcpu), false);
-}
-EXPORT_SYMBOL_GPL(xen_set_affinity_evtchn);
-
 static void enable_dynirq(struct irq_data *data)
 {
 	evtchn_port_t evtchn = evtchn_from_irq(data->irq);
--- a/drivers/xen/evtchn.c
+++ b/drivers/xen/evtchn.c
@@ -421,36 +421,6 @@ static void evtchn_unbind_from_user(stru
 	del_evtchn(u, evtchn);
 }
 
-static DEFINE_PER_CPU(int, bind_last_selected_cpu);
-
-static void evtchn_bind_interdom_next_vcpu(evtchn_port_t evtchn)
-{
-	unsigned int selected_cpu, irq;
-	struct irq_desc *desc;
-	unsigned long flags;
-
-	irq = irq_from_evtchn(evtchn);
-	desc = irq_to_desc(irq);
-
-	if (!desc)
-		return;
-
-	raw_spin_lock_irqsave(&desc->lock, flags);
-	selected_cpu = this_cpu_read(bind_last_selected_cpu);
-	selected_cpu = cpumask_next_and(selected_cpu,
-			desc->irq_common_data.affinity, cpu_online_mask);
-
-	if (unlikely(selected_cpu >= nr_cpu_ids))
-		selected_cpu = cpumask_first_and(desc->irq_common_data.affinity,
-				cpu_online_mask);
-
-	this_cpu_write(bind_last_selected_cpu, selected_cpu);
-
-	/* unmask expects irqs to be disabled */
-	xen_set_affinity_evtchn(desc, selected_cpu);
-	raw_spin_unlock_irqrestore(&desc->lock, flags);
-}
-
 static long evtchn_ioctl(struct file *file,
 			 unsigned int cmd, unsigned long arg)
 {
@@ -508,10 +478,8 @@ static long evtchn_ioctl(struct file *fi
 			break;
 
 		rc = evtchn_bind_to_user(u, bind_interdomain.local_port);
-		if (rc == 0) {
+		if (rc == 0)
 			rc = bind_interdomain.local_port;
-			evtchn_bind_interdom_next_vcpu(rc);
-		}
 		break;
 	}
 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 26/30] xen/events: Use immediate affinity setting
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (24 preceding siblings ...)
  2020-12-10 19:26 ` [patch 25/30] xen/events: Remove disfunct affinity spreading Thomas Gleixner
@ 2020-12-10 19:26 ` Thomas Gleixner
  2020-12-10 19:26 ` [patch 27/30] xen/events: Only force affinity mask for percpu interrupts Thomas Gleixner
                   ` (3 subsequent siblings)
  29 siblings, 0 replies; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:26 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Jakub Kicinski, Will Deacon,
	Michal Simek, linux-s390, afzal mohammed, Stefano Stabellini,
	Dave Jiang, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	xen-devel, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	David Airlie, linux-gpio, Lorenzo Pieralisi, Rodrigo Vivi,
	Bjorn Helgaas, Boris Ostrovsky, linux-arm-kernel, Juergen Gross,
	Tvrtko Ursulin, Lee Jones, linux-parisc, Pankaj Bharadiya,
	Hou Zhiqiang, Tariq Toukan, Jon Mason, linux-ntb, intel-gfx,
	David S. Miller

There is absolutely no reason to mimic the x86 deferred affinity
setting. This mechanism is required to handle the hardware induced issues
of IO/APIC and MSI and is not in use when the interrupts are remapped.

XEN does not need this and can simply change the affinity from the calling
context. The core code invokes this with the interrupt descriptor lock held
so it is fully serialized against any other operation.

Mark the interrupts with IRQ_MOVE_PCNTXT to disable the deferred affinity
setting. The conditional mask/unmask operation is already handled in
xen_rebind_evtchn_to_cpu().

This makes XEN on x86 use the same mechanics as on e.g. ARM64 where
deferred affinity setting is not required and not implemented and the code
path in the ack functions is compiled out.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: xen-devel@lists.xenproject.org
---
 drivers/xen/events/events_base.c |   35 +++++++++--------------------------
 1 file changed, 9 insertions(+), 26 deletions(-)

--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -628,6 +628,11 @@ static void xen_irq_init(unsigned irq)
 	info->refcnt = -1;
 
 	set_info_for_irq(irq, info);
+	/*
+	 * Interrupt affinity setting can be immediate. No point
+	 * in delaying it until an interrupt is handled.
+	 */
+	irq_set_status_flags(irq, IRQ_MOVE_PCNTXT);
 
 	INIT_LIST_HEAD(&info->eoi_list);
 	list_add_tail(&info->list, &xen_irq_list_head);
@@ -739,18 +744,7 @@ static void eoi_pirq(struct irq_data *da
 	if (!VALID_EVTCHN(evtchn))
 		return;
 
-	if (unlikely(irqd_is_setaffinity_pending(data)) &&
-	    likely(!irqd_irq_disabled(data))) {
-		int masked = test_and_set_mask(evtchn);
-
-		clear_evtchn(evtchn);
-
-		irq_move_masked_irq(data);
-
-		if (!masked)
-			unmask_evtchn(evtchn);
-	} else
-		clear_evtchn(evtchn);
+	clear_evtchn(evtchn);
 
 	if (pirq_needs_eoi(data->irq)) {
 		rc = HYPERVISOR_physdev_op(PHYSDEVOP_eoi, &eoi);
@@ -1641,7 +1635,6 @@ void rebind_evtchn_irq(evtchn_port_t evt
 	mutex_unlock(&irq_mapping_update_lock);
 
         bind_evtchn_to_cpu(evtchn, info->cpu);
-	/* This will be deferred until interrupt is processed */
 	irq_set_affinity(irq, cpumask_of(info->cpu));
 
 	/* Unmask the event channel. */
@@ -1688,8 +1681,9 @@ static int set_affinity_irq(struct irq_d
 			    bool force)
 {
 	unsigned tcpu = cpumask_first_and(dest, cpu_online_mask);
-	int ret = xen_rebind_evtchn_to_cpu(evtchn_from_irq(data->irq), tcpu);
+	int ret;
 
+	ret = xen_rebind_evtchn_to_cpu(evtchn_from_irq(data->irq), tcpu);
 	if (!ret)
 		irq_data_update_effective_affinity(data, cpumask_of(tcpu));
 
@@ -1719,18 +1713,7 @@ static void ack_dynirq(struct irq_data *
 	if (!VALID_EVTCHN(evtchn))
 		return;
 
-	if (unlikely(irqd_is_setaffinity_pending(data)) &&
-	    likely(!irqd_irq_disabled(data))) {
-		int masked = test_and_set_mask(evtchn);
-
-		clear_evtchn(evtchn);
-
-		irq_move_masked_irq(data);
-
-		if (!masked)
-			unmask_evtchn(evtchn);
-	} else
-		clear_evtchn(evtchn);
+	clear_evtchn(evtchn);
 }
 
 static void mask_ack_dynirq(struct irq_data *data)

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 27/30] xen/events: Only force affinity mask for percpu interrupts
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (25 preceding siblings ...)
  2020-12-10 19:26 ` [patch 26/30] xen/events: Use immediate affinity setting Thomas Gleixner
@ 2020-12-10 19:26 ` Thomas Gleixner
  2020-12-10 23:20   ` boris.ostrovsky
  2020-12-10 19:26 ` [patch 28/30] xen/events: Reduce irq_info::spurious_cnt storage size Thomas Gleixner
                   ` (2 subsequent siblings)
  29 siblings, 1 reply; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:26 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Jakub Kicinski, Will Deacon,
	Michal Simek, linux-s390, afzal mohammed, Stefano Stabellini,
	Dave Jiang, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	xen-devel, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	David Airlie, linux-gpio, Lorenzo Pieralisi, Rodrigo Vivi,
	Bjorn Helgaas, Boris Ostrovsky, linux-arm-kernel, Juergen Gross,
	Tvrtko Ursulin, Lee Jones, linux-parisc, Pankaj Bharadiya,
	Hou Zhiqiang, Tariq Toukan, Jon Mason, linux-ntb, intel-gfx,
	David S. Miller

All event channel setups bind the interrupt on CPU0 or the target CPU for
percpu interrupts and overwrite the affinity mask with the corresponding
cpumask. That does not make sense.

The XEN implementation of irqchip::irq_set_affinity() already picks a
single target CPU out of the affinity mask and the actual target is stored
in the effective CPU mask, so destroying the user chosen affinity mask
which might contain more than one CPU is wrong.

Change the implementation so that the channel is bound to CPU0 at the XEN
level and leave the affinity mask alone. At startup of the interrupt
affinity will be assigned out of the affinity mask and the XEN binding will
be updated. Only keep the enforcement for real percpu interrupts.

On resume the overwrite is not required either because info->cpu and the
affinity mask are still the same as at the time of suspend. Same for
rebind_evtchn_irq().

This also prepares for proper interrupt spreading.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: xen-devel@lists.xenproject.org
---
 drivers/xen/events/events_base.c |   42 ++++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 14 deletions(-)

--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -433,15 +433,20 @@ static bool pirq_needs_eoi_flag(unsigned
 	return info->u.pirq.flags & PIRQ_NEEDS_EOI;
 }
 
-static void bind_evtchn_to_cpu(evtchn_port_t evtchn, unsigned int cpu)
+static void bind_evtchn_to_cpu(evtchn_port_t evtchn, unsigned int cpu,
+			       bool force_affinity)
 {
 	int irq = get_evtchn_to_irq(evtchn);
 	struct irq_info *info = info_for_irq(irq);
 
 	BUG_ON(irq == -1);
-#ifdef CONFIG_SMP
-	cpumask_copy(irq_get_affinity_mask(irq), cpumask_of(cpu));
-#endif
+
+	if (IS_ENABLED(CONFIG_SMP) && force_affinity) {
+		cpumask_copy(irq_get_affinity_mask(irq), cpumask_of(cpu));
+		cpumask_copy(irq_get_effective_affinity_mask(irq),
+			     cpumask_of(cpu));
+	}
+
 	xen_evtchn_port_bind_to_cpu(evtchn, cpu, info->cpu);
 
 	info->cpu = cpu;
@@ -788,7 +793,7 @@ static unsigned int __startup_pirq(unsig
 		goto err;
 
 	info->evtchn = evtchn;
-	bind_evtchn_to_cpu(evtchn, 0);
+	bind_evtchn_to_cpu(evtchn, 0, false);
 
 	rc = xen_evtchn_port_setup(evtchn);
 	if (rc)
@@ -1107,8 +1112,8 @@ static int bind_evtchn_to_irq_chip(evtch
 			irq = ret;
 			goto out;
 		}
-		/* New interdomain events are bound to VCPU 0. */
-		bind_evtchn_to_cpu(evtchn, 0);
+		/* New interdomain events are initially bound to VCPU 0. */
+		bind_evtchn_to_cpu(evtchn, 0, false);
 	} else {
 		struct irq_info *info = info_for_irq(irq);
 		WARN_ON(info == NULL || info->type != IRQT_EVTCHN);
@@ -1156,7 +1161,11 @@ static int bind_ipi_to_irq(unsigned int
 			irq = ret;
 			goto out;
 		}
-		bind_evtchn_to_cpu(evtchn, cpu);
+		/*
+		 * Force the affinity mask to the target CPU so proc shows
+		 * the correct target.
+		 */
+		bind_evtchn_to_cpu(evtchn, cpu, true);
 	} else {
 		struct irq_info *info = info_for_irq(irq);
 		WARN_ON(info == NULL || info->type != IRQT_IPI);
@@ -1269,7 +1278,11 @@ int bind_virq_to_irq(unsigned int virq,
 			goto out;
 		}
 
-		bind_evtchn_to_cpu(evtchn, cpu);
+		/*
+		 * Force the affinity mask for percpu interrupts so proc
+		 * shows the correct target.
+		 */
+		bind_evtchn_to_cpu(evtchn, cpu, percpu);
 	} else {
 		struct irq_info *info = info_for_irq(irq);
 		WARN_ON(info == NULL || info->type != IRQT_VIRQ);
@@ -1634,8 +1647,7 @@ void rebind_evtchn_irq(evtchn_port_t evt
 
 	mutex_unlock(&irq_mapping_update_lock);
 
-        bind_evtchn_to_cpu(evtchn, info->cpu);
-	irq_set_affinity(irq, cpumask_of(info->cpu));
+	bind_evtchn_to_cpu(evtchn, info->cpu, false);
 
 	/* Unmask the event channel. */
 	enable_irq(irq);
@@ -1669,7 +1681,7 @@ static int xen_rebind_evtchn_to_cpu(evtc
 	 * it, but don't do the xenlinux-level rebind in that case.
 	 */
 	if (HYPERVISOR_event_channel_op(EVTCHNOP_bind_vcpu, &bind_vcpu) >= 0)
-		bind_evtchn_to_cpu(evtchn, tcpu);
+		bind_evtchn_to_cpu(evtchn, tcpu, false);
 
 	if (!masked)
 		unmask_evtchn(evtchn);
@@ -1798,7 +1810,8 @@ static void restore_cpu_virqs(unsigned i
 
 		/* Record the new mapping. */
 		(void)xen_irq_info_virq_setup(cpu, irq, evtchn, virq);
-		bind_evtchn_to_cpu(evtchn, cpu);
+		/* The affinity mask is still valid */
+		bind_evtchn_to_cpu(evtchn, cpu, false);
 	}
 }
 
@@ -1823,7 +1836,8 @@ static void restore_cpu_ipis(unsigned in
 
 		/* Record the new mapping. */
 		(void)xen_irq_info_ipi_setup(cpu, irq, evtchn, ipi);
-		bind_evtchn_to_cpu(evtchn, cpu);
+		/* The affinity mask is still valid */
+		bind_evtchn_to_cpu(evtchn, cpu, false);
 	}
 }
 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 28/30] xen/events: Reduce irq_info::spurious_cnt storage size
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (26 preceding siblings ...)
  2020-12-10 19:26 ` [patch 27/30] xen/events: Only force affinity mask for percpu interrupts Thomas Gleixner
@ 2020-12-10 19:26 ` Thomas Gleixner
  2020-12-10 19:26 ` [patch 29/30] xen/events: Implement irq distribution Thomas Gleixner
  2020-12-10 19:26 ` [patch 30/30] genirq: Remove export of irq_to_desc() Thomas Gleixner
  29 siblings, 0 replies; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:26 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Jakub Kicinski, Will Deacon,
	Michal Simek, linux-s390, afzal mohammed, Stefano Stabellini,
	Dave Jiang, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	xen-devel, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	David Airlie, linux-gpio, Lorenzo Pieralisi, Rodrigo Vivi,
	Bjorn Helgaas, Boris Ostrovsky, linux-arm-kernel, Juergen Gross,
	Tvrtko Ursulin, Lee Jones, linux-parisc, Pankaj Bharadiya,
	Hou Zhiqiang, Tariq Toukan, Jon Mason, linux-ntb, intel-gfx,
	David S. Miller

To prepare for interrupt spreading reduce the storage size of
irq_info::spurious_cnt to u8 so the required flag for the spreading logic
will not increase the storage size.

Protect the usage site against overruns.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: xen-devel@lists.xenproject.org
---
 drivers/xen/events/events_base.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -95,7 +95,7 @@ struct irq_info {
 	struct list_head list;
 	struct list_head eoi_list;
 	short refcnt;
-	short spurious_cnt;
+	u8 spurious_cnt;
 	enum xen_irq_type type; /* type */
 	unsigned irq;
 	evtchn_port_t evtchn;   /* event channel */
@@ -528,8 +528,10 @@ static void xen_irq_lateeoi_locked(struc
 		return;
 
 	if (spurious) {
-		if ((1 << info->spurious_cnt) < (HZ << 2))
-			info->spurious_cnt++;
+		if ((1 << info->spurious_cnt) < (HZ << 2)) {
+			if (info->spurious_cnt != 0xFF)
+				info->spurious_cnt++;
+		}
 		if (info->spurious_cnt > 1) {
 			delay = 1 << (info->spurious_cnt - 2);
 			if (delay > HZ)

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 29/30] xen/events: Implement irq distribution
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (27 preceding siblings ...)
  2020-12-10 19:26 ` [patch 28/30] xen/events: Reduce irq_info::spurious_cnt storage size Thomas Gleixner
@ 2020-12-10 19:26 ` Thomas Gleixner
  2020-12-10 19:26 ` [patch 30/30] genirq: Remove export of irq_to_desc() Thomas Gleixner
  29 siblings, 0 replies; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:26 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Jakub Kicinski, Will Deacon,
	Michal Simek, linux-s390, afzal mohammed, Stefano Stabellini,
	Dave Jiang, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	xen-devel, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	David Airlie, linux-gpio, Lorenzo Pieralisi, Rodrigo Vivi,
	Bjorn Helgaas, Boris Ostrovsky, linux-arm-kernel, Juergen Gross,
	Tvrtko Ursulin, Lee Jones, linux-parisc, Pankaj Bharadiya,
	Hou Zhiqiang, Tariq Toukan, Jon Mason, linux-ntb, intel-gfx,
	David S. Miller

Keep track of the assignments of event channels to CPUs and select the
online CPU with the least assigned channels in the affinity mask which is
handed to irq_chip::irq_set_affinity() from the core code.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: xen-devel@lists.xenproject.org
---
 drivers/xen/events/events_base.c |   72 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 64 insertions(+), 8 deletions(-)

--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -96,6 +96,7 @@ struct irq_info {
 	struct list_head eoi_list;
 	short refcnt;
 	u8 spurious_cnt;
+	u8 is_accounted;
 	enum xen_irq_type type; /* type */
 	unsigned irq;
 	evtchn_port_t evtchn;   /* event channel */
@@ -161,6 +162,9 @@ static DEFINE_PER_CPU(int [NR_VIRQS], vi
 /* IRQ <-> IPI mapping */
 static DEFINE_PER_CPU(int [XEN_NR_IPIS], ipi_to_irq) = {[0 ... XEN_NR_IPIS-1] = -1};
 
+/* Event channel distribution data */
+static atomic_t channels_on_cpu[NR_CPUS];
+
 static int **evtchn_to_irq;
 #ifdef CONFIG_X86
 static unsigned long *pirq_eoi_map;
@@ -257,6 +261,32 @@ static void set_info_for_irq(unsigned in
 		irq_set_chip_data(irq, info);
 }
 
+/* Per CPU channel accounting */
+static void channels_on_cpu_dec(struct irq_info *info)
+{
+	if (!info->is_accounted)
+		return;
+
+	info->is_accounted = 0;
+
+	if (WARN_ON_ONCE(info->cpu >= nr_cpu_ids))
+		return;
+
+	WARN_ON_ONCE(!atomic_add_unless(&channels_on_cpu[info->cpu], -1 , 0));
+}
+
+static void channels_on_cpu_inc(struct irq_info *info)
+{
+	if (WARN_ON_ONCE(info->cpu >= nr_cpu_ids))
+		return;
+
+	if (WARN_ON_ONCE(!atomic_add_unless(&channels_on_cpu[info->cpu], 1,
+					    INT_MAX)))
+		return;
+
+	info->is_accounted = 1;
+}
+
 /* Constructors for packed IRQ information. */
 static int xen_irq_info_common_setup(struct irq_info *info,
 				     unsigned irq,
@@ -339,6 +369,7 @@ static void xen_irq_info_cleanup(struct
 {
 	set_evtchn_to_irq(info->evtchn, -1);
 	info->evtchn = 0;
+	channels_on_cpu_dec(info);
 }
 
 /*
@@ -449,7 +480,9 @@ static void bind_evtchn_to_cpu(evtchn_po
 
 	xen_evtchn_port_bind_to_cpu(evtchn, cpu, info->cpu);
 
+	channels_on_cpu_dec(info);
 	info->cpu = cpu;
+	channels_on_cpu_inc(info);
 }
 
 /**
@@ -622,11 +655,6 @@ static void xen_irq_init(unsigned irq)
 {
 	struct irq_info *info;
 
-#ifdef CONFIG_SMP
-	/* By default all event channels notify CPU#0. */
-	cpumask_copy(irq_get_affinity_mask(irq), cpumask_of(0));
-#endif
-
 	info = kzalloc(sizeof(*info), GFP_KERNEL);
 	if (info == NULL)
 		panic("Unable to allocate metadata for IRQ%d\n", irq);
@@ -1691,10 +1719,34 @@ static int xen_rebind_evtchn_to_cpu(evtc
 	return 0;
 }
 
+/*
+ * Find the CPU within @dest mask which has the least number of channels
+ * assigned. This is not precise as the per cpu counts can be modified
+ * concurrently.
+ */
+static unsigned int select_target_cpu(const struct cpumask *dest)
+{
+	unsigned int cpu, best_cpu = UINT_MAX, minch = UINT_MAX;
+
+	for_each_cpu_and(cpu, dest, cpu_online_mask) {
+		unsigned int curch = atomic_read(&channels_on_cpu[cpu]);
+
+		if (curch < minch) {
+			minch = curch;
+			best_cpu = cpu;
+		}
+	}
+
+	/* If this happens accounting is screwed up */
+	if (WARN_ON_ONCE(best_cpu == UINT_MAX))
+		best_cpu = cpumask_first(dest);
+	return best_cpu;
+}
+
 static int set_affinity_irq(struct irq_data *data, const struct cpumask *dest,
 			    bool force)
 {
-	unsigned tcpu = cpumask_first_and(dest, cpu_online_mask);
+	unsigned int tcpu = select_target_cpu(dest);
 	int ret;
 
 	ret = xen_rebind_evtchn_to_cpu(evtchn_from_irq(data->irq), tcpu);
@@ -1922,8 +1974,12 @@ void xen_irq_resume(void)
 	xen_evtchn_resume();
 
 	/* No IRQ <-> event-channel mappings. */
-	list_for_each_entry(info, &xen_irq_list_head, list)
-		info->evtchn = 0; /* zap event-channel binding */
+	list_for_each_entry(info, &xen_irq_list_head, list) {
+		/* Zap event-channel binding */
+		info->evtchn = 0;
+		/* Adjust accounting */
+		channels_on_cpu_dec(info);
+	}
 
 	clear_evtchn_to_irq_all();
 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [patch 30/30] genirq: Remove export of irq_to_desc()
  2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
                   ` (28 preceding siblings ...)
  2020-12-10 19:26 ` [patch 29/30] xen/events: Implement irq distribution Thomas Gleixner
@ 2020-12-10 19:26 ` Thomas Gleixner
  29 siblings, 0 replies; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-10 19:26 UTC (permalink / raw)
  To: LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, linux-pci, Jakub Kicinski,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Juergen Gross,
	David Airlie, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller

No more (ab)use in modules finally. Remove the export so there won't come
new ones.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/irq/irqdesc.c |    1 -
 1 file changed, 1 deletion(-)

--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -352,7 +352,6 @@ struct irq_desc *irq_to_desc(unsigned in
 {
 	return radix_tree_lookup(&irq_desc_tree, irq);
 }
-EXPORT_SYMBOL(irq_to_desc);
 
 static void delete_irq_desc(unsigned int irq)
 {

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [Intel-gfx] [patch 13/30] drm/i915/lpe_audio: Remove pointless irq_to_desc() usage
  2020-12-10 19:25 ` [patch 13/30] drm/i915/lpe_audio: Remove pointless irq_to_desc() usage Thomas Gleixner
@ 2020-12-10 19:48   ` Ville Syrjälä
  2020-12-11  9:51     ` Jani Nikula
  0 siblings, 1 reply; 73+ messages in thread
From: Ville Syrjälä @ 2020-12-10 19:48 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Russell King, afzal mohammed, Boris Ostrovsky, Lorenzo Pieralisi,
	Dave Jiang, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Michal Simek, Christian Borntraeger, linux-pci,
	xen-devel, intel-gfx, Wambui Karuga, Allen Hubbe, Will Deacon,
	linux-s390, Heiko Carstens, linux-gpio, Stefano Stabellini,
	Jakub Kicinski, Bjorn Helgaas, Lee Jones, linux-arm-kernel,
	Juergen Gross, David Airlie, linux-parisc, netdev, Hou Zhiqiang,
	LKML, Tariq Toukan, Jon Mason, linux-ntb, Saeed Mahameed,
	David S. Miller

On Thu, Dec 10, 2020 at 08:25:49PM +0100, Thomas Gleixner wrote:
> Nothing uses the result and nothing should ever use it in driver code.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Wambui Karuga <wambui.karugax@gmail.com>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_lpe_audio.c |    4 ----
>  1 file changed, 4 deletions(-)
> 
> --- a/drivers/gpu/drm/i915/display/intel_lpe_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_lpe_audio.c
> @@ -297,13 +297,9 @@ int intel_lpe_audio_init(struct drm_i915
>   */
>  void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv)
>  {
> -	struct irq_desc *desc;
> -
>  	if (!HAS_LPE_AUDIO(dev_priv))
>  		return;
>  
> -	desc = irq_to_desc(dev_priv->lpe_audio.irq);
> -
>  	lpe_audio_platdev_destroy(dev_priv);
>  
>  	irq_free_desc(dev_priv->lpe_audio.irq);
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 12/30] s390/irq: Use irq_desc_kstat_cpu() in show_msi_interrupt()
  2020-12-10 19:25 ` [patch 12/30] s390/irq: Use irq_desc_kstat_cpu() in show_msi_interrupt() Thomas Gleixner
@ 2020-12-10 20:31   ` Heiko Carstens
  0 siblings, 0 replies; 73+ messages in thread
From: Heiko Carstens @ 2020-12-10 20:31 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	netdev, Will Deacon, Michal Simek, linux-s390, afzal mohammed,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Marc Zyngier, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, intel-gfx,
	Wambui Karuga, Allen Hubbe, Juergen Gross, David Airlie,
	linux-gpio, Stefano Stabellini, Rodrigo Vivi, Bjorn Helgaas,
	Lee Jones, linux-arm-kernel, Boris Ostrovsky, Tvrtko Ursulin,
	linux-parisc, Pankaj Bharadiya, Hou Zhiqiang, LKML, Tariq Toukan,
	Jon Mason, linux-ntb, Saeed Mahameed, David S. Miller

On Thu, Dec 10, 2020 at 08:25:48PM +0100, Thomas Gleixner wrote:
> The irq descriptor is already there, no need to look it up again.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Heiko Carstens <hca@linux.ibm.com>
> Cc: linux-s390@vger.kernel.org
> ---
>  arch/s390/kernel/irq.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Heiko Carstens <hca@linux.ibm.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 17/30] NTB/msi: Use irq_has_action()
  2020-12-10 19:25 ` [patch 17/30] NTB/msi: Use irq_has_action() Thomas Gleixner
@ 2020-12-10 20:33   ` Logan Gunthorpe
  0 siblings, 0 replies; 73+ messages in thread
From: Logan Gunthorpe @ 2020-12-10 20:33 UTC (permalink / raw)
  To: Thomas Gleixner, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, linux-pci, Jakub Kicinski,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Juergen Gross,
	David Airlie, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller



On 2020-12-10 12:25 p.m., Thomas Gleixner wrote:
> Use the proper core function.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jon Mason <jdmason@kudzu.us>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Allen Hubbe <allenbh@gmail.com>
> Cc: linux-ntb@googlegroups.com

Looks good to me.

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

> ---
>  drivers/ntb/msi.c |    4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> --- a/drivers/ntb/msi.c
> +++ b/drivers/ntb/msi.c
> @@ -282,15 +282,13 @@ int ntbm_msi_request_threaded_irq(struct
>  				  struct ntb_msi_desc *msi_desc)
>  {
>  	struct msi_desc *entry;
> -	struct irq_desc *desc;
>  	int ret;
>  
>  	if (!ntb->msi)
>  		return -EINVAL;
>  
>  	for_each_pci_msi_entry(entry, ntb->pdev) {
> -		desc = irq_to_desc(entry->irq);
> -		if (desc->action)
> +		if (irq_has_action(entry->irq))
>  			continue;
>  
>  		ret = devm_request_threaded_irq(&ntb->dev, entry->irq, handler,
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 18/30] PCI: xilinx-nwl: Use irq_data_get_irq_chip_data()
  2020-12-10 19:25 ` [patch 18/30] PCI: xilinx-nwl: Use irq_data_get_irq_chip_data() Thomas Gleixner
@ 2020-12-10 22:56   ` Rob Herring
  0 siblings, 0 replies; 73+ messages in thread
From: Rob Herring @ 2020-12-10 22:56 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Russell King, James E.J. Bottomley,
	netdev, Will Deacon, Boris Ostrovsky, linux-s390, afzal mohammed,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Marc Zyngier, Helge Deller, Michal Simek,
	Christian Borntraeger, PCI, Jakub Kicinski, Intel Graphics,
	Wambui Karuga, Allen Hubbe, Tvrtko Ursulin, Heiko Carstens,
	open list:GPIO SUBSYSTEM, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, Chris Wilson, linux-arm-kernel,
	Juergen Gross, David Airlie, linux-parisc, Pankaj Bharadiya,
	Hou Zhiqiang, LKML, Tariq Toukan, Jon Mason, linux-ntb,
	Saeed Mahameed, David S. Miller

On Thu, Dec 10, 2020 at 1:42 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Going through a full irq descriptor lookup instead of just using the proper
> helper function which provides direct access is suboptimal.
>
> In fact it _is_ wrong because the chip callback needs to get the chip data
> which is relevant for the chip while using the irq descriptor variant
> returns the irq chip data of the top level chip of a hierarchy. It does not
> matter in this case because the chip is the top level chip, but that
> doesn't make it more correct.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: linux-pci@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  drivers/pci/controller/pcie-xilinx-nwl.c |    8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 19/30] PCI: mobiveil: Use irq_data_get_irq_chip_data()
  2020-12-10 19:25 ` [patch 19/30] PCI: mobiveil: " Thomas Gleixner
@ 2020-12-10 22:56   ` Rob Herring
  0 siblings, 0 replies; 73+ messages in thread
From: Rob Herring @ 2020-12-10 22:56 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, PCI, Jakub Kicinski,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Juergen Gross,
	Tvrtko Ursulin, Hou Zhiqiang, open list:GPIO SUBSYSTEM,
	Stefano Stabellini, Rodrigo Vivi, Bjorn Helgaas, Lee Jones,
	linux-arm-kernel, Boris Ostrovsky, David Airlie, linux-parisc,
	Pankaj Bharadiya, LKML, Tariq Toukan, Jon Mason, linux-ntb,
	Intel Graphics, David S. Miller

On Thu, Dec 10, 2020 at 1:42 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Going through a full irq descriptor lookup instead of just using the proper
> helper function which provides direct access is suboptimal.
>
> In fact it _is_ wrong because the chip callback needs to get the chip data
> which is relevant for the chip while using the irq descriptor variant
> returns the irq chip data of the top level chip of a hierarchy. It does not
> matter in this case because the chip is the top level chip, but that
> doesn't make it more correct.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Karthikeyan Mitran <m.karthikeyan@mobiveil.co.in>
> Cc: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
> ---
>  drivers/pci/controller/mobiveil/pcie-mobiveil-host.c |    8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 24/30] xen/events: Remove unused bind_evtchn_to_irq_lateeoi()
  2020-12-10 19:26 ` [patch 24/30] xen/events: Remove unused bind_evtchn_to_irq_lateeoi() Thomas Gleixner
@ 2020-12-10 23:19   ` boris.ostrovsky
  2020-12-11  0:04     ` Thomas Gleixner
  0 siblings, 1 reply; 73+ messages in thread
From: boris.ostrovsky @ 2020-12-10 23:19 UTC (permalink / raw)
  To: Thomas Gleixner, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Jakub Kicinski, Will Deacon,
	Michal Simek, linux-s390, afzal mohammed, Stefano Stabellini,
	Dave Jiang, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	xen-devel, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	David Airlie, linux-gpio, Lorenzo Pieralisi, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Juergen Gross,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller


On 12/10/20 2:26 PM, Thomas Gleixner wrote:
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: xen-devel@lists.xenproject.org
> ---
>  drivers/xen/events/events_base.c |    6 ------
>  1 file changed, 6 deletions(-)
>
> --- a/drivers/xen/events/events_base.c
> +++ b/drivers/xen/events/events_base.c
> @@ -1132,12 +1132,6 @@ int bind_evtchn_to_irq(evtchn_port_t evt
>  }
>  EXPORT_SYMBOL_GPL(bind_evtchn_to_irq);
>  
> -int bind_evtchn_to_irq_lateeoi(evtchn_port_t evtchn)
> -{
> -	return bind_evtchn_to_irq_chip(evtchn, &xen_lateeoi_chip);
> -}
> -EXPORT_SYMBOL_GPL(bind_evtchn_to_irq_lateeoi);



include/xen/events.h also needs to be updated (and in the next patch for xen_set_affinity_evtchn() as well).


-boris

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 27/30] xen/events: Only force affinity mask for percpu interrupts
  2020-12-10 19:26 ` [patch 27/30] xen/events: Only force affinity mask for percpu interrupts Thomas Gleixner
@ 2020-12-10 23:20   ` boris.ostrovsky
  2020-12-11  0:06     ` Thomas Gleixner
                       ` (2 more replies)
  0 siblings, 3 replies; 73+ messages in thread
From: boris.ostrovsky @ 2020-12-10 23:20 UTC (permalink / raw)
  To: Thomas Gleixner, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Jakub Kicinski, Will Deacon,
	Michal Simek, linux-s390, afzal mohammed, Stefano Stabellini,
	Dave Jiang, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	xen-devel, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	David Airlie, linux-gpio, Lorenzo Pieralisi, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Juergen Gross,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller


On 12/10/20 2:26 PM, Thomas Gleixner wrote:
> All event channel setups bind the interrupt on CPU0 or the target CPU for
> percpu interrupts and overwrite the affinity mask with the corresponding
> cpumask. That does not make sense.
>
> The XEN implementation of irqchip::irq_set_affinity() already picks a
> single target CPU out of the affinity mask and the actual target is stored
> in the effective CPU mask, so destroying the user chosen affinity mask
> which might contain more than one CPU is wrong.
>
> Change the implementation so that the channel is bound to CPU0 at the XEN
> level and leave the affinity mask alone. At startup of the interrupt
> affinity will be assigned out of the affinity mask and the XEN binding will
> be updated. 


If that's the case then I wonder whether we need this call at all and instead bind at startup time.


-boris


> Only keep the enforcement for real percpu interrupts.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 24/30] xen/events: Remove unused bind_evtchn_to_irq_lateeoi()
  2020-12-10 23:19   ` boris.ostrovsky
@ 2020-12-11  0:04     ` Thomas Gleixner
  0 siblings, 0 replies; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-11  0:04 UTC (permalink / raw)
  To: boris.ostrovsky, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Jakub Kicinski, Will Deacon,
	Michal Simek, linux-s390, afzal mohammed, Stefano Stabellini,
	Dave Jiang, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	xen-devel, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	David Airlie, linux-gpio, Lorenzo Pieralisi, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Juergen Gross,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller

On Thu, Dec 10 2020 at 18:19, boris ostrovsky wrote:
> On 12/10/20 2:26 PM, Thomas Gleixner wrote:
>> -EXPORT_SYMBOL_GPL(bind_evtchn_to_irq_lateeoi);
>
> include/xen/events.h also needs to be updated (and in the next patch for xen_set_affinity_evtchn() as well).

Darn, I lost that.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 27/30] xen/events: Only force affinity mask for percpu interrupts
  2020-12-10 23:20   ` boris.ostrovsky
@ 2020-12-11  0:06     ` Thomas Gleixner
  2020-12-11  6:17     ` Jürgen Groß
  2020-12-11 12:10     ` Jürgen Groß
  2 siblings, 0 replies; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-11  0:06 UTC (permalink / raw)
  To: boris.ostrovsky, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Jakub Kicinski, Will Deacon,
	Michal Simek, linux-s390, afzal mohammed, Stefano Stabellini,
	Dave Jiang, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	xen-devel, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	David Airlie, linux-gpio, Lorenzo Pieralisi, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Juergen Gross,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller

On Thu, Dec 10 2020 at 18:20, boris ostrovsky wrote:
> On 12/10/20 2:26 PM, Thomas Gleixner wrote:
>> All event channel setups bind the interrupt on CPU0 or the target CPU for
>> percpu interrupts and overwrite the affinity mask with the corresponding
>> cpumask. That does not make sense.
>>
>> The XEN implementation of irqchip::irq_set_affinity() already picks a
>> single target CPU out of the affinity mask and the actual target is stored
>> in the effective CPU mask, so destroying the user chosen affinity mask
>> which might contain more than one CPU is wrong.
>>
>> Change the implementation so that the channel is bound to CPU0 at the XEN
>> level and leave the affinity mask alone. At startup of the interrupt
>> affinity will be assigned out of the affinity mask and the XEN binding will
>> be updated. 
>
> If that's the case then I wonder whether we need this call at all and
> instead bind at startup time.

I was wondering about that, but my knowledge about the Xen internal
requirements is pretty limited. The current set at least survived basic
testing by Jürgen.

Thanks,

        tglx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 27/30] xen/events: Only force affinity mask for percpu interrupts
  2020-12-10 23:20   ` boris.ostrovsky
  2020-12-11  0:06     ` Thomas Gleixner
@ 2020-12-11  6:17     ` Jürgen Groß
       [not found]       ` <874kksiras.fsf@nanos.tec.linutronix.de>
  2020-12-11 12:10     ` Jürgen Groß
  2 siblings, 1 reply; 73+ messages in thread
From: Jürgen Groß @ 2020-12-11  6:17 UTC (permalink / raw)
  To: boris.ostrovsky, Thomas Gleixner, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Jakub Kicinski, Will Deacon,
	Michal Simek, linux-s390, afzal mohammed, Stefano Stabellini,
	Dave Jiang, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	xen-devel, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	David Airlie, linux-gpio, Lorenzo Pieralisi, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Tvrtko Ursulin,
	linux-parisc, Pankaj Bharadiya, Hou Zhiqiang, Tariq Toukan,
	Jon Mason, linux-ntb, intel-gfx, David S. Miller

[-- Attachment #1.1.1.1: Type: text/plain, Size: 1106 bytes --]

On 11.12.20 00:20, boris.ostrovsky@oracle.com wrote:
> 
> On 12/10/20 2:26 PM, Thomas Gleixner wrote:
>> All event channel setups bind the interrupt on CPU0 or the target CPU for
>> percpu interrupts and overwrite the affinity mask with the corresponding
>> cpumask. That does not make sense.
>>
>> The XEN implementation of irqchip::irq_set_affinity() already picks a
>> single target CPU out of the affinity mask and the actual target is stored
>> in the effective CPU mask, so destroying the user chosen affinity mask
>> which might contain more than one CPU is wrong.
>>
>> Change the implementation so that the channel is bound to CPU0 at the XEN
>> level and leave the affinity mask alone. At startup of the interrupt
>> affinity will be assigned out of the affinity mask and the XEN binding will
>> be updated.
> 
> 
> If that's the case then I wonder whether we need this call at all and instead bind at startup time.

This binding to cpu0 was introduced with commit 97253eeeb792d61ed2
and I have no reason to believe the underlying problem has been
eliminated.


Juergen

[-- Attachment #1.1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 16/30] mfd: ab8500-debugfs: Remove the racy fiddling with irq_desc
  2020-12-10 19:25 ` [patch 16/30] mfd: ab8500-debugfs: Remove the racy fiddling with irq_desc Thomas Gleixner
@ 2020-12-11  8:22   ` Linus Walleij
  2020-12-11 10:04   ` Lee Jones
  2020-12-11 18:12   ` Andy Shevchenko
  2 siblings, 0 replies; 73+ messages in thread
From: Linus Walleij @ 2020-12-11  8:22 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, open list:DRM PANEL DRIVERS, Chris Wilson,
	James E.J. Bottomley, Saeed Mahameed, netdev, afzal mohammed,
	Michal Simek, linux-s390, Lorenzo Pieralisi, Dave Jiang,
	xen-devel, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	Jakub Kicinski, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	Juergen Gross, Will Deacon, David Airlie,
	open list:GPIO SUBSYSTEM, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, Linux ARM, Boris Ostrovsky,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	LKML, Tariq Toukan, Jon Mason, linux-ntb, intel-gfx,
	David S. Miller

On Thu, Dec 10, 2020 at 8:42 PM Thomas Gleixner <tglx@linutronix.de> wrote:

> First of all drivers have absolutely no business to dig into the internals
> of an irq descriptor. That's core code and subject to change. All of this
> information is readily available to /proc/interrupts in a safe and race
> free way.
>
> Remove the inspection code which is a blatant violation of subsystem
> boundaries and racy against concurrent modifications of the interrupt
> descriptor.
>
> Print the irq line instead so the information can be looked up in a sane
> way in /proc/interrupts.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: linux-arm-kernel@lists.infradead.org

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [Intel-gfx] [patch 13/30] drm/i915/lpe_audio: Remove pointless irq_to_desc() usage
  2020-12-10 19:48   ` [Intel-gfx] " Ville Syrjälä
@ 2020-12-11  9:51     ` Jani Nikula
  0 siblings, 0 replies; 73+ messages in thread
From: Jani Nikula @ 2020-12-11  9:51 UTC (permalink / raw)
  To: Ville Syrjälä, Thomas Gleixner
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	afzal mohammed, Michal Simek, Lorenzo Pieralisi, Dave Jiang,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, linux-pci, xen-devel,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Will Deacon,
	linux-s390, intel-gfx, linux-gpio, Stefano Stabellini,
	Jakub Kicinski, Bjorn Helgaas, Boris Ostrovsky, linux-arm-kernel,
	Juergen Gross, David Airlie, Lee Jones, linux-parisc, netdev,
	Hou Zhiqiang, LKML, Tariq Toukan, Jon Mason, linux-ntb,
	Saeed Mahameed, David S. Miller

On Thu, 10 Dec 2020, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Dec 10, 2020 at 08:25:49PM +0100, Thomas Gleixner wrote:
>> Nothing uses the result and nothing should ever use it in driver code.
>> 
>> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: David Airlie <airlied@linux.ie>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Cc: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Wambui Karuga <wambui.karugax@gmail.com>
>> Cc: intel-gfx@lists.freedesktop.org
>> Cc: dri-devel@lists.freedesktop.org
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thomas, I presume you want to merge this series as a whole.

Acked-by: Jani Nikula <jani.nikula@intel.com>

for merging via whichever tree makes most sense. Please let us know if
you want us to pick this up via drm-intel instead.

>
>> ---
>>  drivers/gpu/drm/i915/display/intel_lpe_audio.c |    4 ----
>>  1 file changed, 4 deletions(-)
>> 
>> --- a/drivers/gpu/drm/i915/display/intel_lpe_audio.c
>> +++ b/drivers/gpu/drm/i915/display/intel_lpe_audio.c
>> @@ -297,13 +297,9 @@ int intel_lpe_audio_init(struct drm_i915
>>   */
>>  void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv)
>>  {
>> -	struct irq_desc *desc;
>> -
>>  	if (!HAS_LPE_AUDIO(dev_priv))
>>  		return;
>>  
>> -	desc = irq_to_desc(dev_priv->lpe_audio.irq);
>> -
>>  	lpe_audio_platdev_destroy(dev_priv);
>>  
>>  	irq_free_desc(dev_priv->lpe_audio.irq);
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 14/30] drm/i915/pmu: Replace open coded kstat_irqs() copy
  2020-12-10 19:25 ` [patch 14/30] drm/i915/pmu: Replace open coded kstat_irqs() copy Thomas Gleixner
@ 2020-12-11  9:54   ` Jani Nikula
  2020-12-11 10:13   ` Tvrtko Ursulin
  1 sibling, 0 replies; 73+ messages in thread
From: Jani Nikula @ 2020-12-11  9:54 UTC (permalink / raw)
  To: Thomas Gleixner, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	netdev, David Airlie, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, linux-pci, Jakub Kicinski,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Juergen Gross,
	intel-gfx, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, Saeed Mahameed,
	David S. Miller

On Thu, 10 Dec 2020, Thomas Gleixner <tglx@linutronix.de> wrote:
> Driver code has no business with the internals of the irq descriptor.
>
> Aside of that the count is per interrupt line and therefore takes
> interrupts from other devices into account which share the interrupt line
> and are not handled by the graphics driver.
>
> Replace it with a pmu private count which only counts interrupts which
> originate from the graphics card.
>
> To avoid atomics or heuristics of some sort make the counter field
> 'unsigned long'. That limits the count to 4e9 on 32bit which is a lot and
> postprocessing can easily deal with the occasional wraparound.

I'll let Tvrtko and Chris review the substance here, but assuming they
don't object,

Acked-by: Jani Nikula <jani.nikula@intel.com>

for merging via whichever tree makes most sense.

>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/i915/i915_irq.c |   34 ++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_pmu.c |   18 +-----------------
>  drivers/gpu/drm/i915/i915_pmu.h |    8 ++++++++
>  3 files changed, 43 insertions(+), 17 deletions(-)
>
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -60,6 +60,24 @@
>   * and related files, but that will be described in separate chapters.
>   */
>  
> +/*
> + * Interrupt statistic for PMU. Increments the counter only if the
> + * interrupt originated from the the GPU so interrupts from a device which
> + * shares the interrupt line are not accounted.
> + */
> +static inline void pmu_irq_stats(struct drm_i915_private *priv,
> +				 irqreturn_t res)
> +{
> +	if (unlikely(res != IRQ_HANDLED))
> +		return;
> +
> +	/*
> +	 * A clever compiler translates that into INC. A not so clever one
> +	 * should at least prevent store tearing.
> +	 */
> +	WRITE_ONCE(priv->pmu.irq_count, priv->pmu.irq_count + 1);
> +}
> +
>  typedef bool (*long_pulse_detect_func)(enum hpd_pin pin, u32 val);
>  
>  static const u32 hpd_ilk[HPD_NUM_PINS] = {
> @@ -1599,6 +1617,8 @@ static irqreturn_t valleyview_irq_handle
>  		valleyview_pipestat_irq_handler(dev_priv, pipe_stats);
>  	} while (0);
>  
> +	pmu_irq_stats(dev_priv, ret);
> +
>  	enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>  
>  	return ret;
> @@ -1676,6 +1696,8 @@ static irqreturn_t cherryview_irq_handle
>  		valleyview_pipestat_irq_handler(dev_priv, pipe_stats);
>  	} while (0);
>  
> +	pmu_irq_stats(dev_priv, ret);
> +
>  	enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>  
>  	return ret;
> @@ -2103,6 +2125,8 @@ static irqreturn_t ilk_irq_handler(int i
>  	if (sde_ier)
>  		raw_reg_write(regs, SDEIER, sde_ier);
>  
> +	pmu_irq_stats(i915, ret);
> +
>  	/* IRQs are synced during runtime_suspend, we don't require a wakeref */
>  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
>  
> @@ -2419,6 +2443,8 @@ static irqreturn_t gen8_irq_handler(int
>  
>  	gen8_master_intr_enable(regs);
>  
> +	pmu_irq_stats(dev_priv, IRQ_HANDLED);
> +
>  	return IRQ_HANDLED;
>  }
>  
> @@ -2514,6 +2540,8 @@ static __always_inline irqreturn_t
>  
>  	gen11_gu_misc_irq_handler(gt, gu_misc_iir);
>  
> +	pmu_irq_stats(i915, IRQ_HANDLED);
> +
>  	return IRQ_HANDLED;
>  }
>  
> @@ -3688,6 +3716,8 @@ static irqreturn_t i8xx_irq_handler(int
>  		i8xx_pipestat_irq_handler(dev_priv, iir, pipe_stats);
>  	} while (0);
>  
> +	pmu_irq_stats(dev_priv, ret);
> +
>  	enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>  
>  	return ret;
> @@ -3796,6 +3826,8 @@ static irqreturn_t i915_irq_handler(int
>  		i915_pipestat_irq_handler(dev_priv, iir, pipe_stats);
>  	} while (0);
>  
> +	pmu_irq_stats(dev_priv, ret);
> +
>  	enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>  
>  	return ret;
> @@ -3941,6 +3973,8 @@ static irqreturn_t i965_irq_handler(int
>  		i965_pipestat_irq_handler(dev_priv, iir, pipe_stats);
>  	} while (0);
>  
> +	pmu_irq_stats(dev_priv, IRQ_HANDLED);
> +
>  	enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>  
>  	return ret;
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -423,22 +423,6 @@ static enum hrtimer_restart i915_sample(
>  	return HRTIMER_RESTART;
>  }
>  
> -static u64 count_interrupts(struct drm_i915_private *i915)
> -{
> -	/* open-coded kstat_irqs() */
> -	struct irq_desc *desc = irq_to_desc(i915->drm.pdev->irq);
> -	u64 sum = 0;
> -	int cpu;
> -
> -	if (!desc || !desc->kstat_irqs)
> -		return 0;
> -
> -	for_each_possible_cpu(cpu)
> -		sum += *per_cpu_ptr(desc->kstat_irqs, cpu);
> -
> -	return sum;
> -}
> -
>  static void i915_pmu_event_destroy(struct perf_event *event)
>  {
>  	struct drm_i915_private *i915 =
> @@ -581,7 +565,7 @@ static u64 __i915_pmu_event_read(struct
>  				   USEC_PER_SEC /* to MHz */);
>  			break;
>  		case I915_PMU_INTERRUPTS:
> -			val = count_interrupts(i915);
> +			val = READ_ONCE(pmu->irq_count);
>  			break;
>  		case I915_PMU_RC6_RESIDENCY:
>  			val = get_rc6(&i915->gt);
> --- a/drivers/gpu/drm/i915/i915_pmu.h
> +++ b/drivers/gpu/drm/i915/i915_pmu.h
> @@ -108,6 +108,14 @@ struct i915_pmu {
>  	 */
>  	ktime_t sleep_last;
>  	/**
> +	 * @irq_count: Number of interrupts
> +	 *
> +	 * Intentionally unsigned long to avoid atomics or heuristics on 32bit.
> +	 * 4e9 interrupts are a lot and postprocessing can really deal with an
> +	 * occasional wraparound easily. It's 32bit after all.
> +	 */
> +	unsigned long irq_count;
> +	/**
>  	 * @events_attr_group: Device events attribute group.
>  	 */
>  	struct attribute_group events_attr_group;
>

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 16/30] mfd: ab8500-debugfs: Remove the racy fiddling with irq_desc
  2020-12-10 19:25 ` [patch 16/30] mfd: ab8500-debugfs: Remove the racy fiddling with irq_desc Thomas Gleixner
  2020-12-11  8:22   ` Linus Walleij
@ 2020-12-11 10:04   ` Lee Jones
  2020-12-11 18:12   ` Andy Shevchenko
  2 siblings, 0 replies; 73+ messages in thread
From: Lee Jones @ 2020-12-11 10:04 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	netdev, Will Deacon, Michal Simek, linux-s390, afzal mohammed,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Marc Zyngier, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, intel-gfx,
	Wambui Karuga, Allen Hubbe, Tvrtko Ursulin, Heiko Carstens,
	linux-gpio, Stefano Stabellini, Rodrigo Vivi, Bjorn Helgaas,
	Boris Ostrovsky, linux-arm-kernel, Juergen Gross, David Airlie,
	linux-parisc, Pankaj Bharadiya, Hou Zhiqiang, LKML, Tariq Toukan,
	Jon Mason, linux-ntb, Saeed Mahameed, David S. Miller

On Thu, 10 Dec 2020, Thomas Gleixner wrote:

> First of all drivers have absolutely no business to dig into the internals
> of an irq descriptor. That's core code and subject to change. All of this
> information is readily available to /proc/interrupts in a safe and race
> free way.
> 
> Remove the inspection code which is a blatant violation of subsystem
> boundaries and racy against concurrent modifications of the interrupt
> descriptor.
> 
> Print the irq line instead so the information can be looked up in a sane
> way in /proc/interrupts.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  drivers/mfd/ab8500-debugfs.c |   16 +++-------------
>  1 file changed, 3 insertions(+), 13 deletions(-)

Acked-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 14/30] drm/i915/pmu: Replace open coded kstat_irqs() copy
  2020-12-10 19:25 ` [patch 14/30] drm/i915/pmu: Replace open coded kstat_irqs() copy Thomas Gleixner
  2020-12-11  9:54   ` Jani Nikula
@ 2020-12-11 10:13   ` Tvrtko Ursulin
  2020-12-11 12:57     ` Thomas Gleixner
  1 sibling, 1 reply; 73+ messages in thread
From: Tvrtko Ursulin @ 2020-12-11 10:13 UTC (permalink / raw)
  To: Thomas Gleixner, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	netdev, Will Deacon, Michal Simek, linux-s390, afzal mohammed,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Marc Zyngier, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, Heiko Carstens,
	Wambui Karuga, Allen Hubbe, Juergen Gross, intel-gfx, linux-gpio,
	Stefano Stabellini, Rodrigo Vivi, Bjorn Helgaas, Lee Jones,
	linux-arm-kernel, Boris Ostrovsky, David Airlie, linux-parisc,
	Pankaj Bharadiya, Hou Zhiqiang, Tariq Toukan, Jon Mason,
	linux-ntb, Saeed Mahameed, David S. Miller


On 10/12/2020 19:25, Thomas Gleixner wrote:
> Driver code has no business with the internals of the irq descriptor.
> 
> Aside of that the count is per interrupt line and therefore takes
> interrupts from other devices into account which share the interrupt line
> and are not handled by the graphics driver.
> 
> Replace it with a pmu private count which only counts interrupts which
> originate from the graphics card.
> 
> To avoid atomics or heuristics of some sort make the counter field
> 'unsigned long'. That limits the count to 4e9 on 32bit which is a lot and
> postprocessing can easily deal with the occasional wraparound.

After my failed hasty sketch from last night I had a different one which 
was kind of heuristics based (re-reading the upper dword and retrying if 
it changed on 32-bit). But you are right - it is okay to at least start 
like this today and if later there is a need we can either do that or 
deal with wrap at PMU read time.

So thanks for dealing with it, some small comments below but overall it 
is fine.

> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> ---
>   drivers/gpu/drm/i915/i915_irq.c |   34 ++++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/i915/i915_pmu.c |   18 +-----------------
>   drivers/gpu/drm/i915/i915_pmu.h |    8 ++++++++
>   3 files changed, 43 insertions(+), 17 deletions(-)
> 
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -60,6 +60,24 @@
>    * and related files, but that will be described in separate chapters.
>    */
>   
> +/*
> + * Interrupt statistic for PMU. Increments the counter only if the
> + * interrupt originated from the the GPU so interrupts from a device which
> + * shares the interrupt line are not accounted.
> + */
> +static inline void pmu_irq_stats(struct drm_i915_private *priv,

We never use priv as a local name, it should be either i915 or dev_priv.

> +				 irqreturn_t res)
> +{
> +	if (unlikely(res != IRQ_HANDLED))
> +		return;
> +
> +	/*
> +	 * A clever compiler translates that into INC. A not so clever one
> +	 * should at least prevent store tearing.
> +	 */
> +	WRITE_ONCE(priv->pmu.irq_count, priv->pmu.irq_count + 1);

Curious, probably more educational for me - given x86_32 and x86_64, and 
the context of it getting called, what is the difference from just doing 
irq_count++?

> +}
> +
>   typedef bool (*long_pulse_detect_func)(enum hpd_pin pin, u32 val);
>   
>   static const u32 hpd_ilk[HPD_NUM_PINS] = {
> @@ -1599,6 +1617,8 @@ static irqreturn_t valleyview_irq_handle
>   		valleyview_pipestat_irq_handler(dev_priv, pipe_stats);
>   	} while (0);
>   
> +	pmu_irq_stats(dev_priv, ret);
> +
>   	enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>   
>   	return ret;
> @@ -1676,6 +1696,8 @@ static irqreturn_t cherryview_irq_handle
>   		valleyview_pipestat_irq_handler(dev_priv, pipe_stats);
>   	} while (0);
>   
> +	pmu_irq_stats(dev_priv, ret);
> +
>   	enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>   
>   	return ret;
> @@ -2103,6 +2125,8 @@ static irqreturn_t ilk_irq_handler(int i
>   	if (sde_ier)
>   		raw_reg_write(regs, SDEIER, sde_ier);
>   
> +	pmu_irq_stats(i915, ret);
> +
>   	/* IRQs are synced during runtime_suspend, we don't require a wakeref */
>   	enable_rpm_wakeref_asserts(&i915->runtime_pm);
>   
> @@ -2419,6 +2443,8 @@ static irqreturn_t gen8_irq_handler(int
>   
>   	gen8_master_intr_enable(regs);
>   
> +	pmu_irq_stats(dev_priv, IRQ_HANDLED);
> +
>   	return IRQ_HANDLED;
>   }
>   
> @@ -2514,6 +2540,8 @@ static __always_inline irqreturn_t
>   
>   	gen11_gu_misc_irq_handler(gt, gu_misc_iir);
>   
> +	pmu_irq_stats(i915, IRQ_HANDLED);
> +
>   	return IRQ_HANDLED;
>   }
>   
> @@ -3688,6 +3716,8 @@ static irqreturn_t i8xx_irq_handler(int
>   		i8xx_pipestat_irq_handler(dev_priv, iir, pipe_stats);
>   	} while (0);
>   
> +	pmu_irq_stats(dev_priv, ret);
> +
>   	enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>   
>   	return ret;
> @@ -3796,6 +3826,8 @@ static irqreturn_t i915_irq_handler(int
>   		i915_pipestat_irq_handler(dev_priv, iir, pipe_stats);
>   	} while (0);
>   
> +	pmu_irq_stats(dev_priv, ret);
> +
>   	enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>   
>   	return ret;
> @@ -3941,6 +3973,8 @@ static irqreturn_t i965_irq_handler(int
>   		i965_pipestat_irq_handler(dev_priv, iir, pipe_stats);
>   	} while (0);
>   
> +	pmu_irq_stats(dev_priv, IRQ_HANDLED);
> +
>   	enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>   
>   	return ret;
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -423,22 +423,6 @@ static enum hrtimer_restart i915_sample(
>   	return HRTIMER_RESTART;
>   }

In this file you can also drop the #include <linux/irq.h> line.

>   
> -static u64 count_interrupts(struct drm_i915_private *i915)
> -{
> -	/* open-coded kstat_irqs() */
> -	struct irq_desc *desc = irq_to_desc(i915->drm.pdev->irq);
> -	u64 sum = 0;
> -	int cpu;
> -
> -	if (!desc || !desc->kstat_irqs)
> -		return 0;
> -
> -	for_each_possible_cpu(cpu)
> -		sum += *per_cpu_ptr(desc->kstat_irqs, cpu);
> -
> -	return sum;
> -}
> -
>   static void i915_pmu_event_destroy(struct perf_event *event)
>   {
>   	struct drm_i915_private *i915 =
> @@ -581,7 +565,7 @@ static u64 __i915_pmu_event_read(struct
>   				   USEC_PER_SEC /* to MHz */);
>   			break;
>   		case I915_PMU_INTERRUPTS:
> -			val = count_interrupts(i915);
> +			val = READ_ONCE(pmu->irq_count);

I guess same curiosity about READ_ONCE like in the increment site.

>   			break;
>   		case I915_PMU_RC6_RESIDENCY:
>   			val = get_rc6(&i915->gt);
> --- a/drivers/gpu/drm/i915/i915_pmu.h
> +++ b/drivers/gpu/drm/i915/i915_pmu.h
> @@ -108,6 +108,14 @@ struct i915_pmu {
>   	 */
>   	ktime_t sleep_last;
>   	/**
> +	 * @irq_count: Number of interrupts
> +	 *
> +	 * Intentionally unsigned long to avoid atomics or heuristics on 32bit.
> +	 * 4e9 interrupts are a lot and postprocessing can really deal with an
> +	 * occasional wraparound easily. It's 32bit after all.
> +	 */
> +	unsigned long irq_count;
> +	/**
>   	 * @events_attr_group: Device events attribute group.
>   	 */
>   	struct attribute_group events_attr_group;
> 

Regards,

Tvrtko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 27/30] xen/events: Only force affinity mask for percpu interrupts
       [not found]       ` <874kksiras.fsf@nanos.tec.linutronix.de>
@ 2020-12-11 10:23         ` Jürgen Groß
  0 siblings, 0 replies; 73+ messages in thread
From: Jürgen Groß @ 2020-12-11 10:23 UTC (permalink / raw)
  To: Thomas Gleixner, boris.ostrovsky, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Jakub Kicinski, Will Deacon,
	Michal Simek, linux-s390, afzal mohammed, Stefano Stabellini,
	Dave Jiang, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	xen-devel, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	David Airlie, linux-gpio, Lorenzo Pieralisi, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Tvrtko Ursulin,
	linux-parisc, Pankaj Bharadiya, Hou Zhiqiang, Tariq Toukan,
	Jon Mason, linux-ntb, intel-gfx, David S. Miller

[-- Attachment #1.1.1.1: Type: text/plain, Size: 2388 bytes --]

On 11.12.20 11:13, Thomas Gleixner wrote:
> On Fri, Dec 11 2020 at 07:17, Jürgen Groß wrote:
>> On 11.12.20 00:20, boris.ostrovsky@oracle.com wrote:
>>>
>>> On 12/10/20 2:26 PM, Thomas Gleixner wrote:
>>>> All event channel setups bind the interrupt on CPU0 or the target CPU for
>>>> percpu interrupts and overwrite the affinity mask with the corresponding
>>>> cpumask. That does not make sense.
>>>>
>>>> The XEN implementation of irqchip::irq_set_affinity() already picks a
>>>> single target CPU out of the affinity mask and the actual target is stored
>>>> in the effective CPU mask, so destroying the user chosen affinity mask
>>>> which might contain more than one CPU is wrong.
>>>>
>>>> Change the implementation so that the channel is bound to CPU0 at the XEN
>>>> level and leave the affinity mask alone. At startup of the interrupt
>>>> affinity will be assigned out of the affinity mask and the XEN binding will
>>>> be updated.
>>>
>>>
>>> If that's the case then I wonder whether we need this call at all and instead bind at startup time.
>>
>> This binding to cpu0 was introduced with commit 97253eeeb792d61ed2
>> and I have no reason to believe the underlying problem has been
>> eliminated.
> 
>      "The kernel-side VCPU binding was not being correctly set for newly
>       allocated or bound interdomain events.  In ARM guests where 2-level
>       events were used, this would result in no interdomain events being
>       handled because the kernel-side VCPU masks would all be clear.
> 
>       x86 guests would work because the irq affinity was set during irq
>       setup and this would set the correct kernel-side VCPU binding."
> 
> I'm not convinced that this is really correctly analyzed because affinity
> setting is done at irq startup.
> 
>                  switch (__irq_startup_managed(desc, aff, force)) {
> 	        case IRQ_STARTUP_NORMAL:
> 	                ret = __irq_startup(desc);
>                          irq_setup_affinity(desc);
> 			break;
> 
> which is completely architecture agnostic. So why should this magically
> work on x86 and not on ARM if both are using the same XEN irqchip with
> the same irqchip callbacks.

I think this might be related to _initial_ cpu binding of events and
changing the binding later. This might be handled differently in the
hypervisor.


Juergen

[-- Attachment #1.1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 27/30] xen/events: Only force affinity mask for percpu interrupts
  2020-12-10 23:20   ` boris.ostrovsky
  2020-12-11  0:06     ` Thomas Gleixner
  2020-12-11  6:17     ` Jürgen Groß
@ 2020-12-11 12:10     ` Jürgen Groß
  2020-12-11 12:37       ` Thomas Gleixner
  2 siblings, 1 reply; 73+ messages in thread
From: Jürgen Groß @ 2020-12-11 12:10 UTC (permalink / raw)
  To: boris.ostrovsky, Thomas Gleixner, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Jakub Kicinski, Will Deacon,
	Michal Simek, linux-s390, afzal mohammed, Stefano Stabellini,
	Dave Jiang, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	xen-devel, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	David Airlie, linux-gpio, Lorenzo Pieralisi, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Tvrtko Ursulin,
	linux-parisc, Pankaj Bharadiya, Hou Zhiqiang, Tariq Toukan,
	Jon Mason, linux-ntb, intel-gfx, David S. Miller

[-- Attachment #1.1.1.1: Type: text/plain, Size: 1223 bytes --]

On 11.12.20 00:20, boris.ostrovsky@oracle.com wrote:
> 
> On 12/10/20 2:26 PM, Thomas Gleixner wrote:
>> All event channel setups bind the interrupt on CPU0 or the target CPU for
>> percpu interrupts and overwrite the affinity mask with the corresponding
>> cpumask. That does not make sense.
>>
>> The XEN implementation of irqchip::irq_set_affinity() already picks a
>> single target CPU out of the affinity mask and the actual target is stored
>> in the effective CPU mask, so destroying the user chosen affinity mask
>> which might contain more than one CPU is wrong.
>>
>> Change the implementation so that the channel is bound to CPU0 at the XEN
>> level and leave the affinity mask alone. At startup of the interrupt
>> affinity will be assigned out of the affinity mask and the XEN binding will
>> be updated.
> 
> 
> If that's the case then I wonder whether we need this call at all and instead bind at startup time.

After some discussion with Thomas on IRC and xen-devel archaeology the
result is: this will be needed especially for systems running on a
single vcpu (e.g. small guests), as the .irq_set_affinity() callback
won't be called in this case when starting the irq.


Juergen

[-- Attachment #1.1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 27/30] xen/events: Only force affinity mask for percpu interrupts
  2020-12-11 12:10     ` Jürgen Groß
@ 2020-12-11 12:37       ` Thomas Gleixner
  2020-12-11 14:29         ` boris.ostrovsky
  0 siblings, 1 reply; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-11 12:37 UTC (permalink / raw)
  To: Jürgen Groß, boris.ostrovsky, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Jakub Kicinski, Will Deacon,
	Michal Simek, linux-s390, afzal mohammed, Stefano Stabellini,
	Dave Jiang, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	xen-devel, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	David Airlie, linux-gpio, Lorenzo Pieralisi, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Tvrtko Ursulin,
	linux-parisc, Pankaj Bharadiya, Hou Zhiqiang, Tariq Toukan,
	Jon Mason, linux-ntb, intel-gfx, David S. Miller

On Fri, Dec 11 2020 at 13:10, Jürgen Groß wrote:
> On 11.12.20 00:20, boris.ostrovsky@oracle.com wrote:
>> 
>> On 12/10/20 2:26 PM, Thomas Gleixner wrote:
>>> All event channel setups bind the interrupt on CPU0 or the target CPU for
>>> percpu interrupts and overwrite the affinity mask with the corresponding
>>> cpumask. That does not make sense.
>>>
>>> The XEN implementation of irqchip::irq_set_affinity() already picks a
>>> single target CPU out of the affinity mask and the actual target is stored
>>> in the effective CPU mask, so destroying the user chosen affinity mask
>>> which might contain more than one CPU is wrong.
>>>
>>> Change the implementation so that the channel is bound to CPU0 at the XEN
>>> level and leave the affinity mask alone. At startup of the interrupt
>>> affinity will be assigned out of the affinity mask and the XEN binding will
>>> be updated.
>> 
>> 
>> If that's the case then I wonder whether we need this call at all and instead bind at startup time.
>
> After some discussion with Thomas on IRC and xen-devel archaeology the
> result is: this will be needed especially for systems running on a
> single vcpu (e.g. small guests), as the .irq_set_affinity() callback
> won't be called in this case when starting the irq.

That's right, but not limited to ARM. The same problem exists on x86 UP.
So yes, the call makes sense, but the changelog is not really useful.
Let me add a comment to this.

Thanks,

        tglx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 14/30] drm/i915/pmu: Replace open coded kstat_irqs() copy
  2020-12-11 10:13   ` Tvrtko Ursulin
@ 2020-12-11 12:57     ` Thomas Gleixner
  2020-12-11 14:19       ` David Laight
  0 siblings, 1 reply; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-11 12:57 UTC (permalink / raw)
  To: Tvrtko Ursulin, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	netdev, Will Deacon, Michal Simek, linux-s390, afzal mohammed,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Marc Zyngier, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, Heiko Carstens,
	Wambui Karuga, Allen Hubbe, Juergen Gross, intel-gfx, linux-gpio,
	Stefano Stabellini, Rodrigo Vivi, Bjorn Helgaas, Lee Jones,
	linux-arm-kernel, Boris Ostrovsky, David Airlie, linux-parisc,
	Pankaj Bharadiya, Hou Zhiqiang, Tariq Toukan, Jon Mason,
	linux-ntb, Saeed Mahameed, David S. Miller

On Fri, Dec 11 2020 at 10:13, Tvrtko Ursulin wrote:
> On 10/12/2020 19:25, Thomas Gleixner wrote:

>> 
>> Aside of that the count is per interrupt line and therefore takes
>> interrupts from other devices into account which share the interrupt line
>> and are not handled by the graphics driver.
>> 
>> Replace it with a pmu private count which only counts interrupts which
>> originate from the graphics card.
>> 
>> To avoid atomics or heuristics of some sort make the counter field
>> 'unsigned long'. That limits the count to 4e9 on 32bit which is a lot and
>> postprocessing can easily deal with the occasional wraparound.
>
> After my failed hasty sketch from last night I had a different one which 
> was kind of heuristics based (re-reading the upper dword and retrying if 
> it changed on 32-bit).

The problem is that there will be two seperate modifications for the low
and high word. Several ways how the compiler can translate this, but the
problem is the same for all of them:

CPU 0                           CPU 1
        load low
        load high
        add  low, 1
        addc high, 0            
        store low               load high
--> NMI                         load low
                                load high and compare
        store high

You can't catch that. If this really becomes an issue you need a
sequence counter around it.
      

> But you are right - it is okay to at least start 
> like this today and if later there is a need we can either do that or 
> deal with wrap at PMU read time.

Right.

>> +/*
>> + * Interrupt statistic for PMU. Increments the counter only if the
>> + * interrupt originated from the the GPU so interrupts from a device which
>> + * shares the interrupt line are not accounted.
>> + */
>> +static inline void pmu_irq_stats(struct drm_i915_private *priv,
>
> We never use priv as a local name, it should be either i915 or
> dev_priv.

Sure, will fix.

>> +	/*
>> +	 * A clever compiler translates that into INC. A not so clever one
>> +	 * should at least prevent store tearing.
>> +	 */
>> +	WRITE_ONCE(priv->pmu.irq_count, priv->pmu.irq_count + 1);
>
> Curious, probably more educational for me - given x86_32 and x86_64, and 
> the context of it getting called, what is the difference from just doing 
> irq_count++?

Several reasons:

    1) The compiler can pretty much do what it wants with cnt++
       including tearing and whatever. https://lwn.net/Articles/816850/
       for the full set of insanities.

       Not really a problem here, but

    2) It's annotating the reader and the writer side and documenting
       that this is subject to concurrency

    3) It will prevent KCSAN to complain about the data race,
       i.e. concurrent modification while reading.

Thanks,

        tglx

>> --- a/drivers/gpu/drm/i915/i915_pmu.c
>> +++ b/drivers/gpu/drm/i915/i915_pmu.c
>> @@ -423,22 +423,6 @@ static enum hrtimer_restart i915_sample(
>>   	return HRTIMER_RESTART;
>>   }
>
> In this file you can also drop the #include <linux/irq.h> line.

Indeed.

Thanks,

        tglx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* RE: [patch 14/30] drm/i915/pmu: Replace open coded kstat_irqs() copy
  2020-12-11 12:57     ` Thomas Gleixner
@ 2020-12-11 14:19       ` David Laight
  2020-12-11 21:10         ` Thomas Gleixner
  0 siblings, 1 reply; 73+ messages in thread
From: David Laight @ 2020-12-11 14:19 UTC (permalink / raw)
  To: 'Thomas Gleixner', Tvrtko Ursulin, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	netdev, Will Deacon, Michal Simek, linux-s390, afzal mohammed,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Marc Zyngier, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, Heiko Carstens,
	Wambui Karuga, Allen Hubbe, Juergen Gross, intel-gfx, linux-gpio,
	Stefano Stabellini, Rodrigo Vivi, Bjorn Helgaas, Lee Jones,
	linux-arm-kernel, Boris Ostrovsky, David Airlie, linux-parisc,
	Pankaj Bharadiya, Hou Zhiqiang, Tariq Toukan, Jon Mason,
	linux-ntb, Saeed Mahameed, David S. Miller

From: Thomas Gleixner
> Sent: 11 December 2020 12:58
..
> > After my failed hasty sketch from last night I had a different one which
> > was kind of heuristics based (re-reading the upper dword and retrying if
> > it changed on 32-bit).
> 
> The problem is that there will be two seperate modifications for the low
> and high word. Several ways how the compiler can translate this, but the
> problem is the same for all of them:
> 
> CPU 0                           CPU 1
>         load low
>         load high
>         add  low, 1
>         addc high, 0
>         store low               load high
> --> NMI                         load low
>                                 load high and compare
>         store high
> 
> You can't catch that. If this really becomes an issue you need a
> sequence counter around it.

Or just two copies of the high word.
Provided the accesses are sequenced:
writer:
	load high:low
	add small_value,high:low
	store high
	store low
	store high_copy
reader:
	load high_copy
	load low
	load high
	if (high != high_copy)
		low = 0;

The read value is always stale, so it probably doesn't
matter that the value you have is one that is between the
value when you started and that when you finished.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 27/30] xen/events: Only force affinity mask for percpu interrupts
  2020-12-11 12:37       ` Thomas Gleixner
@ 2020-12-11 14:29         ` boris.ostrovsky
  2020-12-11 21:27           ` Thomas Gleixner
  0 siblings, 1 reply; 73+ messages in thread
From: boris.ostrovsky @ 2020-12-11 14:29 UTC (permalink / raw)
  To: Thomas Gleixner, Jürgen Groß, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Jakub Kicinski, Will Deacon,
	Michal Simek, linux-s390, afzal mohammed, Stefano Stabellini,
	Dave Jiang, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	xen-devel, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	David Airlie, linux-gpio, Lorenzo Pieralisi, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Tvrtko Ursulin,
	linux-parisc, Pankaj Bharadiya, Hou Zhiqiang, Tariq Toukan,
	Jon Mason, linux-ntb, intel-gfx, David S. Miller


On 12/11/20 7:37 AM, Thomas Gleixner wrote:
> On Fri, Dec 11 2020 at 13:10, Jürgen Groß wrote:
>> On 11.12.20 00:20, boris.ostrovsky@oracle.com wrote:
>>> On 12/10/20 2:26 PM, Thomas Gleixner wrote:
>>>> All event channel setups bind the interrupt on CPU0 or the target CPU for
>>>> percpu interrupts and overwrite the affinity mask with the corresponding
>>>> cpumask. That does not make sense.
>>>>
>>>> The XEN implementation of irqchip::irq_set_affinity() already picks a
>>>> single target CPU out of the affinity mask and the actual target is stored
>>>> in the effective CPU mask, so destroying the user chosen affinity mask
>>>> which might contain more than one CPU is wrong.
>>>>
>>>> Change the implementation so that the channel is bound to CPU0 at the XEN
>>>> level and leave the affinity mask alone. At startup of the interrupt
>>>> affinity will be assigned out of the affinity mask and the XEN binding will
>>>> be updated.
>>>
>>> If that's the case then I wonder whether we need this call at all and instead bind at startup time.
>> After some discussion with Thomas on IRC and xen-devel archaeology the
>> result is: this will be needed especially for systems running on a
>> single vcpu (e.g. small guests), as the .irq_set_affinity() callback
>> won't be called in this case when starting the irq.


On UP are we not then going to end up with an empty affinity mask? Or are we guaranteed to have it set to 1 by interrupt generic code?


This is actually why I brought this up in the first place --- a potential mismatch between the affinity mask and Xen-specific data (e.g. info->cpu and then protocol-specific data in event channel code). Even if they are re-synchronized later, at startup time (for SMP).


I don't see anything that would cause a problem right now but I worry that this inconsistency may come up at some point.


-boris


> That's right, but not limited to ARM. The same problem exists on x86 UP.
> So yes, the call makes sense, but the changelog is not really useful.
> Let me add a comment to this.
>
> Thanks,
>
>         tglx
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 03/30] genirq: Move irq_set_lockdep_class() to core
  2020-12-10 19:25 ` [patch 03/30] genirq: Move irq_set_lockdep_class() " Thomas Gleixner
@ 2020-12-11 17:53   ` Andy Shevchenko
  2020-12-11 21:08     ` Thomas Gleixner
  0 siblings, 1 reply; 73+ messages in thread
From: Andy Shevchenko @ 2020-12-11 17:53 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, open list:HFI1 DRIVER, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	Jakub Kicinski, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	Juergen Gross, David Airlie, open list:GPIO SUBSYSTEM,
	Stefano Stabellini, Rodrigo Vivi, Bjorn Helgaas, Lee Jones,
	linux-arm Mailing List, Boris Ostrovsky, Tvrtko Ursulin,
	linux-parisc, Pankaj Bharadiya, Hou Zhiqiang, LKML, Tariq Toukan,
	Jon Mason, linux-ntb, intel-gfx, David S. Miller

On Thu, Dec 10, 2020 at 10:14 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> irq_set_lockdep_class() is used from modules and requires irq_to_desc() to
> be exported. Move it into the core code which lifts another requirement for
> the export.

...

> +       if (IS_ENABLED(CONFIG_LOCKDEP))
> +               __irq_set_lockdep_class(irq, lock_class, request_class);

Maybe I missed something, but even if the compiler does not warn the
use of if IS_ENABLED() with complimentary #ifdef seems inconsistent.

> +#ifdef CONFIG_LOCKDEP
...
> +EXPORT_SYMBOL_GPL(irq_set_lockdep_class);
> +#endif


-- 
With Best Regards,
Andy Shevchenko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 09/30] ARM: smp: Use irq_desc_kstat_cpu() in show_ipi_list()
  2020-12-10 19:25 ` [patch 09/30] ARM: smp: Use irq_desc_kstat_cpu() in show_ipi_list() Thomas Gleixner
@ 2020-12-11 18:08   ` Marc Zyngier
  0 siblings, 0 replies; 73+ messages in thread
From: Marc Zyngier @ 2020-12-11 18:08 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, afzal mohammed, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, Heiko Carstens,
	Wambui Karuga, Allen Hubbe, Juergen Gross, David Airlie,
	linux-gpio, Stefano Stabellini, Rodrigo Vivi, Bjorn Helgaas,
	Lee Jones, linux-arm-kernel, Boris Ostrovsky, Tvrtko Ursulin,
	linux-parisc, Pankaj Bharadiya, Hou Zhiqiang, LKML, Tariq Toukan,
	Jon Mason, linux-ntb, intel-gfx, David S. Miller

On Thu, 10 Dec 2020 19:25:45 +0000,
Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> The irq descriptor is already there, no need to look it up again.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  arch/arm/kernel/smp.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/arch/arm/kernel/smp.c
> +++ b/arch/arm/kernel/smp.c
> @@ -550,7 +550,7 @@ void show_ipi_list(struct seq_file *p, i
>  		seq_printf(p, "%*s%u: ", prec - 1, "IPI", i);
>  
>  		for_each_online_cpu(cpu)
> -			seq_printf(p, "%10u ", kstat_irqs_cpu(irq, cpu));
> +			seq_printf(p, "%10u ", irq_desc_kstat_cpu(ipi_desc[i], cpu));
>  
>  		seq_printf(p, " %s\n", ipi_types[i]);
>  	}
> 
> 

Acked-by: Marc Zyngier <maz@kernel.org>

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 10/30] arm64/smp: Use irq_desc_kstat_cpu() in arch_show_interrupts()
  2020-12-10 19:25 ` [patch 10/30] arm64/smp: Use irq_desc_kstat_cpu() in arch_show_interrupts() Thomas Gleixner
@ 2020-12-11 18:08   ` Marc Zyngier
  0 siblings, 0 replies; 73+ messages in thread
From: Marc Zyngier @ 2020-12-11 18:08 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, afzal mohammed, Michal Simek, linux-s390,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Will Deacon, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, Heiko Carstens,
	Wambui Karuga, Allen Hubbe, Juergen Gross, David Airlie,
	linux-gpio, Stefano Stabellini, Rodrigo Vivi, Bjorn Helgaas,
	Lee Jones, linux-arm-kernel, Boris Ostrovsky, Tvrtko Ursulin,
	linux-parisc, Pankaj Bharadiya, Hou Zhiqiang, LKML, Tariq Toukan,
	Jon Mason, linux-ntb, intel-gfx, David S. Miller

On Thu, 10 Dec 2020 19:25:46 +0000,
Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> The irq descriptor is already there, no need to look it up again.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  arch/arm64/kernel/smp.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -809,7 +809,7 @@ int arch_show_interrupts(struct seq_file
>  		seq_printf(p, "%*s%u:%s", prec - 1, "IPI", i,
>  			   prec >= 4 ? " " : "");
>  		for_each_online_cpu(cpu)
> -			seq_printf(p, "%10u ", kstat_irqs_cpu(irq, cpu));
> +			seq_printf(p, "%10u ", irq_desc_kstat_cpu(ipi_desc[i], cpu));
>  		seq_printf(p, "      %s\n", ipi_types[i]);
>  	}
>  
> 
> 

Acked-by: Marc Zyngier <maz@kernel.org>

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 16/30] mfd: ab8500-debugfs: Remove the racy fiddling with irq_desc
  2020-12-10 19:25 ` [patch 16/30] mfd: ab8500-debugfs: Remove the racy fiddling with irq_desc Thomas Gleixner
  2020-12-11  8:22   ` Linus Walleij
  2020-12-11 10:04   ` Lee Jones
@ 2020-12-11 18:12   ` Andy Shevchenko
  2 siblings, 0 replies; 73+ messages in thread
From: Andy Shevchenko @ 2020-12-11 18:12 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	netdev, afzal mohammed, Michal Simek, linux-s390,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	open list:HFI1 DRIVER, Marc Zyngier, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, intel-gfx,
	Wambui Karuga, Allen Hubbe, Juergen Gross, Will Deacon,
	Tvrtko Ursulin, Heiko Carstens, open list:GPIO SUBSYSTEM,
	Stefano Stabellini, Rodrigo Vivi, Bjorn Helgaas, Lee Jones,
	linux-arm Mailing List, Boris Ostrovsky, David Airlie,
	linux-parisc, Pankaj Bharadiya, Hou Zhiqiang, LKML, Tariq Toukan,
	Jon Mason, linux-ntb, Saeed Mahameed, David S. Miller

On Thu, Dec 10, 2020 at 9:57 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> First of all drivers have absolutely no business to dig into the internals
> of an irq descriptor. That's core code and subject to change. All of this
> information is readily available to /proc/interrupts in a safe and race
> free way.
>
> Remove the inspection code which is a blatant violation of subsystem
> boundaries and racy against concurrent modifications of the interrupt
> descriptor.
>
> Print the irq line instead so the information can be looked up in a sane
> way in /proc/interrupts.

...

> -               seq_printf(s, "%3i:  %6i %4i",
> +               seq_printf(s, "%3i:  %6i %4i %4i\n",

Seems different specifiers, I think the intention was something like
               seq_printf(s, "%3i:  %4i %6i %4i\n",

>                            line,
> +                          line + irq_first,
>                            num_interrupts[line],
>                            num_wake_interrupts[line]);


-- 
With Best Regards,
Andy Shevchenko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 03/30] genirq: Move irq_set_lockdep_class() to core
  2020-12-11 17:53   ` Andy Shevchenko
@ 2020-12-11 21:08     ` Thomas Gleixner
  2020-12-11 22:07       ` Thomas Gleixner
  0 siblings, 1 reply; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-11 21:08 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, open list:HFI1 DRIVER, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	Jakub Kicinski, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	Juergen Gross, David Airlie, open list:GPIO SUBSYSTEM,
	Stefano Stabellini, Rodrigo Vivi, Bjorn Helgaas, Lee Jones,
	linux-arm Mailing List, Boris Ostrovsky, Tvrtko Ursulin,
	linux-parisc, Pankaj Bharadiya, Hou Zhiqiang, LKML, Tariq Toukan,
	Jon Mason, linux-ntb, intel-gfx, David S. Miller

On Fri, Dec 11 2020 at 19:53, Andy Shevchenko wrote:

> On Thu, Dec 10, 2020 at 10:14 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>>
>> irq_set_lockdep_class() is used from modules and requires irq_to_desc() to
>> be exported. Move it into the core code which lifts another requirement for
>> the export.
>
> ...
>
>> +       if (IS_ENABLED(CONFIG_LOCKDEP))
>> +               __irq_set_lockdep_class(irq, lock_class, request_class);

You are right. Let me fix that.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* RE: [patch 14/30] drm/i915/pmu: Replace open coded kstat_irqs() copy
  2020-12-11 14:19       ` David Laight
@ 2020-12-11 21:10         ` Thomas Gleixner
  2020-12-11 22:06           ` David Laight
  0 siblings, 1 reply; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-11 21:10 UTC (permalink / raw)
  To: David Laight, Tvrtko Ursulin, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	netdev, Will Deacon, Michal Simek, linux-s390, afzal mohammed,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Marc Zyngier, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, Heiko Carstens,
	Wambui Karuga, Allen Hubbe, Juergen Gross, intel-gfx, linux-gpio,
	Stefano Stabellini, Rodrigo Vivi, Bjorn Helgaas, Lee Jones,
	linux-arm-kernel, Boris Ostrovsky, David Airlie, linux-parisc,
	Pankaj Bharadiya, Hou Zhiqiang, Tariq Toukan, Jon Mason,
	linux-ntb, Saeed Mahameed, David S. Miller

On Fri, Dec 11 2020 at 14:19, David Laight wrote:
> From: Thomas Gleixner
>> You can't catch that. If this really becomes an issue you need a
>> sequence counter around it.
>
> Or just two copies of the high word.
> Provided the accesses are sequenced:
> writer:
> 	load high:low
> 	add small_value,high:low
> 	store high
> 	store low
> 	store high_copy
> reader:
> 	load high_copy
> 	load low
> 	load high
> 	if (high != high_copy)
> 		low = 0;

And low = 0 is solving what? You need to loop back and retry until it's
consistent and then it's nothing else than an open coded sequence count.

Thanks,

        tglx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 27/30] xen/events: Only force affinity mask for percpu interrupts
  2020-12-11 14:29         ` boris.ostrovsky
@ 2020-12-11 21:27           ` Thomas Gleixner
  2020-12-11 22:21             ` Andrew Cooper
  0 siblings, 1 reply; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-11 21:27 UTC (permalink / raw)
  To: boris.ostrovsky, Jürgen Groß, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Jakub Kicinski, Will Deacon,
	Michal Simek, linux-s390, afzal mohammed, Stefano Stabellini,
	Dave Jiang, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	xen-devel, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	David Airlie, linux-gpio, Lorenzo Pieralisi, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Tvrtko Ursulin,
	linux-parisc, Pankaj Bharadiya, Hou Zhiqiang, Tariq Toukan,
	Jon Mason, linux-ntb, intel-gfx, David S. Miller

On Fri, Dec 11 2020 at 09:29, boris ostrovsky wrote:

> On 12/11/20 7:37 AM, Thomas Gleixner wrote:
>> On Fri, Dec 11 2020 at 13:10, Jürgen Groß wrote:
>>> On 11.12.20 00:20, boris.ostrovsky@oracle.com wrote:
>>>> On 12/10/20 2:26 PM, Thomas Gleixner wrote:
>>>>> Change the implementation so that the channel is bound to CPU0 at the XEN
>>>>> level and leave the affinity mask alone. At startup of the interrupt
>>>>> affinity will be assigned out of the affinity mask and the XEN binding will
>>>>> be updated.
>>>>
>>>> If that's the case then I wonder whether we need this call at all and instead bind at startup time.
>>> After some discussion with Thomas on IRC and xen-devel archaeology the
>>> result is: this will be needed especially for systems running on a
>>> single vcpu (e.g. small guests), as the .irq_set_affinity() callback
>>> won't be called in this case when starting the irq.
>
> On UP are we not then going to end up with an empty affinity mask? Or
> are we guaranteed to have it set to 1 by interrupt generic code?

An UP kernel does not ever look on the affinity mask. The
chip::irq_set_affinity() callback is not invoked so the mask is
irrelevant.

A SMP kernel on a UP machine sets CPU0 in the mask so all is good.

> This is actually why I brought this up in the first place --- a
> potential mismatch between the affinity mask and Xen-specific data
> (e.g. info->cpu and then protocol-specific data in event channel
> code). Even if they are re-synchronized later, at startup time (for
> SMP).

Which is not a problem either. The affinity mask is only relevant for
setting the affinity, but it's not relevant for delivery and never can
be.

> I don't see anything that would cause a problem right now but I worry
> that this inconsistency may come up at some point.

As long as the affinity mask becomes not part of the event channel magic
this should never matter.

Look at it from hardware:

interrupt is affine to CPU0

     CPU0 runs:
     
     set_affinity(CPU0 -> CPU1)
        local_irq_disable()
        
 --> interrupt is raised in hardware and pending on CPU0

        irq hardware is reconfigured to be affine to CPU1

        local_irq_enable()

 --> interrupt is handled on CPU0

the next interrupt will be raised on CPU1

So info->cpu which is registered via the hypercall binds the 'hardware
delivery' and whenever the new affinity is written it is rebound to some
other CPU and the next interrupt is then raised on this other CPU.

It's not any different from the hardware example at least not as far as
I understood the code.

Thanks,

        tglx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* RE: [patch 14/30] drm/i915/pmu: Replace open coded kstat_irqs() copy
  2020-12-11 21:10         ` Thomas Gleixner
@ 2020-12-11 22:06           ` David Laight
  0 siblings, 0 replies; 73+ messages in thread
From: David Laight @ 2020-12-11 22:06 UTC (permalink / raw)
  To: 'Thomas Gleixner', Tvrtko Ursulin, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	netdev, Will Deacon, Michal Simek, linux-s390, afzal mohammed,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Marc Zyngier, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, Heiko Carstens,
	Wambui Karuga, Allen Hubbe, Juergen Gross, intel-gfx, linux-gpio,
	Stefano Stabellini, Rodrigo Vivi, Bjorn Helgaas, Lee Jones,
	linux-arm-kernel, Boris Ostrovsky, David Airlie, linux-parisc,
	Pankaj Bharadiya, Hou Zhiqiang, Tariq Toukan, Jon Mason,
	linux-ntb, Saeed Mahameed, David S. Miller

From: Thomas Gleixner
> Sent: 11 December 2020 21:11
> 
> On Fri, Dec 11 2020 at 14:19, David Laight wrote:
> > From: Thomas Gleixner
> >> You can't catch that. If this really becomes an issue you need a
> >> sequence counter around it.
> >
> > Or just two copies of the high word.
> > Provided the accesses are sequenced:
> > writer:
> > 	load high:low
> > 	add small_value,high:low
> > 	store high
> > 	store low
> > 	store high_copy
> > reader:
> > 	load high_copy
> > 	load low
> > 	load high
> > 	if (high != high_copy)
> > 		low = 0;
> 
> And low = 0 is solving what? You need to loop back and retry until it's
> consistent and then it's nothing else than an open coded sequence count.

If it is a counter or timestamp then the high:0 value happened
some time between when you started trying to read the value and
when you finished trying to read it.

As such it is a perfectly reasonable return value.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 03/30] genirq: Move irq_set_lockdep_class() to core
  2020-12-11 21:08     ` Thomas Gleixner
@ 2020-12-11 22:07       ` Thomas Gleixner
  2020-12-12 13:22         ` Andy Shevchenko
  0 siblings, 1 reply; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-11 22:07 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, open list:HFI1 DRIVER, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	Jakub Kicinski, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	Juergen Gross, David Airlie, open list:GPIO SUBSYSTEM,
	Stefano Stabellini, Rodrigo Vivi, Bjorn Helgaas, Lee Jones,
	linux-arm Mailing List, Boris Ostrovsky, Tvrtko Ursulin,
	linux-parisc, Pankaj Bharadiya, Hou Zhiqiang, LKML, Tariq Toukan,
	Jon Mason, linux-ntb, intel-gfx, David S. Miller

On Fri, Dec 11 2020 at 22:08, Thomas Gleixner wrote:

> On Fri, Dec 11 2020 at 19:53, Andy Shevchenko wrote:
>
>> On Thu, Dec 10, 2020 at 10:14 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>>>
>>> irq_set_lockdep_class() is used from modules and requires irq_to_desc() to
>>> be exported. Move it into the core code which lifts another requirement for
>>> the export.
>>
>> ...
>>
>>> +       if (IS_ENABLED(CONFIG_LOCKDEP))
>>> +               __irq_set_lockdep_class(irq, lock_class, request_class);
>
> You are right. Let me fix that.

No. I have to correct myself. You're wrong.

The inline is evaluated in the compilation units which include that
header and because the function declaration is unconditional it is
happy.

Now the optimizer stage makes the whole thing a NOOP if CONFIG_LOCKDEP=n
and thereby drops the reference to the function which makes it not
required for linking.

So in the file where the function is implemented:

#ifdef CONFIG_LOCKDEP
void __irq_set_lockdep_class(....)
{
}
#endif

The whole block is either discarded because CONFIG_LOCKDEP is not
defined or compile if it is defined which makes it available for the
linker.

And in the latter case the optimizer keeps the call in the inline (it
optimizes the condition away because it's always true).

So in both cases the compiler and the linker are happy and everything
works as expected.

It would fail if the header file had the following:

#ifdef CONFIG_LOCKDEP
void __irq_set_lockdep_class(....);
#endif

Because then it would complain about the missing function prototype when
it evaluates the inline.

Thanks,

        tglx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 27/30] xen/events: Only force affinity mask for percpu interrupts
  2020-12-11 21:27           ` Thomas Gleixner
@ 2020-12-11 22:21             ` Andrew Cooper
  2020-12-11 22:56               ` Thomas Gleixner
  0 siblings, 1 reply; 73+ messages in thread
From: Andrew Cooper @ 2020-12-11 22:21 UTC (permalink / raw)
  To: Thomas Gleixner, boris.ostrovsky, Jürgen Groß, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Jakub Kicinski, Will Deacon,
	Michal Simek, linux-s390, afzal mohammed, Stefano Stabellini,
	Dave Jiang, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	xen-devel, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	David Airlie, linux-gpio, Lorenzo Pieralisi, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Tvrtko Ursulin,
	linux-parisc, Pankaj Bharadiya, Hou Zhiqiang, Tariq Toukan,
	Jon Mason, linux-ntb, intel-gfx, David S. Miller

On 11/12/2020 21:27, Thomas Gleixner wrote:
> On Fri, Dec 11 2020 at 09:29, boris ostrovsky wrote:
>
>> On 12/11/20 7:37 AM, Thomas Gleixner wrote:
>>> On Fri, Dec 11 2020 at 13:10, Jürgen Groß wrote:
>>>> On 11.12.20 00:20, boris.ostrovsky@oracle.com wrote:
>>>>> On 12/10/20 2:26 PM, Thomas Gleixner wrote:
>>>>>> Change the implementation so that the channel is bound to CPU0 at the XEN
>>>>>> level and leave the affinity mask alone. At startup of the interrupt
>>>>>> affinity will be assigned out of the affinity mask and the XEN binding will
>>>>>> be updated.
>>>>> If that's the case then I wonder whether we need this call at all and instead bind at startup time.
>>>> After some discussion with Thomas on IRC and xen-devel archaeology the
>>>> result is: this will be needed especially for systems running on a
>>>> single vcpu (e.g. small guests), as the .irq_set_affinity() callback
>>>> won't be called in this case when starting the irq.
>> On UP are we not then going to end up with an empty affinity mask? Or
>> are we guaranteed to have it set to 1 by interrupt generic code?
> An UP kernel does not ever look on the affinity mask. The
> chip::irq_set_affinity() callback is not invoked so the mask is
> irrelevant.
>
> A SMP kernel on a UP machine sets CPU0 in the mask so all is good.
>
>> This is actually why I brought this up in the first place --- a
>> potential mismatch between the affinity mask and Xen-specific data
>> (e.g. info->cpu and then protocol-specific data in event channel
>> code). Even if they are re-synchronized later, at startup time (for
>> SMP).
> Which is not a problem either. The affinity mask is only relevant for
> setting the affinity, but it's not relevant for delivery and never can
> be.
>
>> I don't see anything that would cause a problem right now but I worry
>> that this inconsistency may come up at some point.
> As long as the affinity mask becomes not part of the event channel magic
> this should never matter.
>
> Look at it from hardware:
>
> interrupt is affine to CPU0
>
>      CPU0 runs:
>      
>      set_affinity(CPU0 -> CPU1)
>         local_irq_disable()
>         
>  --> interrupt is raised in hardware and pending on CPU0
>
>         irq hardware is reconfigured to be affine to CPU1
>
>         local_irq_enable()
>
>  --> interrupt is handled on CPU0
>
> the next interrupt will be raised on CPU1
>
> So info->cpu which is registered via the hypercall binds the 'hardware
> delivery' and whenever the new affinity is written it is rebound to some
> other CPU and the next interrupt is then raised on this other CPU.
>
> It's not any different from the hardware example at least not as far as
> I understood the code.

Xen's event channels do have a couple of quirks.

Binding an event channel always results in one spurious event being
delivered.  This is to cover notifications which can get lost during the
bidirectional setup, or re-setups in certain configurations.

Binding an interdomain or pirq event channel always defaults to vCPU0. 
There is no way to atomically set the affinity while binding.  I believe
the API predates SMP guest support in Xen, and noone has fixed it up since.

As a consequence, the guest will observe the event raised on vCPU0 as
part of setting up the event, even if it attempts to set a different
affinity immediately afterwards.  A little bit of care needs to be taken
when binding an event channel on vCPUs other than 0, to ensure that the
callback is safe with respect to any remaining state needing initialisation.

Beyond this, there is nothing magic I'm aware of.

We have seen soft lockups before in certain scenarios, simply due to the
quantity of events hitting vCPU0 before irqbalance gets around to
spreading the load.  This is why there is an attempt to round-robin the
userspace event channel affinities by default, but I still don't see why
this would need custom affinity logic itself.

Thanks,

~Andrew
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 27/30] xen/events: Only force affinity mask for percpu interrupts
  2020-12-11 22:21             ` Andrew Cooper
@ 2020-12-11 22:56               ` Thomas Gleixner
  0 siblings, 0 replies; 73+ messages in thread
From: Thomas Gleixner @ 2020-12-11 22:56 UTC (permalink / raw)
  To: Andrew Cooper, boris.ostrovsky, Jürgen Groß, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Jakub Kicinski, Will Deacon,
	Michal Simek, linux-s390, afzal mohammed, Stefano Stabellini,
	Dave Jiang, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	xen-devel, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	David Airlie, linux-gpio, Lorenzo Pieralisi, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Tvrtko Ursulin,
	linux-parisc, Pankaj Bharadiya, Hou Zhiqiang, Tariq Toukan,
	Jon Mason, linux-ntb, intel-gfx, David S. Miller

Andrew,

On Fri, Dec 11 2020 at 22:21, Andrew Cooper wrote:
> On 11/12/2020 21:27, Thomas Gleixner wrote:
>> It's not any different from the hardware example at least not as far as
>> I understood the code.
>
> Xen's event channels do have a couple of quirks.

Why am I not surprised?

> Binding an event channel always results in one spurious event being
> delivered.  This is to cover notifications which can get lost during the
> bidirectional setup, or re-setups in certain configurations.
>
> Binding an interdomain or pirq event channel always defaults to vCPU0. 
> There is no way to atomically set the affinity while binding.  I believe
> the API predates SMP guest support in Xen, and noone has fixed it up
> since.

That's fine. I'm not changing that.

What I'm changing is the unwanted and unnecessary overwriting of the
actual affinity mask.

We have a similar issue on real hardware where we can only target _one_
CPU and not all CPUs in the affinity mask. So we still can preserve the
(user) requested mask and just affine it to one CPU which is reflected
in the effective affinity mask. This the right thing to do for two
reasons:

   1) It allows proper interrupt distribution

   2) It does not break (user) requested affinity when the effective
      target CPU goes offline and the affinity mask still contains
      online CPUs. If you overwrite it you lost track of the requested
      broader mask.

> As a consequence, the guest will observe the event raised on vCPU0 as
> part of setting up the event, even if it attempts to set a different
> affinity immediately afterwards.  A little bit of care needs to be taken
> when binding an event channel on vCPUs other than 0, to ensure that the
> callback is safe with respect to any remaining state needing
> initialisation.

That's preserved for all non percpu interrupts. The percpu variant of
VIRQ and IPIs did binding to vCPU != 0 already before this change.

> Beyond this, there is nothing magic I'm aware of.
>
> We have seen soft lockups before in certain scenarios, simply due to the
> quantity of events hitting vCPU0 before irqbalance gets around to
> spreading the load.  This is why there is an attempt to round-robin the
> userspace event channel affinities by default, but I still don't see why
> this would need custom affinity logic itself.

Just the previous attempt makes no sense for the reasons I outlined in
the changelog. So now with this new spreading mechanics you get the
distribution for all cases:

  1) Post setup using and respecting the default affinity mask which can
     be set as a kernel commandline parameter.

  2) Runtime (user) requested affinity change with a mask which contains
     more than one vCPU. The previous logic always chose the first one
     in the mask.

     So assume userspace affines 4 irqs to a CPU 0-3 and 4 irqs to CPU
     4-7 then 4 irqs end up on CPU0 and 4 on CPU4

     The new algorithm which is similar to what we have on x86 (minus
     the vector space limitation) picks the CPU which has the least
     number of channels affine to it at that moment. If e.g. all 8 CPUs
     have the same number of vectors before that change then in the
     example above the first 4 are spread to CPU0-3 and the second 4 to
     CPU4-7

Thanks,

        tglx
   
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 15/30] pinctrl: nomadik: Use irq_has_action()
  2020-12-10 19:25 ` [patch 15/30] pinctrl: nomadik: Use irq_has_action() Thomas Gleixner
@ 2020-12-12  0:45   ` Linus Walleij
  0 siblings, 0 replies; 73+ messages in thread
From: Linus Walleij @ 2020-12-12  0:45 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, open list:DRM PANEL DRIVERS, Chris Wilson,
	James E.J. Bottomley, Saeed Mahameed, netdev, Will Deacon,
	Michal Simek, linux-s390, afzal mohammed, Lorenzo Pieralisi,
	Dave Jiang, xen-devel, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	Jakub Kicinski, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	Juergen Gross, David Airlie, open list:GPIO SUBSYSTEM,
	Stefano Stabellini, Rodrigo Vivi, Bjorn Helgaas, Lee Jones,
	Linux ARM, Boris Ostrovsky, Tvrtko Ursulin, linux-parisc,
	Pankaj Bharadiya, Hou Zhiqiang, LKML, Tariq Toukan, Jon Mason,
	linux-ntb, intel-gfx, David S. Miller

On Thu, Dec 10, 2020 at 8:42 PM Thomas Gleixner <tglx@linutronix.de> wrote:

> Let the core code do the fiddling with irq_desc.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-gpio@vger.kernel.org

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

I suppose you will funnel this directly to Torvalds, else tell me and
I'll apply it to my tree.

Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 03/30] genirq: Move irq_set_lockdep_class() to core
  2020-12-11 22:07       ` Thomas Gleixner
@ 2020-12-12 13:22         ` Andy Shevchenko
  0 siblings, 0 replies; 73+ messages in thread
From: Andy Shevchenko @ 2020-12-12 13:22 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, open list:HFI1 DRIVER, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	Jakub Kicinski, Heiko Carstens, Wambui Karuga, Allen Hubbe,
	Juergen Gross, David Airlie, open list:GPIO SUBSYSTEM,
	Stefano Stabellini, Rodrigo Vivi, Bjorn Helgaas, Lee Jones,
	linux-arm Mailing List, Boris Ostrovsky, Tvrtko Ursulin,
	linux-parisc, Pankaj Bharadiya, Hou Zhiqiang, LKML, Tariq Toukan,
	Jon Mason, linux-ntb, intel-gfx, David S. Miller

On Sat, Dec 12, 2020 at 12:07 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Fri, Dec 11 2020 at 22:08, Thomas Gleixner wrote:
>
> > On Fri, Dec 11 2020 at 19:53, Andy Shevchenko wrote:
> >
> >> On Thu, Dec 10, 2020 at 10:14 PM Thomas Gleixner <tglx@linutronix.de> wrote:
> >>>
> >>> irq_set_lockdep_class() is used from modules and requires irq_to_desc() to
> >>> be exported. Move it into the core code which lifts another requirement for
> >>> the export.
> >>
> >> ...
> >>
> >>> +       if (IS_ENABLED(CONFIG_LOCKDEP))
> >>> +               __irq_set_lockdep_class(irq, lock_class, request_class);
> >
> > You are right. Let me fix that.
>
> No. I have to correct myself. You're wrong.
>
> The inline is evaluated in the compilation units which include that
> header and because the function declaration is unconditional it is
> happy.
>
> Now the optimizer stage makes the whole thing a NOOP if CONFIG_LOCKDEP=n
> and thereby drops the reference to the function which makes it not
> required for linking.
>
> So in the file where the function is implemented:
>
> #ifdef CONFIG_LOCKDEP
> void __irq_set_lockdep_class(....)
> {
> }
> #endif
>
> The whole block is either discarded because CONFIG_LOCKDEP is not
> defined or compile if it is defined which makes it available for the
> linker.
>
> And in the latter case the optimizer keeps the call in the inline (it
> optimizes the condition away because it's always true).
>
> So in both cases the compiler and the linker are happy and everything
> works as expected.
>
> It would fail if the header file had the following:
>
> #ifdef CONFIG_LOCKDEP
> void __irq_set_lockdep_class(....);
> #endif
>
> Because then it would complain about the missing function prototype when
> it evaluates the inline.

I understand that (that's why I put "if even no warning") and what I'm
talking about is the purpose of IS_ENABLED(). It's usually good for
compile testing !CONFIG_FOO cases. But here it seems inconsistent.

The pattern I usually see in the cases like this is

 #ifdef CONFIG_LOCKDEP
 void __irq_set_lockdep_class(....);
 #else
 static inline void ... {}
 #endif

and call it directly in the caller.

It's not a big deal, so up to you.

-- 
With Best Regards,
Andy Shevchenko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 20/30] net/mlx4: Replace irq_to_desc() abuse
  2020-12-10 19:25 ` [patch 20/30] net/mlx4: Replace irq_to_desc() abuse Thomas Gleixner
@ 2020-12-13 11:24   ` Tariq Toukan
  0 siblings, 0 replies; 73+ messages in thread
From: Tariq Toukan @ 2020-12-13 11:24 UTC (permalink / raw)
  To: Thomas Gleixner, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Will Deacon, Michal Simek, linux-s390, afzal mohammed,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Marc Zyngier, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, intel-gfx,
	Wambui Karuga, Allen Hubbe, Juergen Gross, Tvrtko Ursulin,
	Heiko Carstens, Jon Mason, linux-gpio, Stefano Stabellini,
	Rodrigo Vivi, Bjorn Helgaas, Lee Jones, linux-arm-kernel,
	Boris Ostrovsky, David Airlie, linux-parisc, netdev,
	Hou Zhiqiang, Tariq Toukan, Pankaj Bharadiya, linux-ntb,
	Saeed Mahameed, David S. Miller



On 12/10/2020 9:25 PM, Thomas Gleixner wrote:
> No driver has any business with the internals of an interrupt
> descriptor. Storing a pointer to it just to use yet another helper at the
> actual usage site to retrieve the affinity mask is creative at best. Just
> because C does not allow encapsulation does not mean that the kernel has no
> limits.
> 
> Retrieve a pointer to the affinity mask itself and use that. It's still
> using an interface which is usually not for random drivers, but definitely
> less hideous than the previous hack.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Tariq Toukan <tariqt@nvidia.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: netdev@vger.kernel.org
> Cc: linux-rdma@vger.kernel.org
> ---
>   drivers/net/ethernet/mellanox/mlx4/en_cq.c   |    8 +++-----
>   drivers/net/ethernet/mellanox/mlx4/en_rx.c   |    6 +-----
>   drivers/net/ethernet/mellanox/mlx4/mlx4_en.h |    3 ++-
>   3 files changed, 6 insertions(+), 11 deletions(-)
> 

Reviewed-by: Tariq Toukan <tariqt@nvidia.com>

Thanks for your patch.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 21/30] net/mlx4: Use effective interrupt affinity
  2020-12-10 19:25 ` [patch 21/30] net/mlx4: Use effective interrupt affinity Thomas Gleixner
@ 2020-12-13 11:31   ` Tariq Toukan
  0 siblings, 0 replies; 73+ messages in thread
From: Tariq Toukan @ 2020-12-13 11:31 UTC (permalink / raw)
  To: Thomas Gleixner, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Will Deacon, Michal Simek, linux-s390, afzal mohammed,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Marc Zyngier, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, intel-gfx,
	Wambui Karuga, Allen Hubbe, Juergen Gross, Tvrtko Ursulin,
	Heiko Carstens, Jon Mason, linux-gpio, Stefano Stabellini,
	Rodrigo Vivi, Bjorn Helgaas, Lee Jones, linux-arm-kernel,
	Boris Ostrovsky, David Airlie, linux-parisc, netdev,
	Hou Zhiqiang, Tariq Toukan, Pankaj Bharadiya, linux-ntb,
	Saeed Mahameed, David S. Miller



On 12/10/2020 9:25 PM, Thomas Gleixner wrote:
> Using the interrupt affinity mask for checking locality is not really
> working well on architectures which support effective affinity masks.
> 
> The affinity mask is either the system wide default or set by user space,
> but the architecture can or even must reduce the mask to the effective set,
> which means that checking the affinity mask itself does not really tell
> about the actual target CPUs.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Tariq Toukan <tariqt@nvidia.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: netdev@vger.kernel.org
> Cc: linux-rdma@vger.kernel.org
> ---
>   drivers/net/ethernet/mellanox/mlx4/en_cq.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
> @@ -117,7 +117,7 @@ int mlx4_en_activate_cq(struct mlx4_en_p
>   			assigned_eq = true;
>   		}
>   		irq = mlx4_eq_get_irq(mdev->dev, cq->vector);
> -		cq->aff_mask = irq_get_affinity_mask(irq);
> +		cq->aff_mask = irq_get_effective_affinity_mask(irq);
>   	} else {
>   		/* For TX we use the same irq per
>   		ring we assigned for the RX    */
> 

Reviewed-by: Tariq Toukan <tariqt@nvidia.com>

Thanks.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 22/30] net/mlx5: Replace irq_to_desc() abuse
  2020-12-10 19:25 ` [patch 22/30] net/mlx5: Replace irq_to_desc() abuse Thomas Gleixner
@ 2020-12-13 11:34   ` Tariq Toukan
  2020-12-14 21:13   ` Saeed Mahameed
  1 sibling, 0 replies; 73+ messages in thread
From: Tariq Toukan @ 2020-12-13 11:34 UTC (permalink / raw)
  To: Thomas Gleixner, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Saeed Mahameed, netdev, Will Deacon, Michal Simek, linux-s390,
	afzal mohammed, Lorenzo Pieralisi, Dave Jiang, xen-devel,
	Leon Romanovsky, linux-rdma, Marc Zyngier, Helge Deller,
	Russell King, Christian Borntraeger, linux-pci, Jakub Kicinski,
	Heiko Carstens, Wambui Karuga, Allen Hubbe, Juergen Gross,
	David Airlie, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	Tvrtko Ursulin, linux-parisc, Pankaj Bharadiya, Hou Zhiqiang,
	Tariq Toukan, Jon Mason, linux-ntb, intel-gfx, David S. Miller



On 12/10/2020 9:25 PM, Thomas Gleixner wrote:
> No driver has any business with the internals of an interrupt
> descriptor. Storing a pointer to it just to use yet another helper at the
> actual usage site to retrieve the affinity mask is creative at best. Just
> because C does not allow encapsulation does not mean that the kernel has no
> limits.
> 
> Retrieve a pointer to the affinity mask itself and use that. It's still
> using an interface which is usually not for random drivers, but definitely
> less hideous than the previous hack.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>   drivers/net/ethernet/mellanox/mlx5/core/en.h      |    2 +-
>   drivers/net/ethernet/mellanox/mlx5/core/en_main.c |    2 +-
>   drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c |    6 +-----
>   3 files changed, 3 insertions(+), 7 deletions(-)
> 

Reviewed-by: Tariq Toukan <tariqt@nvidia.com>

Thanks.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 23/30] net/mlx5: Use effective interrupt affinity
  2020-12-10 19:25 ` [patch 23/30] net/mlx5: Use effective interrupt affinity Thomas Gleixner
@ 2020-12-13 11:35   ` Tariq Toukan
  2020-12-14 20:58   ` Saeed Mahameed
  1 sibling, 0 replies; 73+ messages in thread
From: Tariq Toukan @ 2020-12-13 11:35 UTC (permalink / raw)
  To: Thomas Gleixner, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	afzal mohammed, Michal Simek, linux-s390, Lorenzo Pieralisi,
	Dave Jiang, xen-devel, Leon Romanovsky, linux-rdma, Marc Zyngier,
	Helge Deller, Russell King, Christian Borntraeger, linux-pci,
	Jakub Kicinski, intel-gfx, Wambui Karuga, Allen Hubbe,
	Juergen Gross, Will Deacon, Tvrtko Ursulin, Heiko Carstens,
	Jon Mason, linux-gpio, Stefano Stabellini, Rodrigo Vivi,
	Bjorn Helgaas, Lee Jones, linux-arm-kernel, Boris Ostrovsky,
	David Airlie, linux-parisc, netdev, Hou Zhiqiang, Tariq Toukan,
	Pankaj Bharadiya, linux-ntb, Saeed Mahameed, David S. Miller



On 12/10/2020 9:25 PM, Thomas Gleixner wrote:
> Using the interrupt affinity mask for checking locality is not really
> working well on architectures which support effective affinity masks.
> 
> The affinity mask is either the system wide default or set by user space,
> but the architecture can or even must reduce the mask to the effective set,
> which means that checking the affinity mask itself does not really tell
> about the actual target CPUs.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Saeed Mahameed <saeedm@nvidia.com>
> Cc: Leon Romanovsky <leon@kernel.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: netdev@vger.kernel.org
> Cc: linux-rdma@vger.kernel.org
> ---
>   drivers/net/ethernet/mellanox/mlx5/core/en_main.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -1998,7 +1998,7 @@ static int mlx5e_open_channel(struct mlx
>   	c->num_tc   = params->num_tc;
>   	c->xdp      = !!params->xdp_prog;
>   	c->stats    = &priv->channel_stats[ix].ch;
> -	c->aff_mask = irq_get_affinity_mask(irq);
> +	c->aff_mask = irq_get_effective_affinity_mask(irq);
>   	c->lag_port = mlx5e_enumerate_lag_port(priv->mdev, ix);
>   
>   	netif_napi_add(netdev, &c->napi, mlx5e_napi_poll, 64);
> 

Reviewed-by: Tariq Toukan <tariqt@nvidia.com>

Thanks.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 23/30] net/mlx5: Use effective interrupt affinity
  2020-12-10 19:25 ` [patch 23/30] net/mlx5: Use effective interrupt affinity Thomas Gleixner
  2020-12-13 11:35   ` Tariq Toukan
@ 2020-12-14 20:58   ` Saeed Mahameed
  1 sibling, 0 replies; 73+ messages in thread
From: Saeed Mahameed @ 2020-12-14 20:58 UTC (permalink / raw)
  To: Thomas Gleixner, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	Will Deacon, Michal Simek, linux-s390, afzal mohammed,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Marc Zyngier, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, intel-gfx,
	Wambui Karuga, Allen Hubbe, Juergen Gross, Tvrtko Ursulin,
	Heiko Carstens, Jon Mason, linux-gpio, Stefano Stabellini,
	Rodrigo Vivi, Bjorn Helgaas, Lee Jones, linux-arm-kernel,
	Boris Ostrovsky, David Airlie, linux-parisc, netdev,
	Hou Zhiqiang, Tariq Toukan, Pankaj Bharadiya, linux-ntb,
	David S. Miller

On Thu, 2020-12-10 at 20:25 +0100, Thomas Gleixner wrote:
> Using the interrupt affinity mask for checking locality is not really
> working well on architectures which support effective affinity masks.
> 
> The affinity mask is either the system wide default or set by user
> space,
> but the architecture can or even must reduce the mask to the
> effective set,
> which means that checking the affinity mask itself does not really
> tell
> about the actual target CPUs.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Saeed Mahameed <saeedm@nvidia.com>
> Cc: Leon Romanovsky <leon@kernel.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: netdev@vger.kernel.org
> Cc: linux-rdma@vger.kernel.org
> 

Acked-by: Saeed Mahameed <saeedm@nvidia.com>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 22/30] net/mlx5: Replace irq_to_desc() abuse
  2020-12-10 19:25 ` [patch 22/30] net/mlx5: Replace irq_to_desc() abuse Thomas Gleixner
  2020-12-13 11:34   ` Tariq Toukan
@ 2020-12-14 21:13   ` Saeed Mahameed
  1 sibling, 0 replies; 73+ messages in thread
From: Saeed Mahameed @ 2020-12-14 21:13 UTC (permalink / raw)
  To: Thomas Gleixner, LKML
  Cc: Mark Rutland, Karthikeyan Mitran, Peter Zijlstra,
	Catalin Marinas, dri-devel, Chris Wilson, James E.J. Bottomley,
	netdev, Will Deacon, Michal Simek, linux-s390, afzal mohammed,
	Lorenzo Pieralisi, Dave Jiang, xen-devel, Leon Romanovsky,
	linux-rdma, Marc Zyngier, Helge Deller, Russell King,
	Christian Borntraeger, linux-pci, Jakub Kicinski, Heiko Carstens,
	Wambui Karuga, Allen Hubbe, Juergen Gross, David Airlie,
	linux-gpio, Stefano Stabellini, Rodrigo Vivi, Bjorn Helgaas,
	Lee Jones, linux-arm-kernel, Boris Ostrovsky, Tvrtko Ursulin,
	linux-parisc, Pankaj Bharadiya, Hou Zhiqiang, Tariq Toukan,
	Jon Mason, linux-ntb, intel-gfx, David S. Miller

On Thu, 2020-12-10 at 20:25 +0100, Thomas Gleixner wrote:
> No driver has any business with the internals of an interrupt
> descriptor. Storing a pointer to it just to use yet another helper at
> the
> actual usage site to retrieve the affinity mask is creative at best.
> Just
> because C does not allow encapsulation does not mean that the kernel
> has no
> limits.
> 

you can't blame the developers for using stuff from include/linux/
Not all developers are the same, and sometime we don't read in between
the lines, you can't assume all driver developers to be expert on irq
APIs disciplines.

your rules must be programmatically expressed, for instance,
you can just hide struct irq_desc and irq_to_desc() in kernel/irq/ and
remove them from include/linux/ header files, if you want privacy in
your subsystem, don't put all your header files on display under
include/linux.


> Retrieve a pointer to the affinity mask itself and use that. It's
> still
> using an interface which is usually not for random drivers, but
> definitely
> less hideous than the previous hack.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en.h      |    2 +-
>  drivers/net/ethernet/mellanox/mlx5/core/en_main.c |    2 +-
>  drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c |    6 +-----
>  3 files changed, 3 insertions(+), 7 deletions(-)
> 
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> @@ -669,7 +669,7 @@ struct mlx5e_channel {
>  	spinlock_t                 async_icosq_lock;
>  
>  	/* data path - accessed per napi poll */
> -	struct irq_desc *irq_desc;
> +	const struct cpumask	  *aff_mask;
>  	struct mlx5e_ch_stats     *stats;
>  
>  	/* control */
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -1998,7 +1998,7 @@ static int mlx5e_open_channel(struct mlx
>  	c->num_tc   = params->num_tc;
>  	c->xdp      = !!params->xdp_prog;
>  	c->stats    = &priv->channel_stats[ix].ch;
> -	c->irq_desc = irq_to_desc(irq);
> +	c->aff_mask = irq_get_affinity_mask(irq);

as long as the affinity mask pointer stays the same for the lifetime of
the irq vector.

Assuming that:
Acked-by: Saeed Mahameed <saeedm@nvidia.com>


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 02/30] genirq: Move status flag checks to core
  2020-12-10 19:25 ` [patch 02/30] genirq: Move status flag checks to core Thomas Gleixner
@ 2020-12-27 19:20   ` Guenter Roeck
  2021-01-11 10:14     ` Thomas Gleixner
  0 siblings, 1 reply; 73+ messages in thread
From: Guenter Roeck @ 2020-12-27 19:20 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Mark Rutland, Karthikeyan Mitran, Pankaj Bharadiya,
	Peter Zijlstra, Catalin Marinas, Rodrigo Vivi, dri-devel,
	Chris Wilson, James E.J. Bottomley, Russell King, afzal mohammed,
	Boris Ostrovsky, linux-s390, Lorenzo Pieralisi, Dave Jiang,
	Leon Romanovsky, linux-rdma, Will Deacon, Helge Deller,
	Michal Simek, Christian Borntraeger, linux-pci, intel-gfx,
	xen-devel, Hou Zhiqiang, Wambui Karuga, Allen Hubbe,
	Tvrtko Ursulin, Heiko Carstens, Jon Mason, linux-gpio,
	Stefano Stabellini, Jakub Kicinski, Bjorn Helgaas, Lee Jones,
	linux-arm-kernel, Juergen Gross, David Airlie, linux-parisc,
	netdev, LKML, Tariq Toukan, Marc Zyngier, linux-ntb,
	Saeed Mahameed, David S. Miller

On Thu, Dec 10, 2020 at 08:25:38PM +0100, Thomas Gleixner wrote:
> These checks are used by modules and prevent the removal of the export of
> irq_to_desc(). Move the accessor into the core.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Yes, but that means that irq_check_status_bit() may be called from modules,
but it is not exported, resulting in build errors such as the following.

arm64:allmodconfig:

ERROR: modpost: "irq_check_status_bit" [drivers/perf/arm_spe_pmu.ko] undefined!

Guenter

> ---
>  include/linux/irqdesc.h |   17 +++++------------
>  kernel/irq/manage.c     |   17 +++++++++++++++++
>  2 files changed, 22 insertions(+), 12 deletions(-)
> 
> --- a/include/linux/irqdesc.h
> +++ b/include/linux/irqdesc.h
> @@ -223,28 +223,21 @@ irq_set_chip_handler_name_locked(struct
>  	data->chip = chip;
>  }
>  
> +bool irq_check_status_bit(unsigned int irq, unsigned int bitmask);
> +
>  static inline bool irq_balancing_disabled(unsigned int irq)
>  {
> -	struct irq_desc *desc;
> -
> -	desc = irq_to_desc(irq);
> -	return desc->status_use_accessors & IRQ_NO_BALANCING_MASK;
> +	return irq_check_status_bit(irq, IRQ_NO_BALANCING_MASK);
>  }
>  
>  static inline bool irq_is_percpu(unsigned int irq)
>  {
> -	struct irq_desc *desc;
> -
> -	desc = irq_to_desc(irq);
> -	return desc->status_use_accessors & IRQ_PER_CPU;
> +	return irq_check_status_bit(irq, IRQ_PER_CPU);
>  }
>  
>  static inline bool irq_is_percpu_devid(unsigned int irq)
>  {
> -	struct irq_desc *desc;
> -
> -	desc = irq_to_desc(irq);
> -	return desc->status_use_accessors & IRQ_PER_CPU_DEVID;
> +	return irq_check_status_bit(irq, IRQ_PER_CPU_DEVID);
>  }
>  
>  static inline void
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -2769,3 +2769,23 @@ bool irq_has_action(unsigned int irq)
>  	return res;
>  }
>  EXPORT_SYMBOL_GPL(irq_has_action);
> +
> +/**
> + * irq_check_status_bit - Check whether bits in the irq descriptor status are set
> + * @irq:	The linux irq number
> + * @bitmask:	The bitmask to evaluate
> + *
> + * Returns: True if one of the bits in @bitmask is set
> + */
> +bool irq_check_status_bit(unsigned int irq, unsigned int bitmask)
> +{
> +	struct irq_desc *desc;
> +	bool res = false;
> +
> +	rcu_read_lock();
> +	desc = irq_to_desc(irq);
> +	if (desc)
> +		res = !!(desc->status_use_accessors & bitmask);
> +	rcu_read_unlock();
> +	return res;
> +}
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [patch 02/30] genirq: Move status flag checks to core
  2020-12-27 19:20   ` Guenter Roeck
@ 2021-01-11 10:14     ` Thomas Gleixner
  0 siblings, 0 replies; 73+ messages in thread
From: Thomas Gleixner @ 2021-01-11 10:14 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Mark Rutland, Karthikeyan Mitran, Pankaj Bharadiya,
	Peter Zijlstra, Catalin Marinas, Rodrigo Vivi, dri-devel,
	Chris Wilson, James E.J. Bottomley, Russell King, afzal mohammed,
	Boris Ostrovsky, linux-s390, Lorenzo Pieralisi, Dave Jiang,
	Leon Romanovsky, linux-rdma, Will Deacon, Helge Deller,
	Michal Simek, Christian Borntraeger, linux-pci, intel-gfx,
	xen-devel, Hou Zhiqiang, Wambui Karuga, Allen Hubbe,
	Tvrtko Ursulin, Heiko Carstens, Jon Mason, linux-gpio,
	Stefano Stabellini, Jakub Kicinski, Bjorn Helgaas, Lee Jones,
	linux-arm-kernel, Juergen Gross, David Airlie, linux-parisc,
	netdev, LKML, Tariq Toukan, Marc Zyngier, linux-ntb,
	Saeed Mahameed, David S. Miller

On Sun, Dec 27 2020 at 11:20, Guenter Roeck wrote:
> On Thu, Dec 10, 2020 at 08:25:38PM +0100, Thomas Gleixner wrote:
> Yes, but that means that irq_check_status_bit() may be called from modules,
> but it is not exported, resulting in build errors such as the following.
>
> arm64:allmodconfig:
>
> ERROR: modpost: "irq_check_status_bit" [drivers/perf/arm_spe_pmu.ko] undefined!

Duh. Yes, that lacks an export obviously.

Thanks,

        tglx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

end of thread, back to index

Thread overview: 73+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-10 19:25 [patch 00/30] genirq: Treewide hunt for irq descriptor abuse and assorted fixes Thomas Gleixner
2020-12-10 19:25 ` [patch 01/30] genirq: Move irq_has_action() into core code Thomas Gleixner
2020-12-10 19:25 ` [patch 02/30] genirq: Move status flag checks to core Thomas Gleixner
2020-12-27 19:20   ` Guenter Roeck
2021-01-11 10:14     ` Thomas Gleixner
2020-12-10 19:25 ` [patch 03/30] genirq: Move irq_set_lockdep_class() " Thomas Gleixner
2020-12-11 17:53   ` Andy Shevchenko
2020-12-11 21:08     ` Thomas Gleixner
2020-12-11 22:07       ` Thomas Gleixner
2020-12-12 13:22         ` Andy Shevchenko
2020-12-10 19:25 ` [patch 04/30] genirq: Provide irq_get_effective_affinity() Thomas Gleixner
2020-12-10 19:25 ` [patch 05/30] genirq: Annotate irq stats data races Thomas Gleixner
2020-12-10 19:25 ` [patch 06/30] parisc/irq: Simplify irq count output for /proc/interrupts Thomas Gleixner
2020-12-10 19:25 ` [patch 07/30] genirq: Make kstat_irqs() static Thomas Gleixner
2020-12-10 19:25 ` [patch 08/30] genirq: Provide kstat_irqdesc_cpu() Thomas Gleixner
2020-12-10 19:25 ` [patch 09/30] ARM: smp: Use irq_desc_kstat_cpu() in show_ipi_list() Thomas Gleixner
2020-12-11 18:08   ` Marc Zyngier
2020-12-10 19:25 ` [patch 10/30] arm64/smp: Use irq_desc_kstat_cpu() in arch_show_interrupts() Thomas Gleixner
2020-12-11 18:08   ` Marc Zyngier
2020-12-10 19:25 ` [patch 11/30] parisc/irq: Use irq_desc_kstat_cpu() in show_interrupts() Thomas Gleixner
2020-12-10 19:25 ` [patch 12/30] s390/irq: Use irq_desc_kstat_cpu() in show_msi_interrupt() Thomas Gleixner
2020-12-10 20:31   ` Heiko Carstens
2020-12-10 19:25 ` [patch 13/30] drm/i915/lpe_audio: Remove pointless irq_to_desc() usage Thomas Gleixner
2020-12-10 19:48   ` [Intel-gfx] " Ville Syrjälä
2020-12-11  9:51     ` Jani Nikula
2020-12-10 19:25 ` [patch 14/30] drm/i915/pmu: Replace open coded kstat_irqs() copy Thomas Gleixner
2020-12-11  9:54   ` Jani Nikula
2020-12-11 10:13   ` Tvrtko Ursulin
2020-12-11 12:57     ` Thomas Gleixner
2020-12-11 14:19       ` David Laight
2020-12-11 21:10         ` Thomas Gleixner
2020-12-11 22:06           ` David Laight
2020-12-10 19:25 ` [patch 15/30] pinctrl: nomadik: Use irq_has_action() Thomas Gleixner
2020-12-12  0:45   ` Linus Walleij
2020-12-10 19:25 ` [patch 16/30] mfd: ab8500-debugfs: Remove the racy fiddling with irq_desc Thomas Gleixner
2020-12-11  8:22   ` Linus Walleij
2020-12-11 10:04   ` Lee Jones
2020-12-11 18:12   ` Andy Shevchenko
2020-12-10 19:25 ` [patch 17/30] NTB/msi: Use irq_has_action() Thomas Gleixner
2020-12-10 20:33   ` Logan Gunthorpe
2020-12-10 19:25 ` [patch 18/30] PCI: xilinx-nwl: Use irq_data_get_irq_chip_data() Thomas Gleixner
2020-12-10 22:56   ` Rob Herring
2020-12-10 19:25 ` [patch 19/30] PCI: mobiveil: " Thomas Gleixner
2020-12-10 22:56   ` Rob Herring
2020-12-10 19:25 ` [patch 20/30] net/mlx4: Replace irq_to_desc() abuse Thomas Gleixner
2020-12-13 11:24   ` Tariq Toukan
2020-12-10 19:25 ` [patch 21/30] net/mlx4: Use effective interrupt affinity Thomas Gleixner
2020-12-13 11:31   ` Tariq Toukan
2020-12-10 19:25 ` [patch 22/30] net/mlx5: Replace irq_to_desc() abuse Thomas Gleixner
2020-12-13 11:34   ` Tariq Toukan
2020-12-14 21:13   ` Saeed Mahameed
2020-12-10 19:25 ` [patch 23/30] net/mlx5: Use effective interrupt affinity Thomas Gleixner
2020-12-13 11:35   ` Tariq Toukan
2020-12-14 20:58   ` Saeed Mahameed
2020-12-10 19:26 ` [patch 24/30] xen/events: Remove unused bind_evtchn_to_irq_lateeoi() Thomas Gleixner
2020-12-10 23:19   ` boris.ostrovsky
2020-12-11  0:04     ` Thomas Gleixner
2020-12-10 19:26 ` [patch 25/30] xen/events: Remove disfunct affinity spreading Thomas Gleixner
2020-12-10 19:26 ` [patch 26/30] xen/events: Use immediate affinity setting Thomas Gleixner
2020-12-10 19:26 ` [patch 27/30] xen/events: Only force affinity mask for percpu interrupts Thomas Gleixner
2020-12-10 23:20   ` boris.ostrovsky
2020-12-11  0:06     ` Thomas Gleixner
2020-12-11  6:17     ` Jürgen Groß
     [not found]       ` <874kksiras.fsf@nanos.tec.linutronix.de>
2020-12-11 10:23         ` Jürgen Groß
2020-12-11 12:10     ` Jürgen Groß
2020-12-11 12:37       ` Thomas Gleixner
2020-12-11 14:29         ` boris.ostrovsky
2020-12-11 21:27           ` Thomas Gleixner
2020-12-11 22:21             ` Andrew Cooper
2020-12-11 22:56               ` Thomas Gleixner
2020-12-10 19:26 ` [patch 28/30] xen/events: Reduce irq_info::spurious_cnt storage size Thomas Gleixner
2020-12-10 19:26 ` [patch 29/30] xen/events: Implement irq distribution Thomas Gleixner
2020-12-10 19:26 ` [patch 30/30] genirq: Remove export of irq_to_desc() Thomas Gleixner

dri-devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dri-devel/0 dri-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dri-devel dri-devel/ https://lore.kernel.org/dri-devel \
		dri-devel@lists.freedesktop.org
	public-inbox-index dri-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.freedesktop.lists.dri-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git