Otherwise would cause trouble...
Cc: Dimitri Sivanich <[email protected]>
Signed-off-by: Sheng Yang <[email protected]>
---
arch/x86/kernel/irq.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
index b0cdde6..78b23d0 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -257,7 +257,8 @@ void smp_generic_interrupt(struct pt_regs *regs)
{
struct pt_regs *old_regs = set_irq_regs(regs);
- ack_APIC_irq();
+ if (!disable_apic)
+ ack_APIC_irq();
exit_idle();
--
1.5.4.5
* Sheng Yang <[email protected]> wrote:
> Otherwise would cause trouble...
>
> Cc: Dimitri Sivanich <[email protected]>
> Signed-off-by: Sheng Yang <[email protected]>
> ---
> arch/x86/kernel/irq.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
> index b0cdde6..78b23d0 100644
> --- a/arch/x86/kernel/irq.c
> +++ b/arch/x86/kernel/irq.c
> @@ -257,7 +257,8 @@ void smp_generic_interrupt(struct pt_regs *regs)
> {
> struct pt_regs *old_regs = set_irq_regs(regs);
>
> - ack_APIC_irq();
> + if (!disable_apic)
> + ack_APIC_irq();
Wont in that case the apic->ack method be a NOP?
Ingo
[Ingo Molnar - Sun, Sep 20, 2009 at 08:06:32PM +0200]
|
| * Sheng Yang <[email protected]> wrote:
|
| > Otherwise would cause trouble...
| >
| > Cc: Dimitri Sivanich <[email protected]>
| > Signed-off-by: Sheng Yang <[email protected]>
| > ---
| > arch/x86/kernel/irq.c | 3 ++-
| > 1 files changed, 2 insertions(+), 1 deletions(-)
| >
| > diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
| > index b0cdde6..78b23d0 100644
| > --- a/arch/x86/kernel/irq.c
| > +++ b/arch/x86/kernel/irq.c
| > @@ -257,7 +257,8 @@ void smp_generic_interrupt(struct pt_regs *regs)
| > {
| > struct pt_regs *old_regs = set_irq_regs(regs);
| >
| > - ack_APIC_irq();
| > + if (!disable_apic)
| > + ack_APIC_irq();
|
| Wont in that case the apic->ack method be a NOP?
|
| Ingo
|
iirc it was Xen related patch. So it's not that simple.
I've pointed out Sheng about disable_apic. I'm not Xen
specialist but Xen team seem to use specific apic setup
so our "dummy" operations are not involved (case they
set disable_apic=1 without "turn off" apic ops in real).
Something like that.
-- Cyrill
* Cyrill Gorcunov <[email protected]> wrote:
> [Ingo Molnar - Sun, Sep 20, 2009 at 08:06:32PM +0200]
> |
> | * Sheng Yang <[email protected]> wrote:
> |
> | > Otherwise would cause trouble...
> | >
> | > Cc: Dimitri Sivanich <[email protected]>
> | > Signed-off-by: Sheng Yang <[email protected]>
> | > ---
> | > arch/x86/kernel/irq.c | 3 ++-
> | > 1 files changed, 2 insertions(+), 1 deletions(-)
> | >
> | > diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
> | > index b0cdde6..78b23d0 100644
> | > --- a/arch/x86/kernel/irq.c
> | > +++ b/arch/x86/kernel/irq.c
> | > @@ -257,7 +257,8 @@ void smp_generic_interrupt(struct pt_regs *regs)
> | > {
> | > struct pt_regs *old_regs = set_irq_regs(regs);
> | >
> | > - ack_APIC_irq();
> | > + if (!disable_apic)
> | > + ack_APIC_irq();
> |
> | Wont in that case the apic->ack method be a NOP?
> |
> | Ingo
> |
>
> iirc it was Xen related patch. So it's not that simple.
>
> I've pointed out Sheng about disable_apic. I'm not Xen
> specialist but Xen team seem to use specific apic setup
> so our "dummy" operations are not involved (case they
> set disable_apic=1 without "turn off" apic ops in real).
> Something like that.
They should then set a NOP function in that case. We really dont want to
slow down hotpath functions like smp_generic_interrupt() with flaggery.
Ingo
[Ingo Molnar - Sun, Sep 20, 2009 at 08:21:24PM +0200]
|
| * Cyrill Gorcunov <[email protected]> wrote:
|
| > [Ingo Molnar - Sun, Sep 20, 2009 at 08:06:32PM +0200]
| > |
| > | * Sheng Yang <[email protected]> wrote:
| > |
| > | > Otherwise would cause trouble...
| > | >
| > | > Cc: Dimitri Sivanich <[email protected]>
| > | > Signed-off-by: Sheng Yang <[email protected]>
| > | > ---
| > | > arch/x86/kernel/irq.c | 3 ++-
| > | > 1 files changed, 2 insertions(+), 1 deletions(-)
| > | >
| > | > diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
| > | > index b0cdde6..78b23d0 100644
| > | > --- a/arch/x86/kernel/irq.c
| > | > +++ b/arch/x86/kernel/irq.c
| > | > @@ -257,7 +257,8 @@ void smp_generic_interrupt(struct pt_regs *regs)
| > | > {
| > | > struct pt_regs *old_regs = set_irq_regs(regs);
| > | >
| > | > - ack_APIC_irq();
| > | > + if (!disable_apic)
| > | > + ack_APIC_irq();
| > |
| > | Wont in that case the apic->ack method be a NOP?
| > |
| > | Ingo
| > |
| >
| > iirc it was Xen related patch. So it's not that simple.
| >
| > I've pointed out Sheng about disable_apic. I'm not Xen
| > specialist but Xen team seem to use specific apic setup
| > so our "dummy" operations are not involved (case they
| > set disable_apic=1 without "turn off" apic ops in real).
| > Something like that.
|
| They should then set a NOP function in that case. We really dont want to
| slow down hotpath functions like smp_generic_interrupt() with flaggery.
|
| Ingo
|
Well, I suppose we should wait for Sheng's comments.
I wish I would answer you but I simply don't know Xen
code :)
-- Cyrill
[Cyrill Gorcunov - Sun, Sep 20, 2009 at 10:30:11PM +0400]
...
| | >
| | > iirc it was Xen related patch. So it's not that simple.
| | >
| | > I've pointed out Sheng about disable_apic. I'm not Xen
| | > specialist but Xen team seem to use specific apic setup
| | > so our "dummy" operations are not involved (case they
| | > set disable_apic=1 without "turn off" apic ops in real).
| | > Something like that.
| |
| | They should then set a NOP function in that case. We really dont want to
| | slow down hotpath functions like smp_generic_interrupt() with flaggery.
| |
| | Ingo
| |
|
| Well, I suppose we should wait for Sheng's comments.
| I wish I would answer you but I simply don't know Xen
| code :)
|
| -- Cyrill
Wait a bit Ingo, please. It seems I'm having different
patch series in mind. Need to restore mail thread.
Will back soon :)
-- Cyrill
[Cyrill Gorcunov - Sun, Sep 20, 2009 at 10:42:03PM +0400]
| [Cyrill Gorcunov - Sun, Sep 20, 2009 at 10:30:11PM +0400]
| ...
| | | >
| | | > iirc it was Xen related patch. So it's not that simple.
| | | >
| | | > I've pointed out Sheng about disable_apic. I'm not Xen
| | | > specialist but Xen team seem to use specific apic setup
| | | > so our "dummy" operations are not involved (case they
| | | > set disable_apic=1 without "turn off" apic ops in real).
| | | > Something like that.
| | |
| | | They should then set a NOP function in that case. We really dont want to
| | | slow down hotpath functions like smp_generic_interrupt() with flaggery.
| | |
| | | Ingo
| | |
| |
| | Well, I suppose we should wait for Sheng's comments.
| | I wish I would answer you but I simply don't know Xen
| | code :)
| |
| | -- Cyrill
|
| Wait a bit Ingo, please. It seems I'm having different
| patch series in mind. Need to restore mail thread.
| Will back soon :)
|
| -- Cyrill
yeah, it comes from Xen RFC series. Here is a quote from
conversation.
> Sheng Yan
>
> | | is there was some problem with it? I'm asking you
> | | because if disable_apic=1 then any apic write/read
> | | operations become NOPs. So I don't see how it may
> | | hurt. But I could be missing something.
> | |
> | | -- Cyrill
> |
> | Ah, I see -- it's due to your other patch...
> | Hmm this makes all "disable apic" idea less
> | general. And safety of ack_APIC_irq is now
> | under suspicious.
>
> Um, probably. I've seen a ack_APIC_irq() in do_IRQ when handle_irq() fail.
> Seems the assumption that ack_APIC_irq() always safe is there. I will check if
> I can make it more elegant - maybe disable the warning in the Xen code...
>
Personally, I think "out-of-xen-thread" this patch is not needed.
And if this apic-ack operation causes any kind of problems --
this problem should be fixed without disable_apic involved.
-- Cyrill
On Monday 21 September 2009 02:49:06 Cyrill Gorcunov wrote:
> [Cyrill Gorcunov - Sun, Sep 20, 2009 at 10:42:03PM +0400]
>
> | [Cyrill Gorcunov - Sun, Sep 20, 2009 at 10:30:11PM +0400]
> | ...
> |
> | | | > iirc it was Xen related patch. So it's not that simple.
> | | | >
> | | | > I've pointed out Sheng about disable_apic. I'm not Xen
> | | | > specialist but Xen team seem to use specific apic setup
> | | | > so our "dummy" operations are not involved (case they
> | | | > set disable_apic=1 without "turn off" apic ops in real).
> | | | > Something like that.
> | | |
> | | | They should then set a NOP function in that case. We really dont want
> | | | to slow down hotpath functions like smp_generic_interrupt() with
> | | | flaggery.
> | | |
> | | | Ingo
> | |
> | | Well, I suppose we should wait for Sheng's comments.
> | | I wish I would answer you but I simply don't know Xen
> | | code :)
> | |
> | | -- Cyrill
> |
> | Wait a bit Ingo, please. It seems I'm having different
> | patch series in mind. Need to restore mail thread.
> | Will back soon :)
> |
> | -- Cyrill
>
> yeah, it comes from Xen RFC series. Here is a quote from
> conversation.
>
> > Sheng Yan
> >
> > | | is there was some problem with it? I'm asking you
> > | | because if disable_apic=1 then any apic write/read
> > | | operations become NOPs. So I don't see how it may
> > | | hurt. But I could be missing something.
> > | |
> > | | -- Cyrill
> > |
> > | Ah, I see -- it's due to your other patch...
> > | Hmm this makes all "disable apic" idea less
> > | general. And safety of ack_APIC_irq is now
> > | under suspicious.
> >
> > Um, probably. I've seen a ack_APIC_irq() in do_IRQ when handle_irq()
> > fail. Seems the assumption that ack_APIC_irq() always safe is there. I
> > will check if I can make it more elegant - maybe disable the warning in
> > the Xen code...
>
> Personally, I think "out-of-xen-thread" this patch is not needed.
> And if this apic-ack operation causes any kind of problems --
> this problem should be fixed without disable_apic involved.
In fact Xen set it to functional nop, but print warning when processing APIC
access. So Xen assumed that no apic access would in the Xen path. But now this
assumption have a little trouble with current hybrid implementation. I am
working on to fix this issue now, so please ignore this patch temporarily, I
would try to figure out a more elegant way to handle this issue and minimal
the native impact.
Thanks.
--
regards
Yang, Sheng