2015-05-04 03:13:35

by Jiang Liu

[permalink] [raw]
Subject: [RFC v1 00/11] Split struct irq_data into common part and per-chip part

Hi all,
Now the irq core supports hierarchy irq and stacked irqchips,
so there may be multiple irq_datas associated with the same irq. But
some fields in struct irq_data are per-irq instance and duplicating
those fields into multiple irq_data may cause troubles.
So this patch introduces a new data structure 'struct
irq_common_data' to host per-irq instance data fields, and struct
irq_data will only host per-chip data fields after the conversion.

It's based on tip/x86/apic plus these two patches at:
https://www.mail-archive.com/[email protected]/msg879700.html

And it passes Fengguang's zeroday test suite.

Thanks!
Gerry

Jiang Liu (11):
genirq: Introduce struct irq_common_data to host shared irq data
genirq: Move field 'node' from struct irq_data into struct
irq_common_data
genirq: Use CONFIG_NUMA instead of CONFIG_SMP to guard
irq_common_data.node
genirq: Move field 'handler_data' from struct irq_data into struct
irq_common_data
mn10300: Fix incorrect use of data->affinity
genirq: Introduce helper function irq_data_get_affinity_mask()
net/mlx4: Cache irq_desc->affinity instead of irq_desc
genirq: Move field 'affinity' from struct irq_data into struct
irq_common_data
genirq: Use helper function to access irq_data->msi_desc
genirq: Move field 'msi_desc' from struct irq_data into struct
irq_common_data
genirq: Pass irq_data to helper function
__irq_set_chip_handler_name_locked()

arch/alpha/kernel/irq.c | 2 +-
arch/arm/kernel/irq.c | 4 +-
arch/arm64/kernel/irq.c | 4 +-
arch/blackfin/mach-common/ints-priority.c | 3 +-
arch/ia64/kernel/iosapic.c | 8 +-
arch/ia64/kernel/irq.c | 6 +-
arch/ia64/kernel/msi_ia64.c | 6 +-
arch/ia64/sn/kernel/msi_sn.c | 4 +-
arch/metag/kernel/irq.c | 10 ++-
arch/mips/alchemy/common/irq.c | 4 +-
arch/mips/bcm63xx/irq.c | 2 +-
arch/mips/cavium-octeon/octeon-irq.c | 14 ++--
arch/mips/pmcs-msp71xx/msp_irq_cic.c | 3 +-
arch/mn10300/kernel/cevt-mn10300.c | 2 +-
arch/mn10300/kernel/irq.c | 13 +--
arch/parisc/kernel/irq.c | 12 +--
arch/powerpc/kernel/irq.c | 2 +-
arch/powerpc/sysdev/xics/ics-opal.c | 4 +-
arch/powerpc/sysdev/xics/ics-rtas.c | 4 +-
arch/sh/kernel/irq.c | 9 ++-
arch/sparc/kernel/irq_64.c | 27 ++++---
arch/sparc/kernel/leon_kernel.c | 6 +-
arch/sparc/kernel/sun4d_irq.c | 4 +-
arch/sparc/kernel/sun4m_irq.c | 6 +-
arch/tile/kernel/pci_gx.c | 2 +-
arch/x86/kernel/apic/io_apic.c | 2 +-
arch/x86/kernel/apic/msi.c | 2 +-
arch/x86/kernel/apic/vector.c | 13 ++-
arch/x86/kernel/hpet.c | 4 +-
arch/x86/kernel/irq.c | 5 +-
arch/x86/platform/uv/uv_irq.c | 2 +-
arch/xtensa/kernel/irq.c | 10 ++-
drivers/gpio/gpio-davinci.c | 2 +-
drivers/gpio/gpio-zynq.c | 9 +--
drivers/irqchip/irq-metag-ext.c | 5 +-
drivers/irqchip/irq-mips-gic.c | 13 ++-
drivers/net/ethernet/mellanox/mlx4/en_cq.c | 6 +-
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 5 +-
drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 2 +-
drivers/parisc/iosapic.c | 2 +-
drivers/pci/msi.c | 2 +-
drivers/sh/intc/chip.c | 6 +-
drivers/sh/intc/virq.c | 4 +-
drivers/xen/events/events_base.c | 4 +-
include/linux/irq.h | 109 +++++++++++++++++---------
include/linux/irqdesc.h | 11 +--
kernel/irq/chip.c | 4 +-
kernel/irq/internals.h | 15 ++--
kernel/irq/irqdesc.c | 27 +++----
kernel/irq/irqdomain.c | 5 +-
kernel/irq/manage.c | 14 ++--
kernel/irq/proc.c | 4 +-
52 files changed, 250 insertions(+), 198 deletions(-)

--
1.7.10.4


2015-05-04 03:13:48

by Jiang Liu

[permalink] [raw]
Subject: [RFC v1 01/11] genirq: Introduce struct irq_common_data to host shared irq data

With introduction of hierarchy irqdomain, struct irq_data becomes
per-chip instead of per-irq and there may be multiple irq_datas
associated with the same irq. Some per-irq data stored in
struct irq_data now may get duplicated into multiple irq_datas,
and causes inconsistent view.

So introduce struct irq_common_data to host per-irq common data and to
achieve consistent view among irq_chips.

Other patches in this patch set will move common fields from
struct irq_data into struct irq_common_data one by one.

Signed-off-by: Jiang Liu <[email protected]>
---
include/linux/irq.h | 54 ++++++++++++++++++++++++++++-------------------
include/linux/irqdesc.h | 1 +
kernel/irq/internals.h | 10 ++++-----
kernel/irq/irqdesc.c | 1 +
kernel/irq/irqdomain.c | 1 +
5 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 62c6901cab55..3b6e0def7f5c 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -126,13 +126,21 @@ struct msi_desc;
struct irq_domain;

/**
- * struct irq_data - per irq and irq chip data passed down to chip functions
+ * struct irq_common_data - per irq data shared by all irqchips
+ * @state_use_accessors: status information for irq chip functions.
+ * Use accessor functions to deal with it
+ */
+struct irq_common_data {
+ unsigned int state_use_accessors;
+};
+
+/**
+ * struct irq_data - per irq chip data passed down to chip functions
* @mask: precomputed bitmask for accessing the chip registers
* @irq: interrupt number
* @hwirq: hardware interrupt number, local to the interrupt domain
* @node: node index useful for balancing
- * @state_use_accessors: status information for irq chip functions.
- * Use accessor functions to deal with it
+ * @common: point to data shared by all irqchips
* @chip: low level interrupt hardware access
* @domain: Interrupt translation domain; responsible for mapping
* between hwirq number and linux irq number.
@@ -153,7 +161,7 @@ struct irq_data {
unsigned int irq;
unsigned long hwirq;
unsigned int node;
- unsigned int state_use_accessors;
+ struct irq_common_data *common;
struct irq_chip *chip;
struct irq_domain *domain;
#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
@@ -166,7 +174,7 @@ struct irq_data {
};

/*
- * Bit masks for irq_data.state
+ * Bit masks for irq_common_data.state_use_accessors
*
* IRQD_TRIGGER_MASK - Mask for the trigger type bits
* IRQD_SETAFFINITY_PENDING - Affinity setting is pending
@@ -198,34 +206,36 @@ enum {
IRQD_WAKEUP_ARMED = (1 << 19),
};

+#define __irqd_to_state(d) ((d)->common->state_use_accessors)
+
static inline bool irqd_is_setaffinity_pending(struct irq_data *d)
{
- return d->state_use_accessors & IRQD_SETAFFINITY_PENDING;
+ return __irqd_to_state(d) & IRQD_SETAFFINITY_PENDING;
}

static inline bool irqd_is_per_cpu(struct irq_data *d)
{
- return d->state_use_accessors & IRQD_PER_CPU;
+ return __irqd_to_state(d) & IRQD_PER_CPU;
}

static inline bool irqd_can_balance(struct irq_data *d)
{
- return !(d->state_use_accessors & (IRQD_PER_CPU | IRQD_NO_BALANCING));
+ return !(__irqd_to_state(d) & (IRQD_PER_CPU | IRQD_NO_BALANCING));
}

static inline bool irqd_affinity_was_set(struct irq_data *d)
{
- return d->state_use_accessors & IRQD_AFFINITY_SET;
+ return __irqd_to_state(d) & IRQD_AFFINITY_SET;
}

static inline void irqd_mark_affinity_was_set(struct irq_data *d)
{
- d->state_use_accessors |= IRQD_AFFINITY_SET;
+ __irqd_to_state(d) |= IRQD_AFFINITY_SET;
}

static inline u32 irqd_get_trigger_type(struct irq_data *d)
{
- return d->state_use_accessors & IRQD_TRIGGER_MASK;
+ return __irqd_to_state(d) & IRQD_TRIGGER_MASK;
}

/*
@@ -233,43 +243,43 @@ static inline u32 irqd_get_trigger_type(struct irq_data *d)
*/
static inline void irqd_set_trigger_type(struct irq_data *d, u32 type)
{
- d->state_use_accessors &= ~IRQD_TRIGGER_MASK;
- d->state_use_accessors |= type & IRQD_TRIGGER_MASK;
+ __irqd_to_state(d) &= ~IRQD_TRIGGER_MASK;
+ __irqd_to_state(d) |= type & IRQD_TRIGGER_MASK;
}

static inline bool irqd_is_level_type(struct irq_data *d)
{
- return d->state_use_accessors & IRQD_LEVEL;
+ return __irqd_to_state(d) & IRQD_LEVEL;
}

static inline bool irqd_is_wakeup_set(struct irq_data *d)
{
- return d->state_use_accessors & IRQD_WAKEUP_STATE;
+ return __irqd_to_state(d) & IRQD_WAKEUP_STATE;
}

static inline bool irqd_can_move_in_process_context(struct irq_data *d)
{
- return d->state_use_accessors & IRQD_MOVE_PCNTXT;
+ return __irqd_to_state(d) & IRQD_MOVE_PCNTXT;
}

static inline bool irqd_irq_disabled(struct irq_data *d)
{
- return d->state_use_accessors & IRQD_IRQ_DISABLED;
+ return __irqd_to_state(d) & IRQD_IRQ_DISABLED;
}

static inline bool irqd_irq_masked(struct irq_data *d)
{
- return d->state_use_accessors & IRQD_IRQ_MASKED;
+ return __irqd_to_state(d) & IRQD_IRQ_MASKED;
}

static inline bool irqd_irq_inprogress(struct irq_data *d)
{
- return d->state_use_accessors & IRQD_IRQ_INPROGRESS;
+ return __irqd_to_state(d) & IRQD_IRQ_INPROGRESS;
}

static inline bool irqd_is_wakeup_armed(struct irq_data *d)
{
- return d->state_use_accessors & IRQD_WAKEUP_ARMED;
+ return __irqd_to_state(d) & IRQD_WAKEUP_ARMED;
}


@@ -280,12 +290,12 @@ static inline bool irqd_is_wakeup_armed(struct irq_data *d)
*/
static inline void irqd_set_chained_irq_inprogress(struct irq_data *d)
{
- d->state_use_accessors |= IRQD_IRQ_INPROGRESS;
+ __irqd_to_state(d) |= IRQD_IRQ_INPROGRESS;
}

static inline void irqd_clr_chained_irq_inprogress(struct irq_data *d)
{
- d->state_use_accessors &= ~IRQD_IRQ_INPROGRESS;
+ __irqd_to_state(d) &= ~IRQD_IRQ_INPROGRESS;
}

