From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH] pvops: fix "xm save -c" issue Date: Tue, 25 Jan 2011 16:32:02 +0000 Message-ID: <1295973122.14780.6550.camel@zakaz.uk.xensource.com> References: <4D391B0D.8010708@jp.fujitsu.com> <20110121.152840.1536707498935022359.kaz@flab.fujitsu.co.jp> <1295601270.14780.151.camel@zakaz.uk.xensource.com> <20110125.162320.1888664918493072394.kaz@flab.fujitsu.co.jp> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20110125.162320.1888664918493072394.kaz@flab.fujitsu.co.jp> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: "SUZUKI, Kazuhiro" Cc: "xen-devel@lists.xensource.com" , "konrad.wilk@oracle.com" List-Id: xen-devel@lists.xenproject.org On Tue, 2011-01-25 at 07:23 +0000, SUZUKI, Kazuhiro wrote: > Hi Ian, > > I split the patch into pm infrastructure part and xen part. > Actually, I've never post to linux community, so I'm not sure what to > do. Documentation/SubmittingPatches in the Linux source tree has some good guidance. In general you need to post your patches inline (not as attachments which is a difference from the Xen maintainer's policy) with a changelog message, in this case explaining the need for the new event type and explaining the use cases etc, and a signed-off-by. You should send to the subsystem maintainer and the relevant mailing list, the MAINTAINERS file in the Linux source would help figure out who they are but in this case I think it is Rafael J. Wysocki and the linux-pm@lists.linux-foundation.org list. You should cc xen-devel as well. It will be interesting to see what upstream make of the use case. I suspect there might be some common ground with the ability to abort a hibernation attempt on native for example. Ian. > > Thanks, > KAZ > > Signed-off-by: Kenji Wakamiya > Signed-off-by: Kazuhiro Suzuki > > > From: Ian Campbell > Subject: Re: [Xen-devel] [PATCH] pvops: fix "xm save -c" issue > Date: Fri, 21 Jan 2011 09:14:30 +0000 > > > On Fri, 2011-01-21 at 06:28 +0000, SUZUKI, Kazuhiro wrote: > >> Hi, > >> > >> The following patch fixes 'xm save -c' issue. > >> We defined 'PMSG_CANCEL' message for suspend cancel situation and > >> suspend_cancel handler in pm_ops struct. > >> If the suspend_cancel is defined, suspend_cancel() is called instead > >> of resume(). > > > > Thanks. I like the general shape of this patch but the core pm > > infrastructure change needs to be split out and sent upstream via the > > power management maintainer. I think this is Rafael J. Wysocki > > and the linux-pm@lists.linux-foundation.org list. > > > > If you also split out the Xen conversion to dev_pm_ops, without the > > suspend_cancel bit, then I think we can take that bit now and then you > > can resend the bit which adds the suspend_cancel bits once the core > > stuff is upstream. > > > > Ian. > > > >> > >> Thanks, > >> KAZ > >> > >> Signed-off-by: Kenji Wakamiya > >> Signed-off-by: Kazuhiro Suzuki > >> > >> > >> From: Kenji Wakamiya > >> Subject: Re: [Xen-devel] [PATCH] pvops: fix "xm save -c" issue > >> Date: Fri, 21 Jan 2011 14:35:09 +0900 > >> > >> > Hi Konrand, and sorry for very late response. > >> > > >> > (2011/01/11 2:01), Konrad Rzeszutek Wilk wrote: > >> >>> With this change how is the effect of dpm_suspend_start undone in the > >> >>> suspend cancelled case? > >> >>> > >> >>> Currently we have > >> >>> dpm_suspend_start(PMSG_SUSPEND) > >> >>> xs_suspend > >> >>> dpm_suspend_noirq(PMSG_SUSPEND) > >> >>> SUSPEND > >> >>> dpm_resume_noirq(PMSG_RESUME) > >> >>> xs_resume or xs_supend_cancel > >> >>> dpm_resume_end(PMSG_RESUME) > >> >>> > >> >>> Which seems nicely nested and logical but by only calling dpm_resume_end > >> >>> in the non-cancelled case we seem to be unbalancing things. > >> >>> > >> >>> Do we need some sort of dpm_resume_cancel, or some way of pushing the > >> >>> cancelled flag down into the individual xenbus_device.resume handlers? > >> >>> > >> >>> Should we maybe simply be using a difference PMSG_XXX in the cancelled > >> >>> case? Is this what one of PMSG_RESTORE or PMSG_RECOVER means? > >> >>> > >> >>> Looks like to propagate the PMSG_* to the actual device resume functions > >> >>> we would need to provide a pm_ops for the struct bus xenbus_frontend > >> >>> instead of relying on the legacy handlers. This is probably a > >> >>> independently good idea anyway. > >> >> > >> >> ping? > >> >> > >> >> Kenji any ideas or patches to address Ian's comments? > >> > > >> > My colleague made a patch which reflected Ian's comments, so I will ask > >> > him to post it. Please wait a little. > >> > > >> > Thanks, > >> > Kenji > >> > > >> > > >> > _______________________________________________ > >> > Xen-devel mailing list > >> > Xen-devel@lists.xensource.com > >> > http://lists.xensource.com/xen-devel > > > >