apic_ack_edge() is explicitely for handling interrupt affinity cleanup when
interrupt remapping is not available or disable.
Remapped interrupts and also some of the platform specific special
interrupts, e.g. UV, invoke ack_APIC_irq() directly.
To address the issue of failing an affinity update with -EBUSY the delayed
affinity mechanism can be reused, but ack_APIC_irq() does not handle
that. Adding this to ack_APIC_irq() is not possible, because that function
is also used for exceptions and directly handled interrupts like IPIs.
Create a new function, which just contains the conditional invocation of
irq_move_irq() and the final ack_APIC_irq(). Making the invocation of
irq_move_irq() conditional avoids the out of line call if the pending bit
is not set.
Reuse the new function in apic_ack_edge().
Preparatory change for the real fix
Fixes: dccfe3147b42 ("x86/vector: Simplify vector move cleanup")
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: [email protected]
---
arch/x86/include/asm/apic.h | 2 ++
arch/x86/kernel/apic/vector.c | 10 ++++++++--
2 files changed, 10 insertions(+), 2 deletions(-)
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -436,6 +436,8 @@ static inline void apic_set_eoi_write(vo
#endif /* CONFIG_X86_LOCAL_APIC */
+extern void apic_ack_irq(struct irq_data *data);
+
static inline void ack_APIC_irq(void)
{
/*
--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -809,11 +809,17 @@ static int apic_retrigger_irq(struct irq
return 1;
}
+void apic_ack_irq(struct irq_data *irqd)
+{
+ if (unlikely(irqd_is_setaffinity_pending(irqd)))
+ irq_move_irq(irqd);
+ ack_APIC_irq();
+}
+
void apic_ack_edge(struct irq_data *irqd)
{
irq_complete_move(irqd_cfg(irqd));
- irq_move_irq(irqd);
- ack_APIC_irq();
+ apic_ack_irq(irqd);
}
static struct irq_chip lapic_controller = {
On Mon, Jun 4, 2018 at 8:33 AM, Thomas Gleixner <[email protected]> wrote:
> apic_ack_edge() is explicitely for handling interrupt affinity cleanup when
> interrupt remapping is not available or disable.
>
> Remapped interrupts and also some of the platform specific special
> interrupts, e.g. UV, invoke ack_APIC_irq() directly.
>
> To address the issue of failing an affinity update with -EBUSY the delayed
> affinity mechanism can be reused, but ack_APIC_irq() does not handle
> that. Adding this to ack_APIC_irq() is not possible, because that function
> is also used for exceptions and directly handled interrupts like IPIs.
>
> Create a new function, which just contains the conditional invocation of
> irq_move_irq() and the final ack_APIC_irq(). Making the invocation of
> irq_move_irq() conditional avoids the out of line call if the pending bit
> is not set.
>
> Reuse the new function in apic_ack_edge().
>
> Preparatory change for the real fix
>
> Fixes: dccfe3147b42 ("x86/vector: Simplify vector move cleanup")
> Signed-off-by: Thomas Gleixner <[email protected]>
> Cc: [email protected]
Tested-by: Song Liu <[email protected]>
> ---
> arch/x86/include/asm/apic.h | 2 ++
> arch/x86/kernel/apic/vector.c | 10 ++++++++--
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> --- a/arch/x86/include/asm/apic.h
> +++ b/arch/x86/include/asm/apic.h
> @@ -436,6 +436,8 @@ static inline void apic_set_eoi_write(vo
>
> #endif /* CONFIG_X86_LOCAL_APIC */
>
> +extern void apic_ack_irq(struct irq_data *data);
> +
> static inline void ack_APIC_irq(void)
> {
> /*
> --- a/arch/x86/kernel/apic/vector.c
> +++ b/arch/x86/kernel/apic/vector.c
> @@ -809,11 +809,17 @@ static int apic_retrigger_irq(struct irq
> return 1;
> }
>
> +void apic_ack_irq(struct irq_data *irqd)
> +{
> + if (unlikely(irqd_is_setaffinity_pending(irqd)))
> + irq_move_irq(irqd);
> + ack_APIC_irq();
> +}
> +
> void apic_ack_edge(struct irq_data *irqd)
> {
> irq_complete_move(irqd_cfg(irqd));
> - irq_move_irq(irqd);
> - ack_APIC_irq();
> + apic_ack_irq(irqd);
> }
>
> static struct irq_chip lapic_controller = {
>
>
Hi Thomas,
At 06/04/2018 11:33 PM, Thomas Gleixner wrote:
> apic_ack_edge() is explicitely for handling interrupt affinity cleanup when
> interrupt remapping is not available or disable.
>
> Remapped interrupts and also some of the platform specific special
> interrupts, e.g. UV, invoke ack_APIC_irq() directly.
>
> To address the issue of failing an affinity update with -EBUSY the delayed
> affinity mechanism can be reused, but ack_APIC_irq() does not handle
> that. Adding this to ack_APIC_irq() is not possible, because that function
> is also used for exceptions and directly handled interrupts like IPIs.
>
> Create a new function, which just contains the conditional invocation of
> irq_move_irq() and the final ack_APIC_irq(). Making the invocation of
> irq_move_irq() conditional avoids the out of line call if the pending bit
> is not set.
>
> Reuse the new function in apic_ack_edge().
>
> Preparatory change for the real fix
>
> Fixes: dccfe3147b42 ("x86/vector: Simplify vector move cleanup")
> Signed-off-by: Thomas Gleixner<[email protected]>
> Cc:[email protected]
> ---
> arch/x86/include/asm/apic.h | 2 ++
> arch/x86/kernel/apic/vector.c | 10 ++++++++--
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> --- a/arch/x86/include/asm/apic.h
> +++ b/arch/x86/include/asm/apic.h
> @@ -436,6 +436,8 @@ static inline void apic_set_eoi_write(vo
>
> #endif /* CONFIG_X86_LOCAL_APIC */
>
> +extern void apic_ack_irq(struct irq_data *data);
> +
> static inline void ack_APIC_irq(void)
> {
> /*
> --- a/arch/x86/kernel/apic/vector.c
> +++ b/arch/x86/kernel/apic/vector.c
> @@ -809,11 +809,17 @@ static int apic_retrigger_irq(struct irq
> return 1;
> }
>
> +void apic_ack_irq(struct irq_data *irqd)
> +{
> + if (unlikely(irqd_is_setaffinity_pending(irqd)))
Affinity pending is also judged in
> + irq_move_irq(irqd);
If we can remove the if(...) statement here
Thanks,
dou
> + ack_APIC_irq();
> +}
> +
> void apic_ack_edge(struct irq_data *irqd)
> {
> irq_complete_move(irqd_cfg(irqd));
> - irq_move_irq(irqd);
> - ack_APIC_irq();
> + apic_ack_irq(irqd);
> }
>
> static struct irq_chip lapic_controller = {
On Tue, 5 Jun 2018, Dou Liyang wrote:
> > +{
> > + if (unlikely(irqd_is_setaffinity_pending(irqd)))
>
> Affinity pending is also judged in
>
> > + irq_move_irq(irqd);
>
> If we can remove the if(...) statement here
That requires to fix all call sites in ia64 and that's why I didn't. But
we can make irq_move_irq() an inline function and have the check in the
inline.
Thanks,
tglx
Hi Thomas,
At 06/05/2018 07:41 PM, Thomas Gleixner wrote:
> On Tue, 5 Jun 2018, Dou Liyang wrote:
>>> +{
>>> + if (unlikely(irqd_is_setaffinity_pending(irqd)))
>>
>> Affinity pending is also judged in
>>
>>> + irq_move_irq(irqd);
>>
>> If we can remove the if(...) statement here
>
> That requires to fix all call sites in ia64 and that's why I didn't. But
I didn't express clearly, I meant remove the if(...) statement from
apic_ack_irq(), it doesn't require to fix the call sites in ia64.
+void apic_ack_irq(struct irq_data *irqd)
+{
+ irq_move_irq(irqd);
+ ack_APIC_irq();
+}
BTW, If apic_ack_irq() can accept _any_ irq_data when hierarchical
irqdomains are enabled[1]? If it is true, If there is a situation in
the original code that we should avoid:
If the top-level irq_data has the IRQD_SETAFFINITY_PENDING state, but
non-top-level irq_data state not, when using non-top-level irq_data in
apic_ack_irq(), we may skip the irq_move_irq() which we should call.
[1] commit 77ed42f18edd("genirq: Prevent crash in irq_move_irq()")
> we can make irq_move_irq() an inline function and have the check in the
> inline.
>
I don't know why do we need to make irq_move_irq() an inline function.
And, yes, irq_move_irq() has already had the check
...
if (likely(!irqd_is_setaffinity_pending(idata)))
return;
...
Thanks,
dou
On Wed, 6 Jun 2018, Dou Liyang wrote:
> Hi Thomas,
>
> At 06/05/2018 07:41 PM, Thomas Gleixner wrote:
> > On Tue, 5 Jun 2018, Dou Liyang wrote:
> > > > +{
> > > > + if (unlikely(irqd_is_setaffinity_pending(irqd)))
> > >
> > > Affinity pending is also judged in
> > >
> > > > + irq_move_irq(irqd);
> > >
> > > If we can remove the if(...) statement here
> >
> > That requires to fix all call sites in ia64 and that's why I didn't. But
>
> I didn't express clearly, I meant remove the if(...) statement from
> apic_ack_irq(), it doesn't require to fix the call sites in ia64.
I put the check there on purpose as I explained in the changelog:
Making the invocation of irq_move_irq() conditional avoids the out of
line call if the pending bit is not set.
Thanks,
tglx
Hi Thomas,
At 06/06/2018 04:04 PM, Thomas Gleixner wrote:
> On Wed, 6 Jun 2018, Dou Liyang wrote:
>
>> Hi Thomas,
>>
>> At 06/05/2018 07:41 PM, Thomas Gleixner wrote:
>>> On Tue, 5 Jun 2018, Dou Liyang wrote:
>>>>> +{
>>>>> + if (unlikely(irqd_is_setaffinity_pending(irqd)))
>>>>
>>>> Affinity pending is also judged in
>>>>
>>>>> + irq_move_irq(irqd);
>>>>
>>>> If we can remove the if(...) statement here
>>>
>>> That requires to fix all call sites in ia64 and that's why I didn't. But
>>
>> I didn't express clearly, I meant remove the if(...) statement from
>> apic_ack_irq(), it doesn't require to fix the call sites in ia64.
>
> I put the check there on purpose as I explained in the changelog:
>
> Making the invocation of irq_move_irq() conditional avoids the out of
> line call if the pending bit is not set.
>
I completely understand now, thank you so much. :-)
Thanks,
dou
> Thanks,
>
> tglx
>
>
>
Commit-ID: d340ebd696f921d3ad01b8c0c29dd38f2ad2bf3e
Gitweb: https://git.kernel.org/tip/d340ebd696f921d3ad01b8c0c29dd38f2ad2bf3e
Author: Thomas Gleixner <[email protected]>
AuthorDate: Wed, 6 Jun 2018 14:46:59 +0200
Committer: Thomas Gleixner <[email protected]>
CommitDate: Wed, 6 Jun 2018 15:18:20 +0200
genirq/migration: Avoid out of line call if pending is not set
The upcoming fix for the -EBUSY return from affinity settings requires to
use the irq_move_irq() functionality even on irq remapped interrupts. To
avoid the out of line call, move the check for the pending bit into an
inline helper.
Preparatory change for the real fix. No functional change.
Fixes: dccfe3147b42 ("x86/vector: Simplify vector move cleanup")
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Song Liu <[email protected]>
Cc: Dmitry Safonov <[email protected]>
Cc: [email protected]
Cc: Mike Travis <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Tariq Toukan <[email protected]>
Cc: Dou Liyang <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
include/linux/irq.h | 7 ++++++-
kernel/irq/migration.c | 5 +----
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 65916a305f3d..4e66378f290b 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -551,7 +551,12 @@ extern int irq_affinity_online_cpu(unsigned int cpu);
#endif
#if defined(CONFIG_SMP) && defined(CONFIG_GENERIC_PENDING_IRQ)
-void irq_move_irq(struct irq_data *data);
+void __irq_move_irq(struct irq_data *data);
+static inline void irq_move_irq(struct irq_data *data)
+{
+ if (unlikely(irqd_is_setaffinity_pending(data)))
+ __irq_move_irq(data);
+}
void irq_move_masked_irq(struct irq_data *data);
void irq_force_complete_move(struct irq_desc *desc);
#else
diff --git a/kernel/irq/migration.c b/kernel/irq/migration.c
index 8b8cecd18cce..def48589ea48 100644
--- a/kernel/irq/migration.c
+++ b/kernel/irq/migration.c
@@ -91,7 +91,7 @@ void irq_move_masked_irq(struct irq_data *idata)
cpumask_clear(desc->pending_mask);
}
-void irq_move_irq(struct irq_data *idata)
+void __irq_move_irq(struct irq_data *idata)
{
bool masked;
@@ -102,9 +102,6 @@ void irq_move_irq(struct irq_data *idata)
*/
idata = irq_desc_get_irq_data(irq_data_to_desc(idata));
- if (likely(!irqd_is_setaffinity_pending(idata)))
- return;
-
if (unlikely(irqd_irq_disabled(idata)))
return;
Commit-ID: c0255770ccdc77ef2184d2a0a2e0cde09d2b44a4
Gitweb: https://git.kernel.org/tip/c0255770ccdc77ef2184d2a0a2e0cde09d2b44a4
Author: Thomas Gleixner <[email protected]>
AuthorDate: Mon, 4 Jun 2018 17:33:55 +0200
Committer: Thomas Gleixner <[email protected]>
CommitDate: Wed, 6 Jun 2018 15:18:20 +0200
x86/apic: Provide apic_ack_irq()
apic_ack_edge() is explicitely for handling interrupt affinity cleanup when
interrupt remapping is not available or disable.
Remapped interrupts and also some of the platform specific special
interrupts, e.g. UV, invoke ack_APIC_irq() directly.
To address the issue of failing an affinity update with -EBUSY the delayed
affinity mechanism can be reused, but ack_APIC_irq() does not handle
that. Adding this to ack_APIC_irq() is not possible, because that function
is also used for exceptions and directly handled interrupts like IPIs.
Create a new function, which just contains the conditional invocation of
irq_move_irq() and the final ack_APIC_irq().
Reuse the new function in apic_ack_edge().
Preparatory change for the real fix.
Fixes: dccfe3147b42 ("x86/vector: Simplify vector move cleanup")
Signed-off-by: Thomas Gleixner <[email protected]>
Tested-by: Song Liu <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Song Liu <[email protected]>
Cc: Dmitry Safonov <[email protected]>
Cc: [email protected]
Cc: Mike Travis <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Tariq Toukan <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
arch/x86/include/asm/apic.h | 2 ++
arch/x86/kernel/apic/vector.c | 9 +++++++--
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 08acd954f00e..74a9e06b6cfd 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -436,6 +436,8 @@ static inline void apic_set_eoi_write(void (*eoi_write)(u32 reg, u32 v)) {}
#endif /* CONFIG_X86_LOCAL_APIC */
+extern void apic_ack_irq(struct irq_data *data);
+
static inline void ack_APIC_irq(void)
{
/*
diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
index 72b575a0b662..b708f597eee3 100644
--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -809,13 +809,18 @@ static int apic_retrigger_irq(struct irq_data *irqd)
return 1;
}
-void apic_ack_edge(struct irq_data *irqd)
+void apic_ack_irq(struct irq_data *irqd)
{
- irq_complete_move(irqd_cfg(irqd));
irq_move_irq(irqd);
ack_APIC_irq();
}
+void apic_ack_edge(struct irq_data *irqd)
+{
+ irq_complete_move(irqd_cfg(irqd));
+ apic_ack_irq(irqd);
+}
+
static struct irq_chip lapic_controller = {
.name = "APIC",
.irq_ack = apic_ack_edge,