static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
index dd1109fb241e..3010e99abf3e 100644
--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -47,6 +47,7 @@ struct pt_regs;
* @name: flow handler name for /proc/interrupts output
*/
struct irq_desc {
+ struct irq_common_data irq_common_data;
struct irq_data irq_data;
unsigned int __percpu *kstat_irqs;
irq_flow_handler_t handle_irq;
diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
index df553b0af936..ed84299788b3 100644
--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h
@@ -170,27 +170,27 @@ irq_put_desc_unlock(struct irq_desc *desc, unsigned long flags)
*/
static inline void irqd_set_move_pending(struct irq_data *d)
{
- d->state_use_accessors |= IRQD_SETAFFINITY_PENDING;
+ __irqd_to_state(d) |= IRQD_SETAFFINITY_PENDING;
}

static inline void irqd_clr_move_pending(struct irq_data *d)
{
- d->state_use_accessors &= ~IRQD_SETAFFINITY_PENDING;
+ __irqd_to_state(d) &= ~IRQD_SETAFFINITY_PENDING;
}

static inline void irqd_clear(struct irq_data *d, unsigned int mask)
{
- d->state_use_accessors &= ~mask;
+ __irqd_to_state(d) &= ~mask;
}

static inline void irqd_set(struct irq_data *d, unsigned int mask)
{
- d->state_use_accessors |= mask;
+ __irqd_to_state(d) |= mask;
}

static inline bool irqd_has_set(struct irq_data *d, unsigned int mask)
{
- return d->state_use_accessors & mask;
+ return __irqd_to_state(d) & mask;
}

static inline void kstat_incr_irqs_this_cpu(unsigned int irq, struct irq_desc *desc)
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 99793b9b6d23..eac1aac906ea 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -76,6 +76,7 @@ static void desc_set_defaults(unsigned int irq, struct irq_desc *desc, int node,
{
int cpu;

+ desc->irq_data.common = &desc->irq_common_data;
desc->irq_data.irq = irq;
desc->irq_data.chip = &no_irq_chip;
desc->irq_data.chip_data = NULL;
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 7fac311057b8..3552b8750efd 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -834,6 +834,7 @@ static struct irq_data *irq_domain_insert_irq_data(struct irq_domain *domain,
if (irq_data) {
child->parent_data = irq_data;
irq_data->irq = child->irq;
+ irq_data->common = child->common;
irq_data->node = child->node;
irq_data->domain = domain;
}
--
1.7.10.4

2015-05-04 03:14:18

by Jiang Liu

[permalink] [raw]
Subject: [RFC v1 02/11] genirq: Move field 'node' from struct irq_data into struct irq_common_data

NUMA node information is per-irq instead of per-irqchip, so move it into
struct irq_common_data.

Signed-off-by: Jiang Liu <[email protected]>
---
arch/sh/kernel/irq.c | 2 +-
arch/x86/kernel/apic/vector.c | 8 ++++----
arch/x86/platform/uv/uv_irq.c | 2 +-
include/linux/irq.h | 20 ++++++++++++++++++--
kernel/irq/internals.h | 5 +++++
kernel/irq/irqdesc.c | 10 ++--------
kernel/irq/irqdomain.c | 4 ++--
kernel/irq/manage.c | 2 +-
kernel/irq/proc.c | 2 +-
9 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/arch/sh/kernel/irq.c b/arch/sh/kernel/irq.c
index eb10ff84015c..8dc677cc136b 100644
--- a/arch/sh/kernel/irq.c
+++ b/arch/sh/kernel/irq.c
@@ -227,7 +227,7 @@ void migrate_irqs(void)
for_each_active_irq(irq) {
struct irq_data *data = irq_get_irq_data(irq);

- if (data->node == cpu) {
+ if (irq_data_get_node(data) == cpu) {
unsigned int newcpu = cpumask_any_and(data->affinity,
cpu_online_mask);
if (newcpu >= nr_cpu_ids) {
diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
index 96ce5068a926..983bea2a09ce 100644
--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -345,7 +345,7 @@ static int x86_vector_alloc_irqs(struct irq_domain *domain, unsigned int virq,
struct irq_alloc_info *info = arg;
struct apic_chip_data *data;
struct irq_data *irq_data;
- int i, err;
+ int i, err, node;

if (disable_apic)
return -ENXIO;
@@ -357,12 +357,13 @@ static int x86_vector_alloc_irqs(struct irq_domain *domain, unsigned int virq,
for (i = 0; i < nr_irqs; i++) {
irq_data = irq_domain_get_irq_data(domain, virq + i);
BUG_ON(!irq_data);
+ node = irq_data_get_node(irq_data);
#ifdef CONFIG_X86_IO_APIC
if (virq + i < nr_legacy_irqs() && legacy_irq_data[virq + i])
data = legacy_irq_data[virq + i];
else
#endif
- data = alloc_apic_chip_data(irq_data->node);
+ data = alloc_apic_chip_data(node);
if (!data) {
err = -ENOMEM;
goto error;
@@ -371,8 +372,7 @@ static int x86_vector_alloc_irqs(struct irq_domain *domain, unsigned int virq,
irq_data->chip = &lapic_controller;
irq_data->chip_data = data;
irq_data->hwirq = virq + i;
- err = assign_irq_vector_policy(virq, irq_data->node, data,
- info);
+ err = assign_irq_vector_policy(virq, node, data, info);
if (err)
goto error;
}
diff --git a/arch/x86/platform/uv/uv_irq.c b/arch/x86/platform/uv/uv_irq.c
index cdf86cd3fd97..bc992b7b041f 100644
--- a/arch/x86/platform/uv/uv_irq.c
+++ b/arch/x86/platform/uv/uv_irq.c
@@ -89,7 +89,7 @@ static int uv_domain_alloc(struct irq_domain *domain, unsigned int virq,
return -EINVAL;

chip_data = kmalloc_node(sizeof(*chip_data), GFP_KERNEL,
- irq_data->node);
+ irq_data_get_node(irq_data));
if (!chip_data)
return -ENOMEM;

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 3b6e0def7f5c..3f999a0af713 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -129,9 +129,13 @@ struct irq_domain;
* struct irq_common_data - per irq data shared by all irqchips
* @state_use_accessors: status information for irq chip functions.
* Use accessor functions to deal with it
+ * @node: node index useful for balancing
*/
struct irq_common_data {
unsigned int state_use_accessors;
+#ifdef CONFIG_SMP
+ unsigned int node;
+#endif
};

/**
@@ -139,7 +143,6 @@ struct irq_common_data {
* @mask: precomputed bitmask for accessing the chip registers
* @irq: interrupt number
* @hwirq: hardware interrupt number, local to the interrupt domain
- * @node: node index useful for balancing
* @common: point to data shared by all irqchips
* @chip: low level interrupt hardware access
* @domain: Interrupt translation domain; responsible for mapping
@@ -160,7 +163,6 @@ struct irq_data {
u32 mask;
unsigned int irq;
unsigned long hwirq;
- unsigned int node;
struct irq_common_data *common;
struct irq_chip *chip;
struct irq_domain *domain;
@@ -634,6 +636,20 @@ static inline u32 irq_get_trigger_type(unsigned int irq)
return d ? irqd_get_trigger_type(d) : 0;
}

+static inline int irq_common_data_get_node(struct irq_common_data *d)
+{
+#ifdef CONFIG_SMP
+ return d->node;
+#else
+ return 0;
+#endif
+}
+
+static inline int irq_data_get_node(struct irq_data *d)
+{
+ return irq_common_data_get_node(d->common);
+}
+
unsigned int arch_dynirq_lower_bound(unsigned int from);

int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
index ed84299788b3..d82a77d39aeb 100644
--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h
@@ -199,6 +199,11 @@ static inline void kstat_incr_irqs_this_cpu(unsigned int irq, struct irq_desc *d
__this_cpu_inc(kstat.irqs_sum);
}

+static inline int irq_desc_get_node(struct irq_desc *desc)
+{
+ return irq_common_data_get_node(&desc->irq_common_data);
+}
+
#ifdef CONFIG_PM_SLEEP
bool irq_pm_check_wakeup(struct irq_desc *desc);
void irq_pm_install_action(struct irq_desc *desc, struct irqaction *action);
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index eac1aac906ea..20773f073e24 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -52,23 +52,17 @@ static int alloc_masks(struct irq_desc *desc, gfp_t gfp, int node)

static void desc_smp_init(struct irq_desc *desc, int node)
{
- desc->irq_data.node = node;
+ desc->irq_common_data.node = node;
cpumask_copy(desc->irq_data.affinity, irq_default_affinity);
#ifdef CONFIG_GENERIC_PENDING_IRQ
cpumask_clear(desc->pending_mask);
#endif
}

-static inline int desc_node(struct irq_desc *desc)
-{
- return desc->irq_data.node;
-}
-
#else
static inline int
alloc_masks(struct irq_desc *desc, gfp_t gfp, int node) { return 0; }
static inline void desc_smp_init(struct irq_desc *desc, int node) { }
-static inline int desc_node(struct irq_desc *desc) { return 0; }
#endif

static void desc_set_defaults(unsigned int irq, struct irq_desc *desc, int node,
@@ -300,7 +294,7 @@ static void free_desc(unsigned int irq)
unsigned long flags;

raw_spin_lock_irqsave(&desc->lock, flags);
- desc_set_defaults(irq, desc, desc_node(desc), NULL);
+ desc_set_defaults(irq, desc, irq_desc_get_node(desc), NULL);
raw_spin_unlock_irqrestore(&desc->lock, flags);
}

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 3552b8750efd..93ef89059022 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -830,12 +830,12 @@ static struct irq_data *irq_domain_insert_irq_data(struct irq_domain *domain,
{
struct irq_data *irq_data;

- irq_data = kzalloc_node(sizeof(*irq_data), GFP_KERNEL, child->node);
+ irq_data = kzalloc_node(sizeof(*irq_data), GFP_KERNEL,
+ irq_data_get_node(child));
if (irq_data) {
child->parent_data = irq_data;
irq_data->irq = child->irq;
irq_data->common = child->common;
- irq_data->node = child->node;
irq_data->domain = domain;
}

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index e68932bb308e..d206aa2c3378 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -332,7 +332,7 @@ static int
setup_affinity(unsigned int irq, struct irq_desc *desc, struct cpumask *mask)
{
struct cpumask *set = irq_default_affinity;
- int node = desc->irq_data.node;
+ int node = irq_desc_get_node(desc);

/* Excludes PER_CPU and NO_BALANCE interrupts */
if (!irq_can_set_affinity(irq))
diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c
index df2f4642d1e7..0e97c142ce40 100644
--- a/kernel/irq/proc.c
+++ b/kernel/irq/proc.c
@@ -241,7 +241,7 @@ static int irq_node_proc_show(struct seq_file *m, void *v)
{
struct irq_desc *desc = irq_to_desc((long) m->private);

- seq_printf(m, "%d\n", desc->irq_data.node);
+ seq_printf(m, "%d\n", irq_desc_get_node(desc));
return 0;
}

--
1.7.10.4

2015-05-04 03:14:35

by Jiang Liu

[permalink] [raw]
Subject: [RFC v1 03/11] genirq: Use CONFIG_NUMA instead of CONFIG_SMP to guard irq_common_data.node

NUMA is enabled by CONFIG_NUMA instead of CONFIG_SMP, so use CONFIG_NUMA
to guard irq_common_data.node.

Signed-off-by: Jiang Liu <[email protected]>
---
include/linux/irq.h | 4 ++--
kernel/irq/irqdesc.c | 4 +++-
2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 3f999a0af713..0138f046642c 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -133,7 +133,7 @@ struct irq_domain;
*/
struct irq_common_data {
unsigned int state_use_accessors;
-#ifdef CONFIG_SMP
+#ifdef CONFIG_NUMA
unsigned int node;
#endif
};
@@ -638,7 +638,7 @@ static inline u32 irq_get_trigger_type(unsigned int irq)

static inline int irq_common_data_get_node(struct irq_common_data *d)
{
-#ifdef CONFIG_SMP
+#ifdef CONFIG_NUMA
return d->node;
#else
return 0;
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 20773f073e24..0c3057e42906 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -52,11 +52,13 @@ static int alloc_masks(struct irq_desc *desc, gfp_t gfp, int node)

static void desc_smp_init(struct irq_desc *desc, int node)
{
- desc->irq_common_data.node = node;
cpumask_copy(desc->irq_data.affinity, irq_default_affinity);
#ifdef CONFIG_GENERIC_PENDING_IRQ
cpumask_clear(desc->pending_mask);
#endif
+#ifdef CONFIG_NUMA
+ desc->irq_common_data.node = node;
+#endif
}

#else
--
1.7.10.4

2015-05-04 03:15:16

by Jiang Liu

[permalink] [raw]
Subject: [RFC v1 04/11] genirq: Move field 'handler_data' from struct irq_data into struct irq_common_data

Handler data (handler_data) is per-irq instead of per irqchip, so move
it into struct irq_common_data.

Signed-off-by: Jiang Liu <[email protected]>
---
arch/sparc/kernel/irq_64.c | 15 +++++++++------
arch/sparc/kernel/sun4d_irq.c | 4 ++--
arch/sparc/kernel/sun4m_irq.c | 6 ++++--
arch/x86/kernel/apic/msi.c | 2 +-
arch/x86/kernel/hpet.c | 4 ++--
drivers/gpio/gpio-davinci.c | 2 +-
drivers/sh/intc/virq.c | 4 ++--
include/linux/irq.h | 8 ++++----
include/linux/irqdesc.h | 2 +-
kernel/irq/chip.c | 2 +-
kernel/irq/irqdesc.c | 3 ++-
11 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/arch/sparc/kernel/irq_64.c b/arch/sparc/kernel/irq_64.c
index 4033c23bdfa6..5130f6e3e68e 100644
--- a/arch/sparc/kernel/irq_64.c
+++ b/arch/sparc/kernel/irq_64.c
@@ -210,21 +210,21 @@ struct irq_handler_data {

static inline unsigned int irq_data_to_handle(struct irq_data *data)
{
- struct irq_handler_data *ihd = data->handler_data;
+ struct irq_handler_data *ihd = irq_data_get_irq_handler_data(data);

return ihd->dev_handle;
}

static inline unsigned int irq_data_to_ino(struct irq_data *data)
{
- struct irq_handler_data *ihd = data->handler_data;
+ struct irq_handler_data *ihd = irq_data_get_irq_handler_data(data);

return ihd->dev_ino;
}

static inline unsigned long irq_data_to_sysino(struct irq_data *data)
{
- struct irq_handler_data *ihd = data->handler_data;
+ struct irq_handler_data *ihd = irq_data_get_irq_handler_data(data);

return ihd->sysino;
}
@@ -370,8 +370,9 @@ static int irq_choose_cpu(unsigned int irq, const struct cpumask *affinity)

static void sun4u_irq_enable(struct irq_data *data)
{
- struct irq_handler_data *handler_data = data->handler_data;
+ struct irq_handler_data *handler_data;

+ handler_data = irq_data_get_irq_handler_data(data);
if (likely(handler_data)) {
unsigned long cpuid, imap, val;
unsigned int tid;
@@ -393,8 +394,9 @@ static void sun4u_irq_enable(struct irq_data *data)
static int sun4u_set_affinity(struct irq_data *data,
const struct cpumask *mask, bool force)
{
- struct irq_handler_data *handler_data = data->handler_data;
+ struct irq_handler_data *handler_data;

+ handler_data = irq_data_get_irq_handler_data(data);
if (likely(handler_data)) {
unsigned long cpuid, imap, val;
unsigned int tid;
@@ -438,8 +440,9 @@ static void sun4u_irq_disable(struct irq_data *data)

static void sun4u_irq_eoi(struct irq_data *data)
{
- struct irq_handler_data *handler_data = data->handler_data;
+ struct irq_handler_data *handler_data;

+ handler_data = irq_data_get_irq_handler_data(data);
if (likely(handler_data))
upa_writeq(ICLR_IDLE, handler_data->iclr);
}
diff --git a/arch/sparc/kernel/sun4d_irq.c b/arch/sparc/kernel/sun4d_irq.c
index a1bb2675b280..a87d0e47c168 100644
--- a/arch/sparc/kernel/sun4d_irq.c
+++ b/arch/sparc/kernel/sun4d_irq.c
@@ -188,7 +188,7 @@ void sun4d_handler_irq(unsigned int pil, struct pt_regs *regs)

static void sun4d_mask_irq(struct irq_data *data)
{
- struct sun4d_handler_data *handler_data = data->handler_data;
+ struct sun4d_handler_data *handler_data = irq_data_get_irq_handler_data(data);
unsigned int real_irq;
#ifdef CONFIG_SMP
int cpuid = handler_data->cpuid;
@@ -206,7 +206,7 @@ static void sun4d_mask_irq(struct irq_data *data)

static void sun4d_unmask_irq(struct irq_data *data)
{
- struct sun4d_handler_data *handler_data = data->handler_data;
+ struct sun4d_handler_data *handler_data = irq_data_get_irq_handler_data(data);
unsigned int real_irq;
#ifdef CONFIG_SMP
int cpuid = handler_data->cpuid;
diff --git a/arch/sparc/kernel/sun4m_irq.c b/arch/sparc/kernel/sun4m_irq.c
index 8bb3b3fddea7..da737c712fa8 100644
--- a/arch/sparc/kernel/sun4m_irq.c
+++ b/arch/sparc/kernel/sun4m_irq.c
@@ -188,9 +188,10 @@ static unsigned long sun4m_imask[0x50] = {

static void sun4m_mask_irq(struct irq_data *data)
{
- struct sun4m_handler_data *handler_data = data->handler_data;
+ struct sun4m_handler_data *handler_data;
int cpu = smp_processor_id();

+ handler_data = irq_data_get_irq_handler_data(data);
if (handler_data->mask) {
unsigned long flags;

@@ -206,9 +207,10 @@ static void sun4m_mask_irq(struct irq_data *data)

static void sun4m_unmask_irq(struct irq_data *data)
{
- struct sun4m_handler_data *handler_data = data->handler_data;
+ struct sun4m_handler_data *handler_data;
int cpu = smp_processor_id();

+ handler_data = irq_data_get_irq_handler_data(data);
if (handler_data->mask) {
unsigned long flags;

diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c
index 58fde664e7c0..e87655fcf30e 100644
--- a/arch/x86/kernel/apic/msi.c
+++ b/arch/x86/kernel/apic/msi.c
@@ -263,7 +263,7 @@ static inline int hpet_dev_id(struct irq_domain *domain)

static void hpet_msi_write_msg(struct irq_data *data, struct msi_msg *msg)
{
- hpet_msi_write(data->handler_data, msg);
+ hpet_msi_write(irq_data_get_irq_handler_data(data), msg);
}

static struct irq_chip hpet_msi_controller = {
diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
index e2449cf38b06..40cf79a55fe7 100644
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -426,7 +426,7 @@ static struct irq_domain *hpet_domain;

void hpet_msi_unmask(struct irq_data *data)
{
- struct hpet_dev *hdev = data->handler_data;
+ struct hpet_dev *hdev = irq_data_get_irq_handler_data(data);
unsigned int cfg;

/* unmask it */
@@ -437,7 +437,7 @@ void hpet_msi_unmask(struct irq_data *data)

void hpet_msi_mask(struct irq_data *data)
{
- struct hpet_dev *hdev = data->handler_data;
+ struct hpet_dev *hdev = irq_data_get_irq_handler_data(data);
unsigned int cfg;

/* mask it */
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index c5e05c82d67c..477d5b8616ab 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -396,7 +396,7 @@ static int gpio_irq_type_unbanked(struct irq_data *data, unsigned trigger)
struct davinci_gpio_regs __iomem *g;
u32 mask;

- d = (struct davinci_gpio_controller *)data->handler_data;
+ d = (struct davinci_gpio_controller *)irq_data_get_irq_handler_data(data);
g = (struct davinci_gpio_regs __iomem *)d->regs;
mask = __gpio_mask(data->irq - d->gpio_irq);

diff --git a/drivers/sh/intc/virq.c b/drivers/sh/intc/virq.c
index f30ac9354ff2..3dbc21696924 100644
--- a/drivers/sh/intc/virq.c
+++ b/drivers/sh/intc/virq.c
@@ -87,8 +87,8 @@ static int add_virq_to_pirq(unsigned int irq, unsigned int virq)
struct irq_data *data = irq_get_irq_data(irq);

/* scan for duplicates */
- last = (struct intc_virq_list **)&data->handler_data;
- for_each_virq(entry, data->handler_data) {
+ last = (struct intc_virq_list **)&data->common->handler_data;
+ for_each_virq(entry, irq_data_get_irq_handler_data(data)) {
if (entry->irq == virq)
return 0;
last = &entry->next;
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 0138f046642c..43581e166298 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -130,12 +130,14 @@ struct irq_domain;
* @state_use_accessors: status information for irq chip functions.
* Use accessor functions to deal with it
* @node: node index useful for balancing
+ * @handler_data: per-IRQ data for the irq_chip methods
*/
struct irq_common_data {
unsigned int state_use_accessors;
#ifdef CONFIG_NUMA
unsigned int node;
#endif
+ void *handler_data;
};

/**
@@ -149,7 +151,6 @@ struct irq_common_data {
* between hwirq number and linux irq number.
* @parent_data: pointer to parent struct irq_data to support hierarchy
* irq_domain
- * @handler_data: per-IRQ data for the irq_chip methods
* @chip_data: platform-specific per-chip private data for the chip
* methods, to allow shared chip implementations
* @msi_desc: MSI descriptor
@@ -169,7 +170,6 @@ struct irq_data {
#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
struct irq_data *parent_data;
#endif
- void *handler_data;
void *chip_data;
struct msi_desc *msi_desc;
cpumask_var_t affinity;
@@ -611,12 +611,12 @@ static inline void *irq_data_get_irq_chip_data(struct irq_data *d)
static inline void *irq_get_handler_data(unsigned int irq)
{
struct irq_data *d = irq_get_irq_data(irq);
- return d ? d->handler_data : NULL;
+ return d ? d->common->handler_data : NULL;
}

static inline void *irq_data_get_irq_handler_data(struct irq_data *d)
{
- return d->handler_data;
+ return d->common->handler_data;
}

static inline struct msi_desc *irq_get_msi_desc(unsigned int irq)
diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
index 3010e99abf3e..5d5776f40129 100644
--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -111,7 +111,7 @@ static inline void *irq_desc_get_chip_data(struct irq_desc *desc)

static inline void *irq_desc_get_handler_data(struct irq_desc *desc)
{
- return desc->irq_data.handler_data;
+ return desc->irq_common_data.handler_data;
}

static inline struct msi_desc *irq_desc_get_msi_desc(struct irq_desc *desc)
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index eb9a4ea394ab..46c487fc71dd 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -83,7 +83,7 @@ int irq_set_handler_data(unsigned int irq, void *data)

if (!desc)
return -EINVAL;
- desc->irq_data.handler_data = data;
+ desc->irq_common_data.handler_data = data;
irq_put_desc_unlock(desc, flags);
return 0;
}
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 0c3057e42906..7f881792ad5f 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -72,11 +72,12 @@ static void desc_set_defaults(unsigned int irq, struct irq_desc *desc, int node,
{
int cpu;

+ desc->irq_common_data.handler_data = NULL;
+
desc->irq_data.common = &desc->irq_common_data;
desc->irq_data.irq = irq;
desc->irq_data.chip = &no_irq_chip;
desc->irq_data.chip_data = NULL;
- desc->irq_data.handler_data = NULL;
desc->irq_data.msi_desc = NULL;
irq_settings_clr_and_set(desc, ~0, _IRQ_DEFAULT_INIT_FLAGS);
irqd_set(&desc->irq_data, IRQD_IRQ_DISABLED);
--
1.7.10.4

2015-05-04 03:15:23

by Jiang Liu

[permalink] [raw]
Subject: [RFC v1 05/11] mn10300: Fix incorrect use of data->affinity

The field affinity in struct irq_data is type of cpumask_var_t, so
we should pass in data->affinity instead of &data->affinity when
calling cpumask_xxxx().

Signed-off-by: Jiang Liu <[email protected]>
---
arch/mn10300/kernel/irq.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/mn10300/kernel/irq.c b/arch/mn10300/kernel/irq.c
index 6ab3b73efcf8..480de70f4059 100644
--- a/arch/mn10300/kernel/irq.c
+++ b/arch/mn10300/kernel/irq.c
@@ -320,11 +320,11 @@ void migrate_irqs(void)
if (irqd_is_per_cpu(data))
continue;

- if (cpumask_test_cpu(self, &data->affinity) &&
+ if (cpumask_test_cpu(self, data->affinity) &&
!cpumask_intersects(&irq_affinity[irq], cpu_online_mask)) {
int cpu_id;
cpu_id = cpumask_first(cpu_online_mask);
- cpumask_set_cpu(cpu_id, &data->affinity);
+ cpumask_set_cpu(cpu_id, data->affinity);
}
/* We need to operate irq_affinity_online atomically. */
arch_local_cli_save(flags);
@@ -335,7 +335,7 @@ void migrate_irqs(void)
GxICR(irq) = x & GxICR_LEVEL;
tmp = GxICR(irq);

- new = cpumask_any_and(&data->affinity,
+ new = cpumask_any_and(data->affinity,
cpu_online_mask);
irq_affinity_online[irq] = new;

--
1.7.10.4

2015-05-04 03:17:06

by Jiang Liu

[permalink] [raw]
Subject: [RFC v1 07/11] net/mlx4: Cache irq_desc->affinity instead of irq_desc

The field 'affinity' in irq_desc won't change once the irq_desc data
structure is created. So cache irq_desc->affinity instead of irq_desc.
This also helps to hide struct irq_desc from device drivers.

Signed-off-by: Jiang Liu <[email protected]>
---
drivers/net/ethernet/mellanox/mlx4/en_cq.c | 6 +++---
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 5 +----
drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 2 +-
3 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_cq.c b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
index 22da4d0d0f05..a03a01625398 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
@@ -31,6 +31,7 @@
*
*/

+#include <linux/irq.h>
#include <linux/mlx4/cq.h>
#include <linux/mlx4/qp.h>
#include <linux/mlx4/cmd.h>
@@ -135,9 +136,8 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
mdev->dev->caps.num_comp_vectors;
}

- cq->irq_desc =
- irq_to_desc(mlx4_eq_get_irq(mdev->dev,
- cq->vector));
+ cq->irq_affinity = irq_get_affinity_mask(
+ mlx4_eq_get_irq(mdev->dev, cq->vector));
} else {
/* For TX we use the same irq per
ring we assigned for the RX */
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 4fdd3c37e47b..59d8395ef31c 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -1022,14 +1022,11 @@ int mlx4_en_poll_rx_cq(struct napi_struct *napi, int budget)
/* If we used up all the quota - we're probably not done yet... */
if (done == budget) {
int cpu_curr;
- const struct cpumask *aff;

INC_PERF_COUNTER(priv->pstats.napi_quota);

cpu_curr = smp_processor_id();
- aff = irq_desc_get_irq_data(cq->irq_desc)->affinity;
-
- if (likely(cpumask_test_cpu(cpu_curr, aff)))
+ if (likely(cpumask_test_cpu(cpu_curr, cq->irq_affinity)))
return budget;

/* Current cpu is not according to smp_irq_affinity -
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index 9de30216b146..5de5e9a51d76 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -357,7 +357,7 @@ struct mlx4_en_cq {
#define CQ_USER_PEND (MLX4_EN_CQ_STATE_POLL | MLX4_EN_CQ_STATE_POLL_YIELD)
spinlock_t poll_lock; /* protects from LLS/napi conflicts */
#endif /* CONFIG_NET_RX_BUSY_POLL */
- struct irq_desc *irq_desc;
+ struct cpumask *irq_affinity;
};

struct mlx4_en_port_profile {
--
1.7.10.4

2015-05-04 03:17:14

by Jiang Liu

[permalink] [raw]
Subject: [RFC v1 08/11] genirq: Move field 'affinity' from struct irq_data into struct irq_common_data

Irq affinity mask is per-irq instead of per irqchip, so move it into
struct irq_common_data.

Signed-off-by: Jiang Liu <[email protected]>
---
include/linux/irq.h | 12 ++++++------
kernel/irq/irqdesc.c | 9 +++++----
kernel/irq/manage.c | 12 ++++++------
kernel/irq/proc.c | 2 +-
4 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index cf386d97b96d..6ac20b3f3053 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -110,8 +110,8 @@ enum {
/*
* Return value for chip->irq_set_affinity()
*
- * IRQ_SET_MASK_OK - OK, core updates irq_data.affinity
- * IRQ_SET_MASK_NOCPY - OK, chip did update irq_data.affinity
+ * IRQ_SET_MASK_OK - OK, core updates irq_common_data.affinity
+ * IRQ_SET_MASK_NOCPY - OK, chip did update irq_common_data.affinity
* IRQ_SET_MASK_OK_DONE - Same as IRQ_SET_MASK_OK for core. Special code to
* support stacked irqchips, which indicates skipping
* all descendent irqchips.
@@ -131,6 +131,7 @@ struct irq_domain;
* Use accessor functions to deal with it
* @node: node index useful for balancing
* @handler_data: per-IRQ data for the irq_chip methods
+ * @affinity: IRQ affinity on SMP
*/
struct irq_common_data {
unsigned int state_use_accessors;
@@ -138,6 +139,7 @@ struct irq_common_data {
unsigned int node;
#endif
void *handler_data;
+ cpumask_var_t affinity;
};

/**
@@ -154,7 +156,6 @@ struct irq_common_data {
* @chip_data: platform-specific per-chip private data for the chip
* methods, to allow shared chip implementations
* @msi_desc: MSI descriptor
- * @affinity: IRQ affinity on SMP
*
* The fields here need to overlay the ones in irq_desc until we
* cleaned up the direct references and switched everything over to
@@ -172,7 +173,6 @@ struct irq_data {
#endif
void *chip_data;
struct msi_desc *msi_desc;
- cpumask_var_t affinity;
};

/*
@@ -653,12 +653,12 @@ static inline int irq_data_get_node(struct irq_data *d)
static inline struct cpumask *irq_get_affinity_mask(int irq)
{
struct irq_data *d = irq_get_irq_data(irq);
- return d ? d->affinity : NULL;
+ return d ? d->common->affinity : NULL;
}

static inline struct cpumask *irq_data_get_affinity_mask(struct irq_data *d)
{
- return d->affinity;
+ return d->common->affinity;
}

unsigned int arch_dynirq_lower_bound(unsigned int from);
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 7f881792ad5f..2c39a203f78b 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -38,12 +38,13 @@ static void __init init_irq_default_affinity(void)
#ifdef CONFIG_SMP
static int alloc_masks(struct irq_desc *desc, gfp_t gfp, int node)
{
- if (!zalloc_cpumask_var_node(&desc->irq_data.affinity, gfp, node))
+ if (!zalloc_cpumask_var_node(&desc->irq_common_data.affinity,
+ gfp, node))
return -ENOMEM;

#ifdef CONFIG_GENERIC_PENDING_IRQ
if (!zalloc_cpumask_var_node(&desc->pending_mask, gfp, node)) {
- free_cpumask_var(desc->irq_data.affinity);
+ free_cpumask_var(desc->irq_common_data.affinity);
return -ENOMEM;
}
#endif
@@ -52,7 +53,7 @@ static int alloc_masks(struct irq_desc *desc, gfp_t gfp, int node)

static void desc_smp_init(struct irq_desc *desc, int node)
{
- cpumask_copy(desc->irq_data.affinity, irq_default_affinity);
+ cpumask_copy(desc->irq_common_data.affinity, irq_default_affinity);
#ifdef CONFIG_GENERIC_PENDING_IRQ
cpumask_clear(desc->pending_mask);
#endif
@@ -124,7 +125,7 @@ static void free_masks(struct irq_desc *desc)
#ifdef CONFIG_GENERIC_PENDING_IRQ
free_cpumask_var(desc->pending_mask);
#endif
- free_cpumask_var(desc->irq_data.affinity);
+ free_cpumask_var(desc->irq_common_data.affinity);
}
#else
static inline void free_masks(struct irq_desc *desc) { }
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index d206aa2c3378..7588bf87e8f3 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -190,7 +190,7 @@ int irq_do_set_affinity(struct irq_data *data, const struct cpumask *mask,
switch (ret) {
case IRQ_SET_MASK_OK:
case IRQ_SET_MASK_OK_DONE:
- cpumask_copy(data->affinity, mask);
+ cpumask_copy(desc->irq_common_data.affinity, mask);
case IRQ_SET_MASK_OK_NOCOPY:
irq_set_thread_affinity(desc);
ret = 0;
@@ -271,7 +271,7 @@ static void irq_affinity_notify(struct work_struct *work)
if (irq_move_pending(&desc->irq_data))
irq_get_pending(cpumask, desc);
else
- cpumask_copy(cpumask, desc->irq_data.affinity);
+ cpumask_copy(cpumask, desc->irq_common_data.affinity);
raw_spin_unlock_irqrestore(&desc->lock, flags);

notify->notify(notify, cpumask);
@@ -343,9 +343,9 @@ setup_affinity(unsigned int irq, struct irq_desc *desc, struct cpumask *mask)
* one of the targets is online.
*/
if (irqd_has_set(&desc->irq_data, IRQD_AFFINITY_SET)) {
- if (cpumask_intersects(desc->irq_data.affinity,
+ if (cpumask_intersects(desc->irq_common_data.affinity,
cpu_online_mask))
- set = desc->irq_data.affinity;
+ set = desc->irq_common_data.affinity;
else
irqd_clear(&desc->irq_data, IRQD_AFFINITY_SET);
}
@@ -796,8 +796,8 @@ irq_thread_check_affinity(struct irq_desc *desc, struct irqaction *action)
* This code is triggered unconditionally. Check the affinity
* mask pointer. For CPU_MASK_OFFSTACK=n this is optimized out.
*/
- if (desc->irq_data.affinity)
- cpumask_copy(mask, desc->irq_data.affinity);
+ if (desc->irq_common_data.affinity)
+ cpumask_copy(mask, desc->irq_common_data.affinity);
else
valid = false;
raw_spin_unlock_irq(&desc->lock);
diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c
index 0e97c142ce40..e3a8c9577ba6 100644
--- a/kernel/irq/proc.c
+++ b/kernel/irq/proc.c
@@ -39,7 +39,7 @@ static struct proc_dir_entry *root_irq_dir;
static int show_irq_affinity(int type, struct seq_file *m, void *v)
{
struct irq_desc *desc = irq_to_desc((long)m->private);
- const struct cpumask *mask = desc->irq_data.affinity;
+ const struct cpumask *mask = desc->irq_common_data.affinity;

#ifdef CONFIG_GENERIC_PENDING_IRQ
if (irqd_is_setaffinity_pending(&desc->irq_data))
--
1.7.10.4

2015-05-04 03:17:43

by Jiang Liu

[permalink] [raw]
Subject: [RFC v1 09/11] genirq: Use helper function to access irq_data->msi_desc

Use helper function to access irq_data->msi_desc, so we could move
msi_desc from struct irq_data into struct irq_common_data later.

Signed-off-by: Jiang Liu <[email protected]>
---
arch/ia64/kernel/msi_ia64.c | 2 +-
arch/ia64/sn/kernel/msi_sn.c | 2 +-
arch/powerpc/sysdev/xics/ics-opal.c | 2 +-
arch/powerpc/sysdev/xics/ics-rtas.c | 2 +-
arch/tile/kernel/pci_gx.c | 2 +-
drivers/pci/msi.c | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/ia64/kernel/msi_ia64.c b/arch/ia64/kernel/msi_ia64.c
index 6c50d332b7d7..591b66d21bd1 100644
--- a/arch/ia64/kernel/msi_ia64.c
+++ b/arch/ia64/kernel/msi_ia64.c
@@ -23,7 +23,7 @@ static int ia64_set_msi_irq_affinity(struct irq_data *idata,
if (irq_prepare_move(irq, cpu))
return -1;

- __get_cached_msi_msg(idata->msi_desc, &msg);
+ __get_cached_msi_msg(irq_data_get_msi(idata), &msg);

addr = msg.address_lo;
addr &= MSI_ADDR_DEST_ID_MASK;
diff --git a/arch/ia64/sn/kernel/msi_sn.c b/arch/ia64/sn/kernel/msi_sn.c
index 42b5a13af142..6ccc39b36947 100644
--- a/arch/ia64/sn/kernel/msi_sn.c
+++ b/arch/ia64/sn/kernel/msi_sn.c
@@ -175,7 +175,7 @@ static int sn_set_msi_irq_affinity(struct irq_data *data,
* Release XIO resources for the old MSI PCI address
*/

- __get_cached_msi_msg(data->msi_desc, &msg);
+ __get_cached_msi_msg(irq_data_get_msi(data), &msg);
sn_pdev = (struct pcidev_info *)sn_irq_info->irq_pciioinfo;
pdev = sn_pdev->pdi_linux_pcidev;
provider = SN_PCIDEV_BUSPROVIDER(pdev);
diff --git a/arch/powerpc/sysdev/xics/ics-opal.c b/arch/powerpc/sysdev/xics/ics-opal.c
index 3996393c254d..a709d81f885a 100644
--- a/arch/powerpc/sysdev/xics/ics-opal.c
+++ b/arch/powerpc/sysdev/xics/ics-opal.c
@@ -72,7 +72,7 @@ static unsigned int ics_opal_startup(struct irq_data *d)
* card, using the MSI mask bits. Firmware doesn't appear to unmask
* at that level, so we do it here by hand.
*/
- if (d->msi_desc)
+ if (irq_data_get_msi(d))
pci_msi_unmask_irq(d);
#endif

diff --git a/arch/powerpc/sysdev/xics/ics-rtas.c b/arch/powerpc/sysdev/xics/ics-rtas.c
index e2665a9dfc0f..f09a7b3136b2 100644
--- a/arch/powerpc/sysdev/xics/ics-rtas.c
+++ b/arch/powerpc/sysdev/xics/ics-rtas.c
@@ -75,7 +75,7 @@ static unsigned int ics_rtas_startup(struct irq_data *d)
* card, using the MSI mask bits. Firmware doesn't appear to unmask
* at that level, so we do it here by hand.
*/
- if (d->msi_desc)
+ if (irq_data_get_msi(d))
pci_msi_unmask_irq(d);
#endif
/* unmask it */
diff --git a/arch/tile/kernel/pci_gx.c b/arch/tile/kernel/pci_gx.c
index b1df847d0686..1438c60e263d 100644
--- a/arch/tile/kernel/pci_gx.c
+++ b/arch/tile/kernel/pci_gx.c
@@ -1442,7 +1442,7 @@ static struct pci_ops tile_cfg_ops = {
/* MSI support starts here. */
static unsigned int tilegx_msi_startup(struct irq_data *d)
{
- if (d->msi_desc)
+ if (irq_data_get_msi(d))
pci_msi_unmask_irq(d);

return 0;
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index c3e7dfcf9ff5..74b48ea0cf26 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1178,7 +1178,7 @@ EXPORT_SYMBOL(pci_enable_msix_range);
*/
void pci_msi_domain_write_msg(struct irq_data *irq_data, struct msi_msg *msg)
{
- struct msi_desc *desc = irq_data->msi_desc;
+ struct msi_desc *desc = irq_data_get_msi(irq_data);

/*
* For MSI-X desc->irq is always equal to irq_data->irq. For
--
1.7.10.4

2015-05-04 03:18:25

by Jiang Liu

[permalink] [raw]
Subject: [RFC v1 10/11] genirq: Move field 'msi_desc' from struct irq_data into struct irq_common_data

MSI descriptors are per-irq instead of per irqchip, so move it into
struct irq_common_data.

Signed-off-by: Jiang Liu <[email protected]>
---
include/linux/irq.h | 8 ++++----
include/linux/irqdesc.h | 2 +-
kernel/irq/chip.c | 2 +-
kernel/irq/irqdesc.c | 2 +-
4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 6ac20b3f3053..e08bf8811cc1 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -132,6 +132,7 @@ struct irq_domain;
* @node: node index useful for balancing
* @handler_data: per-IRQ data for the irq_chip methods
* @affinity: IRQ affinity on SMP
+ * @msi_desc: MSI descriptor
*/
struct irq_common_data {
unsigned int state_use_accessors;
@@ -140,6 +141,7 @@ struct irq_common_data {
#endif
void *handler_data;
cpumask_var_t affinity;
+ struct msi_desc *msi_desc;
};

/**
@@ -155,7 +157,6 @@ struct irq_common_data {
* irq_domain
* @chip_data: platform-specific per-chip private data for the chip
* methods, to allow shared chip implementations
- * @msi_desc: MSI descriptor
*
* The fields here need to overlay the ones in irq_desc until we
* cleaned up the direct references and switched everything over to
@@ -172,7 +173,6 @@ struct irq_data {
struct irq_data *parent_data;
#endif
void *chip_data;
- struct msi_desc *msi_desc;
};

/*
@@ -622,12 +622,12 @@ static inline void *irq_data_get_irq_handler_data(struct irq_data *d)
static inline struct msi_desc *irq_get_msi_desc(unsigned int irq)
{
struct irq_data *d = irq_get_irq_data(irq);
- return d ? d->msi_desc : NULL;
+ return d ? d->common->msi_desc : NULL;
}

static inline struct msi_desc *irq_data_get_msi(struct irq_data *d)
{
- return d->msi_desc;
+ return d->common->msi_desc;
}

static inline u32 irq_get_trigger_type(unsigned int irq)
diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
index 5d5776f40129..81a7231c0379 100644
--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -116,7 +116,7 @@ static inline void *irq_desc_get_handler_data(struct irq_desc *desc)

static inline struct msi_desc *irq_desc_get_msi_desc(struct irq_desc *desc)
{
- return desc->irq_data.msi_desc;
+ return desc->irq_common_data.msi_desc;
}

/*
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 46c487fc71dd..d7a50974b77d 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -105,7 +105,7 @@ int irq_set_msi_desc_off(unsigned int irq_base, unsigned int irq_offset,

if (!desc)
return -EINVAL;
- desc->irq_data.msi_desc = entry;
+ desc->irq_common_data.msi_desc = entry;
if (entry && !irq_offset)
entry->irq = irq_base;
irq_put_desc_unlock(desc, flags);
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 2c39a203f78b..358f620c035a 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -74,12 +74,12 @@ static void desc_set_defaults(unsigned int irq, struct irq_desc *desc, int node,
int cpu;

desc->irq_common_data.handler_data = NULL;
+ desc->irq_common_data.msi_desc = NULL;

desc->irq_data.common = &desc->irq_common_data;
desc->irq_data.irq = irq;
desc->irq_data.chip = &no_irq_chip;
desc->irq_data.chip_data = NULL;
- desc->irq_data.msi_desc = NULL;
irq_settings_clr_and_set(desc, ~0, _IRQ_DEFAULT_INIT_FLAGS);
irqd_set(&desc->irq_data, IRQD_IRQ_DISABLED);
desc->handle_irq = handle_bad_irq;
--
1.7.10.4

2015-05-04 03:18:14

by Jiang Liu

[permalink] [raw]
Subject: [RFC v1 11/11] genirq: Pass irq_data to helper function __irq_set_chip_handler_name_locked()

For most cases, callers pass irq_data->irq to helper function
__irq_set_chip_handler_name_locked() and then it looks up irq_data again
by calling irq_get_irq_data(irq).

So pass irq_data directly instead of irq_data->irq to
__irq_set_chip_handler_name_locked().

Signed-off-by: Jiang Liu <[email protected]>
---
arch/ia64/kernel/iosapic.c | 6 +++---
arch/mips/alchemy/common/irq.c | 4 ++--
drivers/gpio/gpio-zynq.c | 9 ++++-----
drivers/irqchip/irq-metag-ext.c | 5 ++---
drivers/irqchip/irq-mips-gic.c | 11 ++++-------
include/linux/irqdesc.h | 6 +++---
6 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/arch/ia64/kernel/iosapic.c b/arch/ia64/kernel/iosapic.c
index 4d2698d43c39..317993e92cba 100644
--- a/arch/ia64/kernel/iosapic.c
+++ b/arch/ia64/kernel/iosapic.c
@@ -610,9 +610,9 @@ register_intr (unsigned int gsi, int irq, unsigned char delivery,
chip->name, irq_type->name);
chip = irq_type;
}
- __irq_set_chip_handler_name_locked(irq, chip, trigger == IOSAPIC_EDGE ?
- handle_edge_irq : handle_level_irq,
- NULL);
+ __irq_set_chip_handler_name_locked(irq_get_irq_data(irq), chip,
+ trigger == IOSAPIC_EDGE ? handle_edge_irq : handle_level_irq,
+ NULL);
return 0;
}

diff --git a/arch/mips/alchemy/common/irq.c b/arch/mips/alchemy/common/irq.c
index 6cb60abfdcc9..026c4eed37d5 100644
--- a/arch/mips/alchemy/common/irq.c
+++ b/arch/mips/alchemy/common/irq.c
@@ -491,7 +491,7 @@ static int au1x_ic_settype(struct irq_data *d, unsigned int flow_type)
default:
ret = -EINVAL;
}
- __irq_set_chip_handler_name_locked(d->irq, chip, handler, name);
+ __irq_set_chip_handler_name_locked(d, chip, handler, name);

wmb();

@@ -703,7 +703,7 @@ static int au1300_gpic_settype(struct irq_data *d, unsigned int type)
return -EINVAL;
}

- __irq_set_chip_handler_name_locked(d->irq, &au1300_gpic, hdl, name);
+ __irq_set_chip_handler_name_locked(d, &au1300_gpic, hdl, name);

au1300_gpic_chgcfg(d->irq - ALCHEMY_GPIC_INT_BASE, GPIC_CFG_IC_MASK, s);

diff --git a/drivers/gpio/gpio-zynq.c b/drivers/gpio/gpio-zynq.c
index 184c4b1b2558..aea6075e5b2e 100644
--- a/drivers/gpio/gpio-zynq.c
+++ b/drivers/gpio/gpio-zynq.c
@@ -422,13 +422,12 @@ static int zynq_gpio_set_irq_type(struct irq_data *irq_data, unsigned int type)
writel_relaxed(int_any,
gpio->base_addr + ZYNQ_GPIO_INTANY_OFFSET(bank_num));

- if (type & IRQ_TYPE_LEVEL_MASK) {
- __irq_set_chip_handler_name_locked(irq_data->irq,
+ if (type & IRQ_TYPE_LEVEL_MASK)
+ __irq_set_chip_handler_name_locked(irq_data,
&zynq_gpio_level_irqchip, handle_fasteoi_irq, NULL);
- } else {
- __irq_set_chip_handler_name_locked(irq_data->irq,
+ else
+ __irq_set_chip_handler_name_locked(irq_data,
&zynq_gpio_edge_irqchip, handle_level_irq, NULL);
- }

return 0;
}
diff --git a/drivers/irqchip/irq-metag-ext.c b/drivers/irqchip/irq-metag-ext.c
index 2cb474ad8809..52e501d8c8f0 100644
--- a/drivers/irqchip/irq-metag-ext.c
+++ b/drivers/irqchip/irq-metag-ext.c
@@ -404,7 +404,6 @@ static int meta_intc_irq_set_type(struct irq_data *data, unsigned int flow_type)
#ifdef CONFIG_METAG_SUSPEND_MEM
struct meta_intc_priv *priv = &meta_intc_priv;
#endif
- unsigned int irq = data->irq;
irq_hw_number_t hw = data->hwirq;
unsigned int bit = 1 << meta_intc_offset(hw);
void __iomem *level_addr = meta_intc_level_addr(hw);
@@ -413,10 +412,10 @@ static int meta_intc_irq_set_type(struct irq_data *data, unsigned int flow_type)

/* update the chip/handler */
if (flow_type & IRQ_TYPE_LEVEL_MASK)
- __irq_set_chip_handler_name_locked(irq, &meta_intc_level_chip,
+ __irq_set_chip_handler_name_locked(data, &meta_intc_level_chip,
handle_level_irq, NULL);
else
- __irq_set_chip_handler_name_locked(irq, &meta_intc_edge_chip,
+ __irq_set_chip_handler_name_locked(data, &meta_intc_edge_chip,
handle_edge_irq, NULL);

/* and clear/set the bit in HWLEVELEXT */
diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index 09257c301bd2..fb2e64b1f414 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -365,15 +365,12 @@ static int gic_set_type(struct irq_data *d, unsigned int type)
break;
}

- if (is_edge) {
- __irq_set_chip_handler_name_locked(d->irq,
- &gic_edge_irq_controller,
+ if (is_edge)
+ __irq_set_chip_handler_name_locked(d, &gic_edge_irq_controller,
handle_edge_irq, NULL);
- } else {
- __irq_set_chip_handler_name_locked(d->irq,
- &gic_level_irq_controller,
+ else
+ __irq_set_chip_handler_name_locked(d, &gic_level_irq_controller,
handle_level_irq, NULL);
- }
spin_unlock_irqrestore(&gic_lock, flags);

return 0;
diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
index 81a7231c0379..720ff116b533 100644
--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -168,15 +168,15 @@ static inline void __irq_set_handler_locked(unsigned int irq,

/* caller has locked the irq_desc and both params are valid */
static inline void
-__irq_set_chip_handler_name_locked(unsigned int irq, struct irq_chip *chip,
+__irq_set_chip_handler_name_locked(struct irq_data *data, struct irq_chip *chip,
irq_flow_handler_t handler, const char *name)
{
struct irq_desc *desc;

- desc = irq_to_desc(irq);
- irq_desc_get_irq_data(desc)->chip = chip;
+ desc = irq_to_desc(data->irq);
desc->handle_irq = handler;
desc->name = name;
+ data->chip = chip;
}

static inline int irq_balancing_disabled(unsigned int irq)
--
1.7.10.4

2015-05-04 12:10:17

by Amir Vadai

[permalink] [raw]
Subject: Re: [RFC v1 07/11] net/mlx4: Cache irq_desc->affinity instead of irq_desc

On Mon, May 4, 2015 at 6:15 AM, Jiang Liu <[email protected]> wrote:
> The field 'affinity' in irq_desc won't change once the irq_desc data
> structure is created. So cache irq_desc->affinity instead of irq_desc.
> This also helps to hide struct irq_desc from device drivers.

Hi Jiang,

I might not understand the new changes irq core, but up until now
affinity was changed when the user changed it through
/proc/irq/<IRQ>/smp_affinity.
This code is monitoring the affinity from the napi_poll context to
detect affinity changes, and prevent napi from keep running on the
wrong CPU.
Therefore, the affinity can't be cached at the beginning. Please
revert this caching.

Thanks,
Amir

2015-05-04 14:00:57

by Jiang Liu

[permalink] [raw]
Subject: Re: [RFC v1 07/11] net/mlx4: Cache irq_desc->affinity instead of irq_desc

On 2015/5/4 20:10, Amir Vadai wrote:
> On Mon, May 4, 2015 at 6:15 AM, Jiang Liu <[email protected]> wrote:
>> The field 'affinity' in irq_desc won't change once the irq_desc data
>> structure is created. So cache irq_desc->affinity instead of irq_desc.
>> This also helps to hide struct irq_desc from device drivers.
>
> Hi Jiang,
>
> I might not understand the new changes irq core, but up until now
> affinity was changed when the user changed it through
> /proc/irq/<IRQ>/smp_affinity.
> This code is monitoring the affinity from the napi_poll context to
> detect affinity changes, and prevent napi from keep running on the
> wrong CPU.
> Therefore, the affinity can't be cached at the beginning. Please
> revert this caching.
Hi Amir,
Thanks for review:) We want to hide irq_desc implementation
details from device drivers, so made these changes.
Function irq_get_affinity_mask() returns 'struct cpumask *'
and we cache the returned pointer. On the other hand, user may change
IRQ affinity through /proc/irq/<IRQ>/smp_affinity, but that only
changes the bitmap pointed to by the cached pointer and won't change
the pointer itself. So it should always return the latest affinity
setting by calling cpumask_test_cpu(cpu_curr, cq->irq_affinity).
Or am I missing something here?
Thanks!
Gerry
>
> Thanks,
> Amir
>

2015-05-04 15:10:54

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [RFC v1 07/11] net/mlx4: Cache irq_desc->affinity instead of irq_desc

On Mon, 4 May 2015, Amir Vadai wrote:

> On Mon, May 4, 2015 at 6:15 AM, Jiang Liu <[email protected]> wrote:
> > The field 'affinity' in irq_desc won't change once the irq_desc data
> > structure is created. So cache irq_desc->affinity instead of irq_desc.
> > This also helps to hide struct irq_desc from device drivers.
>
> Hi Jiang,
>
> I might not understand the new changes irq core, but up until now
> affinity was changed when the user changed it through
> /proc/irq/<IRQ>/smp_affinity.
> This code is monitoring the affinity from the napi_poll context to
> detect affinity changes, and prevent napi from keep running on the
> wrong CPU.
> Therefore, the affinity can't be cached at the beginning. Please
> revert this caching.

Sigh. All of this is completely wrong. Polling in the internals of
irq_desc is just crap.

irq_set_affinity_notifier() has been added for exactly this purpose.

Please get rid of your irq_desc abuse.

Thanks,

tglx

2015-05-05 09:07:21

by Amir Vadai

[permalink] [raw]
Subject: Re: [RFC v1 07/11] net/mlx4: Cache irq_desc->affinity instead of irq_desc

On Mon, May 4, 2015 at 5:00 PM, Jiang Liu <[email protected]> wrote:
> On 2015/5/4 20:10, Amir Vadai wrote:
>> On Mon, May 4, 2015 at 6:15 AM, Jiang Liu <[email protected]> wrote:
>>> The field 'affinity' in irq_desc won't change once the irq_desc data
>>> structure is created. So cache irq_desc->affinity instead of irq_desc.
>>> This also helps to hide struct irq_desc from device drivers.
>>
>> Hi Jiang,
>>
>> I might not understand the new changes irq core, but up until now
>> affinity was changed when the user changed it through
>> /proc/irq/<IRQ>/smp_affinity.
>> This code is monitoring the affinity from the napi_poll context to
>> detect affinity changes, and prevent napi from keep running on the
>> wrong CPU.
>> Therefore, the affinity can't be cached at the beginning. Please
>> revert this caching.
> Hi Amir,
> Thanks for review:) We want to hide irq_desc implementation
> details from device drivers, so made these changes.
> Function irq_get_affinity_mask() returns 'struct cpumask *'
> and we cache the returned pointer. On the other hand, user may change
> IRQ affinity through /proc/irq/<IRQ>/smp_affinity, but that only
> changes the bitmap pointed to by the cached pointer and won't change
> the pointer itself. So it should always return the latest affinity
> setting by calling cpumask_test_cpu(cpu_curr, cq->irq_affinity).
> Or am I missing something here?
No - you are completely right - I missed it.

Anyway I tested it to make sure, and everything works as expected.

Acked-By: Amir Vadai <[email protected]>

Thanks,
Amir


> Thanks!
> Gerry
>>
>> Thanks,
>> Amir
>>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2015-05-05 09:17:42

by Amir Vadai

[permalink] [raw]
Subject: Re: [RFC v1 07/11] net/mlx4: Cache irq_desc->affinity instead of irq_desc

On Mon, May 4, 2015 at 6:10 PM, Thomas Gleixner <[email protected]> wrote:
> On Mon, 4 May 2015, Amir Vadai wrote:
>
>> On Mon, May 4, 2015 at 6:15 AM, Jiang Liu <[email protected]> wrote:
>> > The field 'affinity' in irq_desc won't change once the irq_desc data
>> > structure is created. So cache irq_desc->affinity instead of irq_desc.
>> > This also helps to hide struct irq_desc from device drivers.
>>
>> Hi Jiang,
>>
>> I might not understand the new changes irq core, but up until now
>> affinity was changed when the user changed it through
>> /proc/irq/<IRQ>/smp_affinity.
>> This code is monitoring the affinity from the napi_poll context to
>> detect affinity changes, and prevent napi from keep running on the
>> wrong CPU.
>> Therefore, the affinity can't be cached at the beginning. Please
>> revert this caching.
>
> Sigh. All of this is completely wrong. Polling in the internals of
> irq_desc is just crap.
Hi,

>
> irq_set_affinity_notifier() has been added for exactly this purpose.
rmap is already registered on irq_set_affinity_notifier().
Actually, I did post an extension to the affinity notifier, to use
affinity chain which you rejected [1] (and BTW, convinced me that this
is not the way to do it).
Current code is according to your suggestion.

[1] - https://lkml.org/lkml/2014/5/26/222

Amir

>
> Please get rid of your irq_desc abuse.
>
> Thanks,
>
> tglx
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2015-05-05 16:08:16

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [RFC v1 07/11] net/mlx4: Cache irq_desc->affinity instead of irq_desc

On Tue, 5 May 2015, Amir Vadai wrote:

> On Mon, May 4, 2015 at 6:10 PM, Thomas Gleixner <[email protected]> wrote:
> > On Mon, 4 May 2015, Amir Vadai wrote:
> >
> >> On Mon, May 4, 2015 at 6:15 AM, Jiang Liu <[email protected]> wrote:
> >> > The field 'affinity' in irq_desc won't change once the irq_desc data
> >> > structure is created. So cache irq_desc->affinity instead of irq_desc.
> >> > This also helps to hide struct irq_desc from device drivers.
> >>
> >> Hi Jiang,
> >>
> >> I might not understand the new changes irq core, but up until now
> >> affinity was changed when the user changed it through
> >> /proc/irq/<IRQ>/smp_affinity.
> >> This code is monitoring the affinity from the napi_poll context to
> >> detect affinity changes, and prevent napi from keep running on the
> >> wrong CPU.
> >> Therefore, the affinity can't be cached at the beginning. Please
> >> revert this caching.
> >
> > Sigh. All of this is completely wrong. Polling in the internals of
> > irq_desc is just crap.
> Hi,
>
> >
> > irq_set_affinity_notifier() has been added for exactly this purpose.
> rmap is already registered on irq_set_affinity_notifier().
> Actually, I did post an extension to the affinity notifier, to use
> affinity chain which you rejected [1] (and BTW, convinced me that this
> is not the way to do it).
> Current code is according to your suggestion.
>
> [1] - https://lkml.org/lkml/2014/5/26/222

No, it's not. I wrote:

"So what's the point of adding notifier call chain complexity,
ordering problems etc., if you can simply note the fact that the
affinity changed in the rmap itself and check that in the RX path?"

I did not tell you to fiddle in irqdesc, really.

Thanks,

tglx

2015-05-07 10:42:03

by Amir Vadai

[permalink] [raw]
Subject: Re: [RFC v1 07/11] net/mlx4: Cache irq_desc->affinity instead of irq_desc

On 5/5/2015 5:53 PM, Thomas Gleixner wrote:
> On Tue, 5 May 2015, Amir Vadai wrote:
>
>> On Mon, May 4, 2015 at 6:10 PM, Thomas Gleixner <[email protected]> wrote:
>>> On Mon, 4 May 2015, Amir Vadai wrote:
>>>
>>>> On Mon, May 4, 2015 at 6:15 AM, Jiang Liu <[email protected]> wrote:
>>>>> The field 'affinity' in irq_desc won't change once the irq_desc data
>>>>> structure is created. So cache irq_desc->affinity instead of irq_desc.
>>>>> This also helps to hide struct irq_desc from device drivers.
>>>>
>>>> Hi Jiang,
>>>>
>>>> I might not understand the new changes irq core, but up until now
>>>> affinity was changed when the user changed it through
>>>> /proc/irq/<IRQ>/smp_affinity.
>>>> This code is monitoring the affinity from the napi_poll context to
>>>> detect affinity changes, and prevent napi from keep running on the
>>>> wrong CPU.
>>>> Therefore, the affinity can't be cached at the beginning. Please
>>>> revert this caching.
>>>
>>> Sigh. All of this is completely wrong. Polling in the internals of
>>> irq_desc is just crap.
>> Hi,
>>
>>>
>>> irq_set_affinity_notifier() has been added for exactly this purpose.
>> rmap is already registered on irq_set_affinity_notifier().
>> Actually, I did post an extension to the affinity notifier, to use
>> affinity chain which you rejected [1] (and BTW, convinced me that this
>> is not the way to do it).
>> Current code is according to your suggestion.
>>
>> [1] - https://lkml.org/lkml/2014/5/26/222
>
> No, it's not. I wrote:
>
> "So what's the point of adding notifier call chain complexity,
> ordering problems etc., if you can simply note the fact that the
> affinity changed in the rmap itself and check that in the RX path?"
>
> I did not tell you to fiddle in irqdesc, really.
>
> Thanks,
>
> tglx
>

It is problematic, since I didn't want to link the napi polling affinity
issue and CONFIG_CPU_RMAP being selected. That's why I CC'd you when
submitted the patch [1]. Guess it was my bad that I didn't do it in a
better way that would give you a chance to fully review it.

Anyway, I understand now that I shouldn't access irqdesc attributes from
a device driver.
I will see if I can live with disabling this whole feature (migrate a
currently looping NAPI when affinity is changed), when CONFIG_CPU_RMAP
is not selected. If not, I might modify the rmap code to give me this
information even when it is not selected. I assume that rmap notifier
callback can access this information.

Thanks,
Amir

2015-05-08 02:35:21

by Abel Wu

[permalink] [raw]
Subject: Re: [RFC v1 01/11] genirq: Introduce struct irq_common_data to host shared irq data

On 2015/5/4 11:15, Jiang Liu wrote:

[...]
> diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
> index dd1109fb241e..3010e99abf3e 100644
> --- a/include/linux/irqdesc.h
> +++ b/include/linux/irqdesc.h
> @@ -47,6 +47,7 @@ struct pt_regs;
> * @name: flow handler name for /proc/interrupts output
> */
> struct irq_desc {
> + struct irq_common_data irq_common_data;

Hi Gerry,

Please update description as well. :)

Thanks,
Abel

> struct irq_data irq_data;
> unsigned int __percpu *kstat_irqs;
> irq_flow_handler_t handle_irq;
> diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
> index df553b0af936..ed84299788b3 100644
> --- a/kernel/irq/internals.h
> +++ b/kernel/irq/internals.h
> @@ -170,27 +170,27 @@ irq_put_desc_unlock(struct irq_desc *desc, unsigned long flags)
> */
> static inline void irqd_set_move_pending(struct irq_data *d)
> {
> - d->state_use_accessors |= IRQD_SETAFFINITY_PENDING;
> + __irqd_to_state(d) |= IRQD_SETAFFINITY_PENDING;
> }
>
> static inline void irqd_clr_move_pending(struct irq_data *d)
> {
> - d->state_use_accessors &= ~IRQD_SETAFFINITY_PENDING;
> + __irqd_to_state(d) &= ~IRQD_SETAFFINITY_PENDING;
> }
>
> static inline void irqd_clear(struct irq_data *d, unsigned int mask)
> {
> - d->state_use_accessors &= ~mask;
> + __irqd_to_state(d) &= ~mask;
> }
>
> static inline void irqd_set(struct irq_data *d, unsigned int mask)
> {
> - d->state_use_accessors |= mask;
> + __irqd_to_state(d) |= mask;
> }
>
> static inline bool irqd_has_set(struct irq_data *d, unsigned int mask)
> {
> - return d->state_use_accessors & mask;
> + return __irqd_to_state(d) & mask;
> }
>
> static inline void kstat_incr_irqs_this_cpu(unsigned int irq, struct irq_desc *desc)
> diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
> index 99793b9b6d23..eac1aac906ea 100644
> --- a/kernel/irq/irqdesc.c
> +++ b/kernel/irq/irqdesc.c
> @@ -76,6 +76,7 @@ static void desc_set_defaults(unsigned int irq, struct irq_desc *desc, int node,
> {
> int cpu;
>
> + desc->irq_data.common = &desc->irq_common_data;
> desc->irq_data.irq = irq;
> desc->irq_data.chip = &no_irq_chip;
> desc->irq_data.chip_data = NULL;
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 7fac311057b8..3552b8750efd 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -834,6 +834,7 @@ static struct irq_data *irq_domain_insert_irq_data(struct irq_domain *domain,
> if (irq_data) {
> child->parent_data = irq_data;
> irq_data->irq = child->irq;
> + irq_data->common = child->common;
> irq_data->node = child->node;
> irq_data->domain = domain;
> }


2015-05-08 02:34:19

by Abel Wu

[permalink] [raw]
Subject: Re: [RFC v1 02/11] genirq: Move field 'node' from struct irq_data into struct irq_common_data

Hi Gerry,
On 2015/5/4 11:15, Jiang Liu wrote:

> NUMA node information is per-irq instead of per-irqchip, so move it into
> struct irq_common_data.
>
> Signed-off-by: Jiang Liu <[email protected]>
> ---
> arch/sh/kernel/irq.c | 2 +-
> arch/x86/kernel/apic/vector.c | 8 ++++----
> arch/x86/platform/uv/uv_irq.c | 2 +-
> include/linux/irq.h | 20 ++++++++++++++++++--
> kernel/irq/internals.h | 5 +++++
> kernel/irq/irqdesc.c | 10 ++--------
> kernel/irq/irqdomain.c | 4 ++--
> kernel/irq/manage.c | 2 +-
> kernel/irq/proc.c | 2 +-
> 9 files changed, 35 insertions(+), 20 deletions(-)
>
> diff --git a/arch/sh/kernel/irq.c b/arch/sh/kernel/irq.c
> index eb10ff84015c..8dc677cc136b 100644
> --- a/arch/sh/kernel/irq.c
> +++ b/arch/sh/kernel/irq.c
> @@ -227,7 +227,7 @@ void migrate_irqs(void)
> for_each_active_irq(irq) {
> struct irq_data *data = irq_get_irq_data(irq);
>
> - if (data->node == cpu) {
> + if (irq_data_get_node(data) == cpu) {
> unsigned int newcpu = cpumask_any_and(data->affinity,
> cpu_online_mask);
> if (newcpu >= nr_cpu_ids) {
> diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
> index 96ce5068a926..983bea2a09ce 100644
> --- a/arch/x86/kernel/apic/vector.c
> +++ b/arch/x86/kernel/apic/vector.c
> @@ -345,7 +345,7 @@ static int x86_vector_alloc_irqs(struct irq_domain *domain, unsigned int virq,
> struct irq_alloc_info *info = arg;
> struct apic_chip_data *data;
> struct irq_data *irq_data;
> - int i, err;
> + int i, err, node;
>
> if (disable_apic)
> return -ENXIO;
> @@ -357,12 +357,13 @@ static int x86_vector_alloc_irqs(struct irq_domain *domain, unsigned int virq,
> for (i = 0; i < nr_irqs; i++) {
> irq_data = irq_domain_get_irq_data(domain, virq + i);
> BUG_ON(!irq_data);
> + node = irq_data_get_node(irq_data);
> #ifdef CONFIG_X86_IO_APIC
> if (virq + i < nr_legacy_irqs() && legacy_irq_data[virq + i])
> data = legacy_irq_data[virq + i];
> else
> #endif
> - data = alloc_apic_chip_data(irq_data->node);
> + data = alloc_apic_chip_data(node);
> if (!data) {
> err = -ENOMEM;
> goto error;
> @@ -371,8 +372,7 @@ static int x86_vector_alloc_irqs(struct irq_domain *domain, unsigned int virq,
> irq_data->chip = &lapic_controller;
> irq_data->chip_data = data;
> irq_data->hwirq = virq + i;
> - err = assign_irq_vector_policy(virq, irq_data->node, data,
> - info);
> + err = assign_irq_vector_policy(virq, node, data, info);
> if (err)
> goto error;
> }
> diff --git a/arch/x86/platform/uv/uv_irq.c b/arch/x86/platform/uv/uv_irq.c
> index cdf86cd3fd97..bc992b7b041f 100644
> --- a/arch/x86/platform/uv/uv_irq.c
> +++ b/arch/x86/platform/uv/uv_irq.c
> @@ -89,7 +89,7 @@ static int uv_domain_alloc(struct irq_domain *domain, unsigned int virq,
> return -EINVAL;
>
> chip_data = kmalloc_node(sizeof(*chip_data), GFP_KERNEL,
> - irq_data->node);
> + irq_data_get_node(irq_data));
> if (!chip_data)
> return -ENOMEM;
>
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 3b6e0def7f5c..3f999a0af713 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -129,9 +129,13 @@ struct irq_domain;
> * struct irq_common_data - per irq data shared by all irqchips
> * @state_use_accessors: status information for irq chip functions.
> * Use accessor functions to deal with it
> + * @node: node index useful for balancing
> */
> struct irq_common_data {
> unsigned int state_use_accessors;
> +#ifdef CONFIG_SMP

Would CONFIG_NUMA be a little more appropriate?
Or even let @node be always compiled?

Thanks,
Abel

> + unsigned int node;
> +#endif
> };
>
> /**
> @@ -139,7 +143,6 @@ struct irq_common_data {
> * @mask: precomputed bitmask for accessing the chip registers
> * @irq: interrupt number
> * @hwirq: hardware interrupt number, local to the interrupt domain
> - * @node: node index useful for balancing
> * @common: point to data shared by all irqchips
> * @chip: low level interrupt hardware access
> * @domain: Interrupt translation domain; responsible for mapping
> @@ -160,7 +163,6 @@ struct irq_data {
> u32 mask;
> unsigned int irq;
> unsigned long hwirq;
> - unsigned int node;
> struct irq_common_data *common;
> struct irq_chip *chip;
> struct irq_domain *domain;
> @@ -634,6 +636,20 @@ static inline u32 irq_get_trigger_type(unsigned int irq)
> return d ? irqd_get_trigger_type(d) : 0;
> }
>
> +static inline int irq_common_data_get_node(struct irq_common_data *d)
> +{
> +#ifdef CONFIG_SMP
> + return d->node;
> +#else
> + return 0;
> +#endif
> +}
> +
> +static inline int irq_data_get_node(struct irq_data *d)
> +{
> + return irq_common_data_get_node(d->common);
> +}
> +
> unsigned int arch_dynirq_lower_bound(unsigned int from);
>
> int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
> diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
> index ed84299788b3..d82a77d39aeb 100644
> --- a/kernel/irq/internals.h
> +++ b/kernel/irq/internals.h
> @@ -199,6 +199,11 @@ static inline void kstat_incr_irqs_this_cpu(unsigned int irq, struct irq_desc *d
> __this_cpu_inc(kstat.irqs_sum);
> }
>
> +static inline int irq_desc_get_node(struct irq_desc *desc)
> +{
> + return irq_common_data_get_node(&desc->irq_common_data);
> +}
> +
> #ifdef CONFIG_PM_SLEEP
> bool irq_pm_check_wakeup(struct irq_desc *desc);
> void irq_pm_install_action(struct irq_desc *desc, struct irqaction *action);
> diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
> index eac1aac906ea..20773f073e24 100644
> --- a/kernel/irq/irqdesc.c
> +++ b/kernel/irq/irqdesc.c
> @@ -52,23 +52,17 @@ static int alloc_masks(struct irq_desc *desc, gfp_t gfp, int node)
>
> static void desc_smp_init(struct irq_desc *desc, int node)
> {
> - desc->irq_data.node = node;
> + desc->irq_common_data.node = node;
> cpumask_copy(desc->irq_data.affinity, irq_default_affinity);
> #ifdef CONFIG_GENERIC_PENDING_IRQ
> cpumask_clear(desc->pending_mask);
> #endif
> }
>
> -static inline int desc_node(struct irq_desc *desc)
> -{
> - return desc->irq_data.node;
> -}
> -
> #else
> static inline int
> alloc_masks(struct irq_desc *desc, gfp_t gfp, int node) { return 0; }
> static inline void desc_smp_init(struct irq_desc *desc, int node) { }
> -static inline int desc_node(struct irq_desc *desc) { return 0; }
> #endif
>
> static void desc_set_defaults(unsigned int irq, struct irq_desc *desc, int node,
> @@ -300,7 +294,7 @@ static void free_desc(unsigned int irq)
> unsigned long flags;
>
> raw_spin_lock_irqsave(&desc->lock, flags);
> - desc_set_defaults(irq, desc, desc_node(desc), NULL);
> + desc_set_defaults(irq, desc, irq_desc_get_node(desc), NULL);
> raw_spin_unlock_irqrestore(&desc->lock, flags);
> }
>
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 3552b8750efd..93ef89059022 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -830,12 +830,12 @@ static struct irq_data *irq_domain_insert_irq_data(struct irq_domain *domain,
> {
> struct irq_data *irq_data;
>
> - irq_data = kzalloc_node(sizeof(*irq_data), GFP_KERNEL, child->node);
> + irq_data = kzalloc_node(sizeof(*irq_data), GFP_KERNEL,
> + irq_data_get_node(child));
> if (irq_data) {
> child->parent_data = irq_data;
> irq_data->irq = child->irq;
> irq_data->common = child->common;
> - irq_data->node = child->node;
> irq_data->domain = domain;
> }
>
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index e68932bb308e..d206aa2c3378 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -332,7 +332,7 @@ static int
> setup_affinity(unsigned int irq, struct irq_desc *desc, struct cpumask *mask)
> {
> struct cpumask *set = irq_default_affinity;
> - int node = desc->irq_data.node;
> + int node = irq_desc_get_node(desc);
>
> /* Excludes PER_CPU and NO_BALANCE interrupts */
> if (!irq_can_set_affinity(irq))
> diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c
> index df2f4642d1e7..0e97c142ce40 100644
> --- a/kernel/irq/proc.c
> +++ b/kernel/irq/proc.c
> @@ -241,7 +241,7 @@ static int irq_node_proc_show(struct seq_file *m, void *v)
> {
> struct irq_desc *desc = irq_to_desc((long) m->private);
>
> - seq_printf(m, "%d\n", desc->irq_data.node);
> + seq_printf(m, "%d\n", irq_desc_get_node(desc));
> return 0;
> }
>


2015-05-08 03:05:49

by Abel Wu

[permalink] [raw]
Subject: Re: [RFC v1 02/11] genirq: Move field 'node' from struct irq_data into struct irq_common_data

On 2015/5/8 10:29, Yun Wu (Abel) wrote:

> Hi Gerry,
> On 2015/5/4 11:15, Jiang Liu wrote:
>
>> NUMA node information is per-irq instead of per-irqchip, so move it into
>> struct irq_common_data.
>>
>> Signed-off-by: Jiang Liu <[email protected]>
>> ---
>> arch/sh/kernel/irq.c | 2 +-
>> arch/x86/kernel/apic/vector.c | 8 ++++----
>> arch/x86/platform/uv/uv_irq.c | 2 +-
>> include/linux/irq.h | 20 ++++++++++++++++++--
>> kernel/irq/internals.h | 5 +++++
>> kernel/irq/irqdesc.c | 10 ++--------
>> kernel/irq/irqdomain.c | 4 ++--
>> kernel/irq/manage.c | 2 +-
>> kernel/irq/proc.c | 2 +-
>> 9 files changed, 35 insertions(+), 20 deletions(-)
>>
>> diff --git a/arch/sh/kernel/irq.c b/arch/sh/kernel/irq.c
>> index eb10ff84015c..8dc677cc136b 100644
>> --- a/arch/sh/kernel/irq.c
>> +++ b/arch/sh/kernel/irq.c
>> @@ -227,7 +227,7 @@ void migrate_irqs(void)
>> for_each_active_irq(irq) {
>> struct irq_data *data = irq_get_irq_data(irq);
>>
>> - if (data->node == cpu) {
>> + if (irq_data_get_node(data) == cpu) {
>> unsigned int newcpu = cpumask_any_and(data->affinity,
>> cpu_online_mask);
>> if (newcpu >= nr_cpu_ids) {
>> diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
>> index 96ce5068a926..983bea2a09ce 100644
>> --- a/arch/x86/kernel/apic/vector.c
>> +++ b/arch/x86/kernel/apic/vector.c
>> @@ -345,7 +345,7 @@ static int x86_vector_alloc_irqs(struct irq_domain *domain, unsigned int virq,
>> struct irq_alloc_info *info = arg;
>> struct apic_chip_data *data;
>> struct irq_data *irq_data;
>> - int i, err;
>> + int i, err, node;
>>
>> if (disable_apic)
>> return -ENXIO;
>> @@ -357,12 +357,13 @@ static int x86_vector_alloc_irqs(struct irq_domain *domain, unsigned int virq,
>> for (i = 0; i < nr_irqs; i++) {
>> irq_data = irq_domain_get_irq_data(domain, virq + i);
>> BUG_ON(!irq_data);
>> + node = irq_data_get_node(irq_data);
>> #ifdef CONFIG_X86_IO_APIC
>> if (virq + i < nr_legacy_irqs() && legacy_irq_data[virq + i])
>> data = legacy_irq_data[virq + i];
>> else
>> #endif
>> - data = alloc_apic_chip_data(irq_data->node);
>> + data = alloc_apic_chip_data(node);
>> if (!data) {
>> err = -ENOMEM;
>> goto error;
>> @@ -371,8 +372,7 @@ static int x86_vector_alloc_irqs(struct irq_domain *domain, unsigned int virq,
>> irq_data->chip = &lapic_controller;
>> irq_data->chip_data = data;
>> irq_data->hwirq = virq + i;
>> - err = assign_irq_vector_policy(virq, irq_data->node, data,
>> - info);
>> + err = assign_irq_vector_policy(virq, node, data, info);
>> if (err)
>> goto error;
>> }
>> diff --git a/arch/x86/platform/uv/uv_irq.c b/arch/x86/platform/uv/uv_irq.c
>> index cdf86cd3fd97..bc992b7b041f 100644
>> --- a/arch/x86/platform/uv/uv_irq.c
>> +++ b/arch/x86/platform/uv/uv_irq.c
>> @@ -89,7 +89,7 @@ static int uv_domain_alloc(struct irq_domain *domain, unsigned int virq,
>> return -EINVAL;
>>
>> chip_data = kmalloc_node(sizeof(*chip_data), GFP_KERNEL,
>> - irq_data->node);
>> + irq_data_get_node(irq_data));
>> if (!chip_data)
>> return -ENOMEM;
>>
>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>> index 3b6e0def7f5c..3f999a0af713 100644
>> --- a/include/linux/irq.h
>> +++ b/include/linux/irq.h
>> @@ -129,9 +129,13 @@ struct irq_domain;
>> * struct irq_common_data - per irq data shared by all irqchips
>> * @state_use_accessors: status information for irq chip functions.
>> * Use accessor functions to deal with it
>> + * @node: node index useful for balancing
>> */
>> struct irq_common_data {
>> unsigned int state_use_accessors;
>> +#ifdef CONFIG_SMP
>
> Would CONFIG_NUMA be a little more appropriate?
> Or even let @node be always compiled?

Sorry for comment before reading your next patch in which you replaced
CONFIG_SMP with CONFIG_NUMA.
And letting @node be under CONFIG_NUMA makes sense, because it's useless
in non-NUMA scenario.

Thanks,
Abel

>
>
>> + unsigned int node;
>> +#endif
>> };
>>
>> /**
>> @@ -139,7 +143,6 @@ struct irq_common_data {
>> * @mask: precomputed bitmask for accessing the chip registers
>> * @irq: interrupt number
>> * @hwirq: hardware interrupt number, local to the interrupt domain
>> - * @node: node index useful for balancing
>> * @common: point to data shared by all irqchips
>> * @chip: low level interrupt hardware access
>> * @domain: Interrupt translation domain; responsible for mapping
>> @@ -160,7 +163,6 @@ struct irq_data {
>> u32 mask;
>> unsigned int irq;
>> unsigned long hwirq;
>> - unsigned int node;
>> struct irq_common_data *common;
>> struct irq_chip *chip;
>> struct irq_domain *domain;
>> @@ -634,6 +636,20 @@ static inline u32 irq_get_trigger_type(unsigned int irq)
>> return d ? irqd_get_trigger_type(d) : 0;
>> }
>>
>> +static inline int irq_common_data_get_node(struct irq_common_data *d)
>> +{
>> +#ifdef CONFIG_SMP
>> + return d->node;
>> +#else
>> + return 0;
>> +#endif
>> +}
>> +
>> +static inline int irq_data_get_node(struct irq_data *d)
>> +{
>> + return irq_common_data_get_node(d->common);
>> +}
>> +
>> unsigned int arch_dynirq_lower_bound(unsigned int from);
>>
>> int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
>> diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
>> index ed84299788b3..d82a77d39aeb 100644
>> --- a/kernel/irq/internals.h
>> +++ b/kernel/irq/internals.h
>> @@ -199,6 +199,11 @@ static inline void kstat_incr_irqs_this_cpu(unsigned int irq, struct irq_desc *d
>> __this_cpu_inc(kstat.irqs_sum);
>> }
>>
>> +static inline int irq_desc_get_node(struct irq_desc *desc)
>> +{
>> + return irq_common_data_get_node(&desc->irq_common_data);
>> +}
>> +
>> #ifdef CONFIG_PM_SLEEP
>> bool irq_pm_check_wakeup(struct irq_desc *desc);
>> void irq_pm_install_action(struct irq_desc *desc, struct irqaction *action);
>> diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
>> index eac1aac906ea..20773f073e24 100644
>> --- a/kernel/irq/irqdesc.c
>> +++ b/kernel/irq/irqdesc.c
>> @@ -52,23 +52,17 @@ static int alloc_masks(struct irq_desc *desc, gfp_t gfp, int node)
>>
>> static void desc_smp_init(struct irq_desc *desc, int node)
>> {
>> - desc->irq_data.node = node;
>> + desc->irq_common_data.node = node;
>> cpumask_copy(desc->irq_data.affinity, irq_default_affinity);
>> #ifdef CONFIG_GENERIC_PENDING_IRQ
>> cpumask_clear(desc->pending_mask);
>> #endif
>> }
>>
>> -static inline int desc_node(struct irq_desc *desc)
>> -{
>> - return desc->irq_data.node;
>> -}
>> -
>> #else
>> static inline int
>> alloc_masks(struct irq_desc *desc, gfp_t gfp, int node) { return 0; }
>> static inline void desc_smp_init(struct irq_desc *desc, int node) { }
>> -static inline int desc_node(struct irq_desc *desc) { return 0; }
>> #endif
>>
>> static void desc_set_defaults(unsigned int irq, struct irq_desc *desc, int node,
>> @@ -300,7 +294,7 @@ static void free_desc(unsigned int irq)
>> unsigned long flags;
>>
>> raw_spin_lock_irqsave(&desc->lock, flags);
>> - desc_set_defaults(irq, desc, desc_node(desc), NULL);
>> + desc_set_defaults(irq, desc, irq_desc_get_node(desc), NULL);
>> raw_spin_unlock_irqrestore(&desc->lock, flags);
>> }
>>
>> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
>> index 3552b8750efd..93ef89059022 100644
>> --- a/kernel/irq/irqdomain.c
>> +++ b/kernel/irq/irqdomain.c
>> @@ -830,12 +830,12 @@ static struct irq_data *irq_domain_insert_irq_data(struct irq_domain *domain,
>> {
>> struct irq_data *irq_data;
>>
>> - irq_data = kzalloc_node(sizeof(*irq_data), GFP_KERNEL, child->node);
>> + irq_data = kzalloc_node(sizeof(*irq_data), GFP_KERNEL,
>> + irq_data_get_node(child));
>> if (irq_data) {
>> child->parent_data = irq_data;
>> irq_data->irq = child->irq;
>> irq_data->common = child->common;
>> - irq_data->node = child->node;
>> irq_data->domain = domain;
>> }
>>
>> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
>> index e68932bb308e..d206aa2c3378 100644
>> --- a/kernel/irq/manage.c
>> +++ b/kernel/irq/manage.c
>> @@ -332,7 +332,7 @@ static int
>> setup_affinity(unsigned int irq, struct irq_desc *desc, struct cpumask *mask)
>> {
>> struct cpumask *set = irq_default_affinity;
>> - int node = desc->irq_data.node;
>> + int node = irq_desc_get_node(desc);
>>
>> /* Excludes PER_CPU and NO_BALANCE interrupts */
>> if (!irq_can_set_affinity(irq))
>> diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c
>> index df2f4642d1e7..0e97c142ce40 100644
>> --- a/kernel/irq/proc.c
>> +++ b/kernel/irq/proc.c
>> @@ -241,7 +241,7 @@ static int irq_node_proc_show(struct seq_file *m, void *v)
>> {
>> struct irq_desc *desc = irq_to_desc((long) m->private);
>>
>> - seq_printf(m, "%d\n", desc->irq_data.node);
>> + seq_printf(m, "%d\n", irq_desc_get_node(desc));
>> return 0;
>> }
>>
>
>


2015-05-15 20:43:10

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [RFC v1 02/11] genirq: Move field 'node' from struct irq_data into struct irq_common_data

On Mon, 4 May 2015, Jiang Liu wrote:

> NUMA node information is per-irq instead of per-irqchip, so move it into
> struct irq_common_data.
>
> Signed-off-by: Jiang Liu <[email protected]>
> ---
> arch/sh/kernel/irq.c | 2 +-
> arch/x86/kernel/apic/vector.c | 8 ++++----
> arch/x86/platform/uv/uv_irq.c | 2 +-
> include/linux/irq.h | 20 ++++++++++++++++++--
> kernel/irq/internals.h | 5 +++++
> kernel/irq/irqdesc.c | 10 ++--------
> kernel/irq/irqdomain.c | 4 ++--
> kernel/irq/manage.c | 2 +-
> kernel/irq/proc.c | 2 +-

Can you please split this into several patches for review and
bisection sake?

1) Do the cleanup of s/data->node/irq_data_get_node(data)/

1 a) core code
1 b) arch/sh
1 c) arch/x86

2) Do the actual move of node to common_data

Thanks,

tglx

2015-05-15 20:44:37

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [RFC v1 03/11] genirq: Use CONFIG_NUMA instead of CONFIG_SMP to guard irq_common_data.node

On Mon, 4 May 2015, Jiang Liu wrote:

> NUMA is enabled by CONFIG_NUMA instead of CONFIG_SMP, so use CONFIG_NUMA
> to guard irq_common_data.node.

Please move this change to the front and do it on irq_data. That would
have avoided confusing Abel :)

Thanks,

tglx

2015-05-15 20:49:05

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [RFC v1 11/11] genirq: Pass irq_data to helper function __irq_set_chip_handler_name_locked()

On Mon, 4 May 2015, Jiang Liu wrote:
> /* caller has locked the irq_desc and both params are valid */
> static inline void
> -__irq_set_chip_handler_name_locked(unsigned int irq, struct irq_chip *chip,
> +__irq_set_chip_handler_name_locked(struct irq_data *data, struct irq_chip *chip,
> irq_flow_handler_t handler, const char *name)
> {
> struct irq_desc *desc;
>
> - desc = irq_to_desc(irq);
> - irq_desc_get_irq_data(desc)->chip = chip;
> + desc = irq_to_desc(data->irq);

We should have a irq_data_to_desc() helper and use that instead of
going through a full lookup again.

Thanks,

tglx

2015-05-15 20:58:04

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [RFC v1 00/11] Split struct irq_data into common part and per-chip part

On Mon, 4 May 2015, Jiang Liu wrote:
> Hi all,
> Now the irq core supports hierarchy irq and stacked irqchips,
> so there may be multiple irq_datas associated with the same irq. But
> some fields in struct irq_data are per-irq instance and duplicating
> those fields into multiple irq_data may cause troubles.
> So this patch introduces a new data structure 'struct
> irq_common_data' to host per-irq instance data fields, and struct
> irq_data will only host per-chip data fields after the conversion.

I really like that change in general. Please address the few review
comments and resubmit.

Thanks,

tglx

2015-05-18 02:58:34

by Jiang Liu

[permalink] [raw]
Subject: Re: [RFC v1 01/11] genirq: Introduce struct irq_common_data to host shared irq data

On 2015/5/8 10:23, Yun Wu (Abel) wrote:
> On 2015/5/4 11:15, Jiang Liu wrote:
>
> [...]
>> diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
>> index dd1109fb241e..3010e99abf3e 100644
>> --- a/include/linux/irqdesc.h
>> +++ b/include/linux/irqdesc.h
>> @@ -47,6 +47,7 @@ struct pt_regs;
>> * @name: flow handler name for /proc/interrupts output
>> */
>> struct irq_desc {
>> + struct irq_common_data irq_common_data;
>
> Hi Gerry,
>
> Please update description as well. :)
Thanks, Abel!

2015-05-18 05:18:13

by Jiang Liu

[permalink] [raw]
Subject: Re: [RFC v1 03/11] genirq: Use CONFIG_NUMA instead of CONFIG_SMP to guard irq_common_data.node

On 2015/5/16 4:44, Thomas Gleixner wrote:
> On Mon, 4 May 2015, Jiang Liu wrote:
>
>> NUMA is enabled by CONFIG_NUMA instead of CONFIG_SMP, so use CONFIG_NUMA
>> to guard irq_common_data.node.
>
> Please move this change to the front and do it on irq_data. That would
> have avoided confusing Abel :)
Hi Thomas,
Reordering these two patches will cause too much unnecessary code
changes. so I will merge this patch into the patch to move node from
struct irq_data into struct irq_common_data, which is more
straight forward.
Thanks!
Gerry

>
> Thanks,
>
> tglx
>