Re: [Xen-devel] Re: 2.6.39 merge window (git pulls and what is planned to go in)

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Wed, 2011-03-16 at 15:23 +0000, Shriram Rajagopalan wrote:
> On 2011-03-16, at 2:40 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx> wrote:
> 
> > On Tue, 2011-03-15 at 22:16 +0000, Konrad Rzeszutek Wilk wrote:
> > 
> >>> If Rafael is happy with that plan then fine, but I didn't see him ack it
> >>> in that thread (AFAICT he only acked the concept of what the patch would
> >>> do). Either way someone still needs to follow up with him to get an Ack
> >>> on the 4/5 patch since it is to the PM core, if he's subsequently also
> >> 
> >> Yup. Please do ping him for his ACK. He needs to OK
> >> 
> >>      PM: Add visible HIBERNATION_INTERFACE and hide HIBERNATION
> >> 
> >> 
> >> patch. 
> > 
> > So I was chewing on this last night and I don't see why 2/5 "xen: use
> > freeze/restore/thaw PM events for suspend/resume/chkpt" needs to be
> > blocked by these core Kconfig changes.
> > 
> > The patch makes the Kconfig breakage different but it doesn't make it
> > any worse -- it just exchanges a false(/implicit?) dependency on
> > CONFIG_PM_SLEEP for one on CONFIG_HIBERNATE instead.
> > 
> > I suspect the change from CONFIG_PM_SLEEP->CONFIG_XEN_SAVE_RESTORE in
> > 2/5 could be done as CONFIG_PM_SLEEP->CONFIG_HIBERNATE for now and only
> > switch to CONFIG_XEN_SAVE_RESTORE when the Kconfig stuff is sorted in
> > 5/5.
> > 

> Why the change? The patch 2/5 currently currently makes the code block
> depend on the configuration that it actually corresponds to.

Well, apart from incorrectly mixing two fixes in one patch it just
fuzzes the issue and makes it harder to say that 2/5 is independently
OK.

5/5 is all about cleaning up the Kconfig stuff so lets leave it until
then and then there can be no argument about the correctness or
appropriateness of what remains 2/5 since only the obvious bugfix
remains.

> > So IMHO:
> > 
> > 1/5 xen: xenbus PM events support
> > 
> >        Can go in now via a Xen tree. No brainer.
> > 
> > 2/5 xen: use freeze/restore/thaw PM events for suspend/resume/chkpt
> > 
> >        Can go in now via a Xen tree. Contains a real bugfix and doesn't
> >        make the Kconfig situation any worse. Should probably have
> >        s/CONFIG_XEN_SAVE_RESTORE/CONFIG_HIBERNATION/ done to it for the
> >        time being (see 5/5).
> > 
> > 3/5 PM: pm.h - Add comments about Xen save/restore/chkpt use case
> > 
> >        Can go in via the PM tree at leisure.
> > 
> > 4/5 PM: Add visible HIBERNATION_INTERFACE and hide HIBERNATION
> > 
> >        Should go via PM tree once Rafael is happy with it.
> > 
> > 5/5 xen: fix XEN_SAVE_RESTORE Kconfig dependencies
> > 
> >        Needs to follow both 2/5 and 4/5.

> Nope. I think it still can go in as is. All it does is to select
> HIBERNATION (not clean, unless 4 exists but not buggy either). See
> below for reasoning.

It's not allowed to "select" a user configurable option so I'm afraid it
is buggy. You cannot select HIBERNATION as it is, which is why it needs
the 4/5 refactoring.

> > Can most likely go via either
> >        tree with cross-maintainer agreement. Probably best via PM tree
> >        due to dependency on 4/5 since 2/5 can go in independently
> >        before then.
> > 
> >        Should incorporate the bit of 2/5 undone via the
> >        s/CONFIG_XEN_SAVE_RESTORE/CONFIG_HIBERNATION/
> > 
> > Basically 1/5..2/5 and 3/5..5/5 are two separate series and 1/5..2/5 can
> > go in now...
> > 
> > Worst case is 2/5 makes it into 2.6.39 and 5/5 doesn't, in which case we
> > just need to remember to ask "Have you enabled CONFIG_HIBERNATE?"
> > instead of "Have you enabled CONFIG_PM_SLEEP?". In reality most kernel
> > configs are distro like and probably have both anyway.
> > 
> Exactly. Which means the current 5/5's (select HIBERNATION) would be a
> dud in most distro kernels. For those who configure by hand, in case
> "select HIBERNATION" fails, xen-save-restore would be disabled and
> things won't work.

It breaks the converse case -- i.e. people who want to disable
HIBERNATION will not be able to and will have no clue why in the
configuration interface. This is why the convention against "select"ing
user visible options exists.

Ian.



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