Re: [Xen-devel] [PATCH] pvops: fix "xm save -c" issue

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

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 <rjw@xxxxxxx>
and the linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx 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 <wkenji@xxxxxxxxxxxxxx>
> Signed-off-by: Kazuhiro Suzuki <kaz@xxxxxxxxxxxxxx>
> 
> 
> From: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
> 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
> > <rjw@xxxxxxx> and the linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx 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 <wkenji@xxxxxxxxxxxxxx>
> >> Signed-off-by: Kazuhiro Suzuki <kaz@xxxxxxxxxxxxxx>
> >> 
> >> 
> >> From: Kenji Wakamiya <wkenji@xxxxxxxxxxxxxx>
> >> 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@xxxxxxxxxxxxxxxxxxx
> >> > http://lists.xensource.com/xen-devel
> > 
> > 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel