From: Thomas Gleixner <tglx@linutronix.de>
To: Anchal Agarwal <anchalag@amazon.com>
Cc: mingo@redhat.com, bp@alien8.de, hpa@zytor.com, x86@kernel.org,
boris.ostrovsky@oracle.com, jgross@suse.com,
linux-pm@vger.kernel.org, linux-mm@kvack.org, kamatam@amazon.com,
sstabellini@kernel.org, konrad.wilk@oracle.co,
roger.pau@citrix.com, axboe@kernel.dk, davem@davemloft.net,
rjw@rjwysocki.net, len.brown@intel.com, pavel@ucw.cz,
peterz@infradead.org, eduval@amazon.com, sblbir@amazon.com,
xen-devel@lists.xenproject.org, vkuznets@redhat.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
dwmw@amazon.co.uk, fllinden@amazon.com, anchalag@amazon.com
Subject: Re: [RFC PATCH V2 09/11] xen: Clear IRQD_IRQ_STARTED flag during shutdown PIRQs
Date: Fri, 10 Jan 2020 20:13:16 +0100 [thread overview]
Message-ID: <87zhevrupf.fsf@nanos.tec.linutronix.de> (raw)
In-Reply-To: <20200109234050.GA26381@dev-dsk-anchalag-2a-9c2d1d96.us-west-2.amazon.com>
Anchal,
Anchal Agarwal <anchalag@amazon.com> writes:
> On Thu, Jan 09, 2020 at 01:07:27PM +0100, Thomas Gleixner wrote:
>> Anchal Agarwal <anchalag@amazon.com> writes:
>> So either you can handle it purely on the XEN side without touching any
>> core state or you need to come up with some way of letting the core code
>> know that it should invoke shutdown instead of disable.
>>
>> Something like the completely untested patch below.
>
> Understandable. Really appreciate the patch suggestion below and i will test it
> for sure and see if things can be fixed properly in irq core if thats the only
> option. In the meanwhile, I tried to fix it on xen side unless it gives you the
> same feeling as above? MSI-x are just fine, just ioapic ones don't get any event
> channel asssigned hence enable_dynirq does nothing. Those needs to be restarted.
>
> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
> index 1bb0b522d004..2ed152f35816 100644
> --- a/drivers/xen/events/events_base.c
> +++ b/drivers/xen/events/events_base.c
> @@ -575,6 +575,11 @@ static void shutdown_pirq(struct irq_data *data)
>
> static void enable_pirq(struct irq_data *data)
> {
> +/*ioapic interrupts don't get event channel assigned
> + * after being explicitly shutdown during guest
> + * hibernation. They need to be restarted*/
> + if(!evtchn_from_irq(data->irq))
> + startup_pirq(data);
> enable_dynirq(data);
> }
Interesting patch format :)
Doing the shutdown from syscore_ops and the startup conditionally in a
totaly unrelated function is not really intuitive.
So either you do it symmetrically in XEN via syscore_ops callbacks or
you let the irq core code help you out with the patch I provided
Thanks,
tglx
WARNING: multiple messages have this Message-ID
From: Thomas Gleixner <tglx@linutronix.de>
To: Anchal Agarwal <anchalag@amazon.com>
Cc: konrad.wilk@oracle.co, eduval@amazon.com, peterz@infradead.org,
x86@kernel.org, linux-mm@kvack.org, axboe@kernel.dk,
pavel@ucw.cz, hpa@zytor.com, sstabellini@kernel.org,
kamatam@amazon.com, mingo@redhat.com,
xen-devel@lists.xenproject.org, sblbir@amazon.com,
len.brown@intel.com, linux-pm@vger.kernel.org,
anchalag@amazon.com, bp@alien8.de, boris.ostrovsky@oracle.com,
jgross@suse.com, netdev@vger.kernel.org, fllinden@amazon.com,
rjw@rjwysocki.net, linux-kernel@vger.kernel.org,
vkuznets@redhat.com, davem@davemloft.net, dwmw@amazon.co.uk,
roger.pau@citrix.com
Subject: Re: [Xen-devel] [RFC PATCH V2 09/11] xen: Clear IRQD_IRQ_STARTED flag during shutdown PIRQs
Date: Fri, 10 Jan 2020 20:13:16 +0100 [thread overview]
Message-ID: <87zhevrupf.fsf@nanos.tec.linutronix.de> (raw)
In-Reply-To: <20200109234050.GA26381@dev-dsk-anchalag-2a-9c2d1d96.us-west-2.amazon.com>
Anchal,
Anchal Agarwal <anchalag@amazon.com> writes:
> On Thu, Jan 09, 2020 at 01:07:27PM +0100, Thomas Gleixner wrote:
>> Anchal Agarwal <anchalag@amazon.com> writes:
>> So either you can handle it purely on the XEN side without touching any
>> core state or you need to come up with some way of letting the core code
>> know that it should invoke shutdown instead of disable.
>>
>> Something like the completely untested patch below.
>
> Understandable. Really appreciate the patch suggestion below and i will test it
> for sure and see if things can be fixed properly in irq core if thats the only
> option. In the meanwhile, I tried to fix it on xen side unless it gives you the
> same feeling as above? MSI-x are just fine, just ioapic ones don't get any event
> channel asssigned hence enable_dynirq does nothing. Those needs to be restarted.
>
> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
> index 1bb0b522d004..2ed152f35816 100644
> --- a/drivers/xen/events/events_base.c
> +++ b/drivers/xen/events/events_base.c
> @@ -575,6 +575,11 @@ static void shutdown_pirq(struct irq_data *data)
>
> static void enable_pirq(struct irq_data *data)
> {
> +/*ioapic interrupts don't get event channel assigned
> + * after being explicitly shutdown during guest
> + * hibernation. They need to be restarted*/
> + if(!evtchn_from_irq(data->irq))
> + startup_pirq(data);
> enable_dynirq(data);
> }
Interesting patch format :)
Doing the shutdown from syscore_ops and the startup conditionally in a
totaly unrelated function is not really intuitive.
So either you do it symmetrically in XEN via syscore_ops callbacks or
you let the irq core code help you out with the patch I provided
Thanks,
tglx
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2020-01-10 19:13 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-07 23:44 [RFC PATCH V2 09/11] xen: Clear IRQD_IRQ_STARTED flag during shutdown PIRQs Anchal Agarwal
2020-01-07 23:44 ` [Xen-devel] " Anchal Agarwal
2020-01-08 15:23 ` Thomas Gleixner
2020-01-08 15:23 ` [Xen-devel] " Thomas Gleixner
2020-01-08 21:24 ` Anchal Agarwal
2020-01-08 21:24 ` [Xen-devel] " Anchal Agarwal
2020-01-09 12:07 ` Thomas Gleixner
2020-01-09 12:07 ` [Xen-devel] " Thomas Gleixner
2020-01-09 23:40 ` Anchal Agarwal
2020-01-09 23:40 ` [Xen-devel] " Anchal Agarwal
2020-01-10 19:13 ` Thomas Gleixner [this message]
2020-01-10 19:13 ` Thomas Gleixner
2020-01-10 22:57 ` Anchal Agarwal
2020-01-10 22:57 ` [Xen-devel] " Anchal Agarwal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87zhevrupf.fsf@nanos.tec.linutronix.de \
--to=tglx@linutronix.de \
--cc=anchalag@amazon.com \
--cc=axboe@kernel.dk \
--cc=boris.ostrovsky@oracle.com \
--cc=bp@alien8.de \
--cc=davem@davemloft.net \
--cc=dwmw@amazon.co.uk \
--cc=eduval@amazon.com \
--cc=fllinden@amazon.com \
--cc=hpa@zytor.com \
--cc=jgross@suse.com \
--cc=kamatam@amazon.com \
--cc=konrad.wilk@oracle.co \
--cc=len.brown@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-pm@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=pavel@ucw.cz \
--cc=peterz@infradead.org \
--cc=rjw@rjwysocki.net \
--cc=roger.pau@citrix.com \
--cc=sblbir@amazon.com \
--cc=sstabellini@kernel.org \
--cc=vkuznets@redhat.com \
--cc=x86@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.