From: Anchal Agarwal <anchalag@amazon.com>
To: Thomas Gleixner <tglx@linutronix.de>
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.com>, <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@amaozn.com>,
<benh@kernel.crashing.org>
Subject: Re: [EXTERNAL][RFC PATCH v3 07/12] genirq: Shutdown irq chips in suspend/resume during hibernation
Date: Mon, 9 Mar 2020 22:37:02 +0000 [thread overview]
Message-ID: <20200309223702.GA8513@dev-dsk-anchalag-2a-9c2d1d96.us-west-2.amazon.com> (raw)
In-Reply-To: <87ftelaxwn.fsf@nanos.tec.linutronix.de>
On Sat, Mar 07, 2020 at 12:03:52AM +0100, Thomas Gleixner wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> Anchal Agarwal <anchalag@amazon.com> writes:
>
> > There are no pm handlers for the legacy devices, so during tear down
> > stale event channel <> IRQ mapping may still remain in the image and
> > resume may fail. To avoid adding much code by implementing handlers for
> > legacy devices, add a new irq_chip flag IRQCHIP_SHUTDOWN_ON_SUSPEND which
> > when enabled on an irq-chip e.g xen-pirq, it will let core suspend/resume
> > irq code to shutdown and restart the active irqs. PM suspend/hibernation
> > code will rely on this.
> > Without this, in PM hibernation, information about the event channel
> > remains in hibernation image, but there is no guarantee that the same
> > event channel numbers are assigned to the devices when restoring the
> > system. This may cause conflict like the following and prevent some
> > devices from being restored correctly.
>
> The above is just an agglomeration of words and acronyms and some of
> these sentences do not even make sense. Anyone who is not aware of event
> channels and whatever XENisms you talk about will be entirely
> confused. Changelogs really need to be understandable for mere mortals
> and there is no space restriction so acronyms can be written out.
>
I don't understand what does not makes sense here. Of course the one you
described is more elaborate and explanatory and I agree I just wrote a short
one from perspective of PM hibernation related to Xen domU.
All I explained was why teardown is needed, what is the solution and
what will happen if we do not clear those mappings.
> Something like this:
>
> Many legacy device drivers do not implement power management (PM)
> functions which means that interrupts requested by these drivers stay
> in active state when the kernel is hibernated.
>
> This does not matter on bare metal and on most hypervisors because the
> interrupt is restored on resume without any noticable side effects as
> it stays connected to the same physical or virtual interrupt line.
>
> The XEN interrupt mechanism is different as it maintains a mapping
> between the Linux interrupt number and a XEN event channel. If the
> interrupt stays active on hibernation this mapping is preserved but
> there is unfortunately no guarantee that on resume the same event
> channels are reassigned to these devices. This can result in event
> channel conflicts which prevent the affected devices from being
> restored correctly.
>
> One way to solve this would be to add the necessary power management
> functions to all affected legacy device drivers, but that's a
> questionable effort which does not provide any benefits on non-XEN
> environments.
>
> The least intrusive and most efficient solution is to provide a
> mechanism which allows the core interrupt code to tear down these
> interrupts on hibernation and bring them back up again on resume. This
> allows the XEN event channel mechanism to assign an arbitrary event
> channel on resume without affecting the functionality of these
> devices.
>
> Fortunately all these device interrupts are handled by a dedicated XEN
> interrupt chip so the chip can be marked that all interrupts connected
> to it are handled this way. This is pretty much in line with the other
> interrupt chip specific quirks, e.g. IRQCHIP_MASK_ON_SUSPEND.
>
> Add a new quirk flag IRQCHIP_SHUTDOWN_ON_SUSPEND and add support for
> it the core interrupt suspend/resume paths.
>
> Hmm?
>
Sure.
> > Signed-off-by: Anchal Agarwal <anchalag@amazon.com>
> > Suggested-by: Thomas Gleixner <tglx@linutronix.de>
>
> Not that I care much, but now that I've written both the patch and the
> changelog you might change that attribution slightly. For completeness
> sake:
>
Why not. That's mandated now :)
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>
> Thanks,
>
> tglx
Thanks,
Anchal
next prev parent reply other threads:[~2020-03-09 22:37 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-14 23:21 [RFC RESEND PATCH v3 00/12] Enable PM hibernation on guest VMs Anchal Agarwal
2020-02-14 23:22 ` [RFC PATCH v3 01/12] xen/manage: keep track of the on-going suspend mode Anchal Agarwal
2020-02-14 23:23 ` [RFC PATCH v3 02/12] xenbus: add freeze/thaw/restore callbacks support Anchal Agarwal
2020-02-14 23:23 ` [RFC PATCH v3 03/12] x86/xen: Introduce new function to map HYPERVISOR_shared_info on Resume Anchal Agarwal
2020-02-14 23:24 ` [RFC PATCH v3 04/12] x86/xen: add system core suspend and resume callbacks Anchal Agarwal
2020-02-14 23:24 ` [RFC PATCH v3 05/12] xen-netfront: add callbacks for PM suspend and hibernation support Anchal Agarwal
2020-02-14 23:25 ` [RFC PATCH v3 06/12] xen-blkfront: add callbacks for PM suspend and hibernation Anchal Agarwal
2020-02-17 10:05 ` Roger Pau Monné
2020-02-17 23:05 ` Anchal Agarwal
2020-02-18 9:16 ` Roger Pau Monné
2020-02-19 18:04 ` Anchal Agarwal
2020-02-20 8:39 ` Roger Pau Monné
2020-02-20 8:54 ` [Xen-devel] " Durrant, Paul
2020-02-20 15:45 ` Roger Pau Monné
2020-02-20 16:23 ` Durrant, Paul
2020-02-20 16:48 ` Roger Pau Monné
2020-02-20 17:01 ` Durrant, Paul
2020-02-21 0:49 ` Anchal Agarwal
2020-02-21 9:47 ` Roger Pau Monné
2020-02-21 9:22 ` Roger Pau Monné
2020-02-21 9:56 ` Durrant, Paul
2020-02-21 10:21 ` Roger Pau Monné
2020-02-21 10:33 ` Durrant, Paul
2020-02-21 11:51 ` Roger Pau Monné
2020-02-21 14:24 ` Roger Pau Monné
2020-03-06 18:40 ` Anchal Agarwal
2020-03-09 9:54 ` Roger Pau Monné
[not found] ` <FA688A68-5372-4757-B075-A69A45671CB9@amazon.com>
[not found] ` <20200312090435.GK24449@Air-de-Roger.citrite.net>
2020-03-13 17:21 ` Anchal Agarwal
2020-02-14 23:25 ` [RFC PATCH v3 07/12] genirq: Shutdown irq chips in suspend/resume during hibernation Anchal Agarwal
2020-03-06 23:03 ` Thomas Gleixner
2020-03-09 22:37 ` Anchal Agarwal [this message]
2020-02-14 23:26 ` [RFC PATCH v3 08/12] xen/time: introduce xen_{save,restore}_steal_clock Anchal Agarwal
2020-02-14 23:27 ` [RFC PATCH v3 09/12] x86/xen: save and restore steal clock Anchal Agarwal
2020-02-14 23:27 ` [RFC PATCH v3 10/12] xen: Introduce wrapper for save/restore sched clock offset Anchal Agarwal
2020-02-14 23:27 ` [RFC PATCH v3 11/12] xen: Update sched clock offset to avoid system instability in hibernation Anchal Agarwal
2020-02-14 23:28 ` [RFC PATCH v3 12/12] PM / hibernate: update the resume offset on SNAPSHOT_SET_SWAP_AREA 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=20200309223702.GA8513@dev-dsk-anchalag-2a-9c2d1d96.us-west-2.amazon.com \
--to=anchalag@amazon.com \
--cc=axboe@kernel.dk \
--cc=benh@kernel.crashing.org \
--cc=boris.ostrovsky@oracle.com \
--cc=bp@alien8.de \
--cc=davem@davemloft.net \
--cc=dwmw@amazon.co.uk \
--cc=eduval@amazon.com \
--cc=fllinden@amaozn.com \
--cc=hpa@zytor.com \
--cc=jgross@suse.com \
--cc=kamatam@amazon.com \
--cc=konrad.wilk@oracle.com \
--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=tglx@linutronix.de \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).