Re: [PATCH] swsusp: Use platform mode by default
On Wednesday, 17 October 2007 05:44, Qi Yong wrote: > On Wed, Oct 17, 2007 at 10:46:12AM +0800, Qi Yong wrote: > > On 12/05/2007, Linus Torvalds <[EMAIL PROTECTED]> wrote: > > > On Fri, 11 May 2007, Rafael J. Wysocki wrote: [--snip--] > > please apply. > > Signed-off-by: Qi Yong <[EMAIL PROTECTED]> > --- With your patch applied the default for ACPI systems changes from HIBERNATION_PLATFORM to HIBERNATION_SHUTDOWN. However, it has been HIBERNATION_PLATFORM since 2.6.20 and I'd really prefer it to stay this way. If HIBERNATION_PLATFORM doesn't work for you, please do "echo shutdown > /sys/power/disk" before hibernation or, if you use the userland suspend tools, change the s2disk's configuration file to use the "shutdown" mode. > diff --git a/kernel/power/disk.c b/kernel/power/disk.c > index eb72255..95b66ee 100644 > --- a/kernel/power/disk.c > +++ b/kernel/power/disk.c > @@ -61,9 +61,11 @@ void hibernation_set_ops(struct hibernation_ops *ops) > } > mutex_lock(_mutex); > hibernation_ops = ops; > - if (ops) > - hibernation_mode = HIBERNATION_PLATFORM; > - else if (hibernation_mode == HIBERNATION_PLATFORM) > + > + /* > + * Turn off HIBERNATION_PLATFORM if we no longer have any platform ops. > + */ > + if (!ops && hibernation_mode == HIBERNATION_PLATFORM) > hibernation_mode = HIBERNATION_SHUTDOWN; > > mutex_unlock(_mutex); Greetings, Rafael - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
On Wednesday, 17 October 2007 05:44, Qi Yong wrote: On Wed, Oct 17, 2007 at 10:46:12AM +0800, Qi Yong wrote: On 12/05/2007, Linus Torvalds [EMAIL PROTECTED] wrote: On Fri, 11 May 2007, Rafael J. Wysocki wrote: [--snip--] please apply. Signed-off-by: Qi Yong [EMAIL PROTECTED] --- With your patch applied the default for ACPI systems changes from HIBERNATION_PLATFORM to HIBERNATION_SHUTDOWN. However, it has been HIBERNATION_PLATFORM since 2.6.20 and I'd really prefer it to stay this way. If HIBERNATION_PLATFORM doesn't work for you, please do echo shutdown /sys/power/disk before hibernation or, if you use the userland suspend tools, change the s2disk's configuration file to use the shutdown mode. diff --git a/kernel/power/disk.c b/kernel/power/disk.c index eb72255..95b66ee 100644 --- a/kernel/power/disk.c +++ b/kernel/power/disk.c @@ -61,9 +61,11 @@ void hibernation_set_ops(struct hibernation_ops *ops) } mutex_lock(pm_mutex); hibernation_ops = ops; - if (ops) - hibernation_mode = HIBERNATION_PLATFORM; - else if (hibernation_mode == HIBERNATION_PLATFORM) + + /* + * Turn off HIBERNATION_PLATFORM if we no longer have any platform ops. + */ + if (!ops hibernation_mode == HIBERNATION_PLATFORM) hibernation_mode = HIBERNATION_SHUTDOWN; mutex_unlock(pm_mutex); Greetings, Rafael - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
On Wed, Oct 17, 2007 at 10:46:12AM +0800, Qi Yong wrote: > On 12/05/2007, Linus Torvalds <[EMAIL PROTECTED]> wrote: > > > > > > On Fri, 11 May 2007, Rafael J. Wysocki wrote: > > > > > > We're working on fixing the breakage, but currently it's difficult, > > > because > > > none of my testboxes has problems with the 'platform' hibernation and I > > > cannot reproduce the reported issues. > > > > The rule for anything ACPI-related has been: no regressions. > > > > It doesn't matter if something fixes 10 boxes, if it breaks a single one, > > it's going to get reverted. > > > > We had much too much of the "two steps forward, one step back" dance with > > ACPI a few years ago, which is the reason that rule got installed (and > > which is why it's ACPI-only: in some other subsystems we accept the fact > > that sometimes we don't know how to fix some hardware issue, but the new > > situation is at least better than the old one). > > > > I agree that it can be aggravating to know that you can fix a problem for > > some people, but then being limited by the fact that it breaks for others. > > But beign able to *rely* on something that used to work is just too > > important, and with ACPI, you can never make a good judgement of which way > > works better (since it really just depends on some random firmware issues > > that we have zero visibility into). > > > > Also, quite often, it may *seem* like something fixes more boxes than it > > breaks, but it's because people report *breakage* only, and then a few > > months later it turns out that it's exactly the other way around: now it's > > a hundred people who report breakage with the *new* code, and the reason > > people thought it fixed more than it broke was that the people for whom > > the old code worked fine obviously never reported it! > > > > So this is why "a single regression is considered more important than ten > > fixes" - because a single regressionr report tends to actually be just the > > first indication of a lot of people who simply haven't tested the new code > > yet! People for whom the old code is broken are more likely to test new > > things. > > > > So I'd just suggest changing the default back to PM_DISK_SHUTDOWN (but > > leave the "pm_ops->enter" testing in place - ie not reverting the other > > commits in the series). > > > > The patch would look something like this. Coywolf, does this fix it for > > you? > > > > Yes, it fixes my problem. > > (Sorry for this long delayed report. I didn't get the chance to test > and reboot my box.) > > This quick test explains me the problem that we should not set > hibernation_mode to HIBERNATION_PLATFORM if it is not !ops". I will > post a formal patch later. > > diff --git a/kernel/power/disk.c b/kernel/power/disk.c > index eb72255..8e52553 100644 > --- a/kernel/power/disk.c > +++ b/kernel/power/disk.c > @@ -62,7 +62,7 @@ void hibernation_set_ops(struct hibernation_ops *ops) > mutex_lock(_mutex); > hibernation_ops = ops; > if (ops) > - hibernation_mode = HIBERNATION_PLATFORM; > + ; > else if (hibernation_mode == HIBERNATION_PLATFORM) > hibernation_mode = HIBERNATION_SHUTDOWN; please apply. Signed-off-by: Qi Yong <[EMAIL PROTECTED]> --- diff --git a/kernel/power/disk.c b/kernel/power/disk.c index eb72255..95b66ee 100644 --- a/kernel/power/disk.c +++ b/kernel/power/disk.c @@ -61,9 +61,11 @@ void hibernation_set_ops(struct hibernation_ops *ops) } mutex_lock(_mutex); hibernation_ops = ops; - if (ops) - hibernation_mode = HIBERNATION_PLATFORM; - else if (hibernation_mode == HIBERNATION_PLATFORM) + + /* +* Turn off HIBERNATION_PLATFORM if we no longer have any platform ops. +*/ + if (!ops && hibernation_mode == HIBERNATION_PLATFORM) hibernation_mode = HIBERNATION_SHUTDOWN; mutex_unlock(_mutex); > > > Linus > > > > --- > > kernel/power/disk.c |6 +++--- > > 1 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/kernel/power/disk.c b/kernel/power/disk.c > > index b5f0543..f6aa06e 100644 > > --- a/kernel/power/disk.c > > +++ b/kernel/power/disk.c > > @@ -60,9 +60,9 @@ void hibernation_set_ops(struct hibernation_ops *ops) > > } > > mutex_lock(_mutex); > > hibernation_ops = ops; > > - if (ops) > > - hibernation_mode = HIBERNATION_PLATFORM; > > - else if (hibernation_mode == HIBERNATION_PLATFORM) > > + > > + /* Turn off HIBERNATION_PLATFORM if we no longer have any platform > > ops */ > > + if (!ops && hibernation_mode == HIBERNATION_PLATFORM) > > hibernation_mode = HIBERNATION_SHUTDOWN; > > > > mutex_unlock(_mutex); > > > > > -- > Qi Yong > - > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to [EMAIL PROTECTED] > More majordomo info at
Re: [PATCH] swsusp: Use platform mode by default
On Wed, 17 Oct 2007, Qi Yong wrote: > > The key point is "fall back to shutdown _only_ if !ops, otherwise > don't touch hibernation_mode". And that solves my problem. Please, when resurrecting a five-month-old discussion, give more of the old context. I don't know about anybody else, but I get enough email that "five months" might as well be "last century" when it comes to email discussions, and I have long since forgotten the details of whatever caused the issue to begin with ;) Linus - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
On 12/05/2007, Linus Torvalds <[EMAIL PROTECTED]> wrote: > > > On Fri, 11 May 2007, Rafael J. Wysocki wrote: > > > > We're working on fixing the breakage, but currently it's difficult, because > > none of my testboxes has problems with the 'platform' hibernation and I > > cannot reproduce the reported issues. > > The rule for anything ACPI-related has been: no regressions. > > It doesn't matter if something fixes 10 boxes, if it breaks a single one, > it's going to get reverted. > > We had much too much of the "two steps forward, one step back" dance with > ACPI a few years ago, which is the reason that rule got installed (and > which is why it's ACPI-only: in some other subsystems we accept the fact > that sometimes we don't know how to fix some hardware issue, but the new > situation is at least better than the old one). > > I agree that it can be aggravating to know that you can fix a problem for > some people, but then being limited by the fact that it breaks for others. > But beign able to *rely* on something that used to work is just too > important, and with ACPI, you can never make a good judgement of which way > works better (since it really just depends on some random firmware issues > that we have zero visibility into). > > Also, quite often, it may *seem* like something fixes more boxes than it > breaks, but it's because people report *breakage* only, and then a few > months later it turns out that it's exactly the other way around: now it's > a hundred people who report breakage with the *new* code, and the reason > people thought it fixed more than it broke was that the people for whom > the old code worked fine obviously never reported it! > > So this is why "a single regression is considered more important than ten > fixes" - because a single regressionr report tends to actually be just the > first indication of a lot of people who simply haven't tested the new code > yet! People for whom the old code is broken are more likely to test new > things. > > So I'd just suggest changing the default back to PM_DISK_SHUTDOWN (but > leave the "pm_ops->enter" testing in place - ie not reverting the other > commits in the series). > > The patch would look something like this. Coywolf, does this fix it for > you? > Yes, it fixes my problem. (Sorry for this long delayed report. I didn't get the chance to test and reboot my box.) This quick test explains me the problem that we should not set hibernation_mode to HIBERNATION_PLATFORM if it is not !ops". I will post a formal patch later. diff --git a/kernel/power/disk.c b/kernel/power/disk.c index eb72255..8e52553 100644 --- a/kernel/power/disk.c +++ b/kernel/power/disk.c @@ -62,7 +62,7 @@ void hibernation_set_ops(struct hibernation_ops *ops) mutex_lock(_mutex); hibernation_ops = ops; if (ops) - hibernation_mode = HIBERNATION_PLATFORM; + ; else if (hibernation_mode == HIBERNATION_PLATFORM) hibernation_mode = HIBERNATION_SHUTDOWN; > Linus > > --- > kernel/power/disk.c |6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/kernel/power/disk.c b/kernel/power/disk.c > index b5f0543..f6aa06e 100644 > --- a/kernel/power/disk.c > +++ b/kernel/power/disk.c > @@ -60,9 +60,9 @@ void hibernation_set_ops(struct hibernation_ops *ops) > } > mutex_lock(_mutex); > hibernation_ops = ops; > - if (ops) > - hibernation_mode = HIBERNATION_PLATFORM; > - else if (hibernation_mode == HIBERNATION_PLATFORM) > + > + /* Turn off HIBERNATION_PLATFORM if we no longer have any platform > ops */ > + if (!ops && hibernation_mode == HIBERNATION_PLATFORM) > hibernation_mode = HIBERNATION_SHUTDOWN; > > mutex_unlock(_mutex); > -- Qi Yong - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
On 14/05/2007, Stefan Seyfried <[EMAIL PROTECTED]> wrote: > On Fri, May 11, 2007 at 03:51:38PM -0700, Linus Torvalds wrote: > > > > > > On Fri, 11 May 2007, Rafael J. Wysocki wrote: > > > > > > Just to clarify, the change in question isn't new. It was introduced by > > > the > > > commit 9185cfa92507d07ac787bc73d06c4eec7239 before 2.6.20, at Seife's > > > request and with Pavel's acceptance. > > > > Ok, if it's that old, we migt as leave it in. Clearly there weren't many > > regressions, and this isn't a case of other monsters lurking behind a lack > > of testers. > > I pushed for this since on ACPI machines, "platform" is the right thing to do, > and i still think it will only break on machines that have a broken ACPI BIOS. > (Are there machines with broken ACPI BIOS around? ;-) > > Your additional "fall back to shutdown if !(ops)"-fix looks very sane, > however, and is definitely a good idea. The key point is "fall back to shutdown _only_ if !ops, otherwise don't touch hibernation_mode". And that solves my problem. -- Qi Yong - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
On 14/05/2007, Stefan Seyfried [EMAIL PROTECTED] wrote: On Fri, May 11, 2007 at 03:51:38PM -0700, Linus Torvalds wrote: On Fri, 11 May 2007, Rafael J. Wysocki wrote: Just to clarify, the change in question isn't new. It was introduced by the commit 9185cfa92507d07ac787bc73d06c4eec7239 before 2.6.20, at Seife's request and with Pavel's acceptance. Ok, if it's that old, we migt as leave it in. Clearly there weren't many regressions, and this isn't a case of other monsters lurking behind a lack of testers. I pushed for this since on ACPI machines, platform is the right thing to do, and i still think it will only break on machines that have a broken ACPI BIOS. (Are there machines with broken ACPI BIOS around? ;-) Your additional fall back to shutdown if !(ops)-fix looks very sane, however, and is definitely a good idea. The key point is fall back to shutdown _only_ if !ops, otherwise don't touch hibernation_mode. And that solves my problem. -- Qi Yong - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
On 12/05/2007, Linus Torvalds [EMAIL PROTECTED] wrote: On Fri, 11 May 2007, Rafael J. Wysocki wrote: We're working on fixing the breakage, but currently it's difficult, because none of my testboxes has problems with the 'platform' hibernation and I cannot reproduce the reported issues. The rule for anything ACPI-related has been: no regressions. It doesn't matter if something fixes 10 boxes, if it breaks a single one, it's going to get reverted. We had much too much of the two steps forward, one step back dance with ACPI a few years ago, which is the reason that rule got installed (and which is why it's ACPI-only: in some other subsystems we accept the fact that sometimes we don't know how to fix some hardware issue, but the new situation is at least better than the old one). I agree that it can be aggravating to know that you can fix a problem for some people, but then being limited by the fact that it breaks for others. But beign able to *rely* on something that used to work is just too important, and with ACPI, you can never make a good judgement of which way works better (since it really just depends on some random firmware issues that we have zero visibility into). Also, quite often, it may *seem* like something fixes more boxes than it breaks, but it's because people report *breakage* only, and then a few months later it turns out that it's exactly the other way around: now it's a hundred people who report breakage with the *new* code, and the reason people thought it fixed more than it broke was that the people for whom the old code worked fine obviously never reported it! So this is why a single regression is considered more important than ten fixes - because a single regressionr report tends to actually be just the first indication of a lot of people who simply haven't tested the new code yet! People for whom the old code is broken are more likely to test new things. So I'd just suggest changing the default back to PM_DISK_SHUTDOWN (but leave the pm_ops-enter testing in place - ie not reverting the other commits in the series). The patch would look something like this. Coywolf, does this fix it for you? Yes, it fixes my problem. (Sorry for this long delayed report. I didn't get the chance to test and reboot my box.) This quick test explains me the problem that we should not set hibernation_mode to HIBERNATION_PLATFORM if it is not !ops. I will post a formal patch later. diff --git a/kernel/power/disk.c b/kernel/power/disk.c index eb72255..8e52553 100644 --- a/kernel/power/disk.c +++ b/kernel/power/disk.c @@ -62,7 +62,7 @@ void hibernation_set_ops(struct hibernation_ops *ops) mutex_lock(pm_mutex); hibernation_ops = ops; if (ops) - hibernation_mode = HIBERNATION_PLATFORM; + ; else if (hibernation_mode == HIBERNATION_PLATFORM) hibernation_mode = HIBERNATION_SHUTDOWN; Linus --- kernel/power/disk.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/power/disk.c b/kernel/power/disk.c index b5f0543..f6aa06e 100644 --- a/kernel/power/disk.c +++ b/kernel/power/disk.c @@ -60,9 +60,9 @@ void hibernation_set_ops(struct hibernation_ops *ops) } mutex_lock(pm_mutex); hibernation_ops = ops; - if (ops) - hibernation_mode = HIBERNATION_PLATFORM; - else if (hibernation_mode == HIBERNATION_PLATFORM) + + /* Turn off HIBERNATION_PLATFORM if we no longer have any platform ops */ + if (!ops hibernation_mode == HIBERNATION_PLATFORM) hibernation_mode = HIBERNATION_SHUTDOWN; mutex_unlock(pm_mutex); -- Qi Yong - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
On Wed, 17 Oct 2007, Qi Yong wrote: The key point is fall back to shutdown _only_ if !ops, otherwise don't touch hibernation_mode. And that solves my problem. Please, when resurrecting a five-month-old discussion, give more of the old context. I don't know about anybody else, but I get enough email that five months might as well be last century when it comes to email discussions, and I have long since forgotten the details of whatever caused the issue to begin with ;) Linus - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
On Wed, Oct 17, 2007 at 10:46:12AM +0800, Qi Yong wrote: On 12/05/2007, Linus Torvalds [EMAIL PROTECTED] wrote: On Fri, 11 May 2007, Rafael J. Wysocki wrote: We're working on fixing the breakage, but currently it's difficult, because none of my testboxes has problems with the 'platform' hibernation and I cannot reproduce the reported issues. The rule for anything ACPI-related has been: no regressions. It doesn't matter if something fixes 10 boxes, if it breaks a single one, it's going to get reverted. We had much too much of the two steps forward, one step back dance with ACPI a few years ago, which is the reason that rule got installed (and which is why it's ACPI-only: in some other subsystems we accept the fact that sometimes we don't know how to fix some hardware issue, but the new situation is at least better than the old one). I agree that it can be aggravating to know that you can fix a problem for some people, but then being limited by the fact that it breaks for others. But beign able to *rely* on something that used to work is just too important, and with ACPI, you can never make a good judgement of which way works better (since it really just depends on some random firmware issues that we have zero visibility into). Also, quite often, it may *seem* like something fixes more boxes than it breaks, but it's because people report *breakage* only, and then a few months later it turns out that it's exactly the other way around: now it's a hundred people who report breakage with the *new* code, and the reason people thought it fixed more than it broke was that the people for whom the old code worked fine obviously never reported it! So this is why a single regression is considered more important than ten fixes - because a single regressionr report tends to actually be just the first indication of a lot of people who simply haven't tested the new code yet! People for whom the old code is broken are more likely to test new things. So I'd just suggest changing the default back to PM_DISK_SHUTDOWN (but leave the pm_ops-enter testing in place - ie not reverting the other commits in the series). The patch would look something like this. Coywolf, does this fix it for you? Yes, it fixes my problem. (Sorry for this long delayed report. I didn't get the chance to test and reboot my box.) This quick test explains me the problem that we should not set hibernation_mode to HIBERNATION_PLATFORM if it is not !ops. I will post a formal patch later. diff --git a/kernel/power/disk.c b/kernel/power/disk.c index eb72255..8e52553 100644 --- a/kernel/power/disk.c +++ b/kernel/power/disk.c @@ -62,7 +62,7 @@ void hibernation_set_ops(struct hibernation_ops *ops) mutex_lock(pm_mutex); hibernation_ops = ops; if (ops) - hibernation_mode = HIBERNATION_PLATFORM; + ; else if (hibernation_mode == HIBERNATION_PLATFORM) hibernation_mode = HIBERNATION_SHUTDOWN; please apply. Signed-off-by: Qi Yong [EMAIL PROTECTED] --- diff --git a/kernel/power/disk.c b/kernel/power/disk.c index eb72255..95b66ee 100644 --- a/kernel/power/disk.c +++ b/kernel/power/disk.c @@ -61,9 +61,11 @@ void hibernation_set_ops(struct hibernation_ops *ops) } mutex_lock(pm_mutex); hibernation_ops = ops; - if (ops) - hibernation_mode = HIBERNATION_PLATFORM; - else if (hibernation_mode == HIBERNATION_PLATFORM) + + /* +* Turn off HIBERNATION_PLATFORM if we no longer have any platform ops. +*/ + if (!ops hibernation_mode == HIBERNATION_PLATFORM) hibernation_mode = HIBERNATION_SHUTDOWN; mutex_unlock(pm_mutex); Linus --- kernel/power/disk.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/power/disk.c b/kernel/power/disk.c index b5f0543..f6aa06e 100644 --- a/kernel/power/disk.c +++ b/kernel/power/disk.c @@ -60,9 +60,9 @@ void hibernation_set_ops(struct hibernation_ops *ops) } mutex_lock(pm_mutex); hibernation_ops = ops; - if (ops) - hibernation_mode = HIBERNATION_PLATFORM; - else if (hibernation_mode == HIBERNATION_PLATFORM) + + /* Turn off HIBERNATION_PLATFORM if we no longer have any platform ops */ + if (!ops hibernation_mode == HIBERNATION_PLATFORM) hibernation_mode = HIBERNATION_SHUTDOWN; mutex_unlock(pm_mutex); -- Qi Yong - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to
Re: [PATCH] swsusp: Use platform mode by default
On Fri, May 11, 2007 at 03:51:38PM -0700, Linus Torvalds wrote: > > > On Fri, 11 May 2007, Rafael J. Wysocki wrote: > > > > Just to clarify, the change in question isn't new. It was introduced by the > > commit 9185cfa92507d07ac787bc73d06c4eec7239 before 2.6.20, at Seife's > > request and with Pavel's acceptance. > > Ok, if it's that old, we migt as leave it in. Clearly there weren't many > regressions, and this isn't a case of other monsters lurking behind a lack > of testers. I pushed for this since on ACPI machines, "platform" is the right thing to do, and i still think it will only break on machines that have a broken ACPI BIOS. (Are there machines with broken ACPI BIOS around? ;-) Your additional "fall back to shutdown if !(ops)"-fix looks very sane, however, and is definitely a good idea. -- Stefan Seyfried QA / R Team Mobile Devices| "Any ideas, John?" SUSE LINUX Products GmbH, Nürnberg | "Well, surrounding them's out." This footer brought to you by insane German lawmakers: SUSE Linux Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
On Fri, May 11, 2007 at 03:51:38PM -0700, Linus Torvalds wrote: On Fri, 11 May 2007, Rafael J. Wysocki wrote: Just to clarify, the change in question isn't new. It was introduced by the commit 9185cfa92507d07ac787bc73d06c4eec7239 before 2.6.20, at Seife's request and with Pavel's acceptance. Ok, if it's that old, we migt as leave it in. Clearly there weren't many regressions, and this isn't a case of other monsters lurking behind a lack of testers. I pushed for this since on ACPI machines, platform is the right thing to do, and i still think it will only break on machines that have a broken ACPI BIOS. (Are there machines with broken ACPI BIOS around? ;-) Your additional fall back to shutdown if !(ops)-fix looks very sane, however, and is definitely a good idea. -- Stefan Seyfried QA / RD Team Mobile Devices| Any ideas, John? SUSE LINUX Products GmbH, Nürnberg | Well, surrounding them's out. This footer brought to you by insane German lawmakers: SUSE Linux Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
Rafael J. Wysocki wrote: On Friday, 11 May 2007 18:30, Linus Torvalds wrote: On Fri, 11 May 2007, Rafael J. Wysocki wrote: We're working on fixing the breakage, but currently it's difficult, because none of my testboxes has problems with the 'platform' hibernation and I cannot reproduce the reported issues. The rule for anything ACPI-related has been: no regressions. It doesn't matter if something fixes 10 boxes, if it breaks a single one, it's going to get reverted. [Well, I think I should stop explaining decisions that weren't mine. Yet, I feel responsible for patches that I sign-off.] Just to clarify, the change in question isn't new. It was introduced by the commit 9185cfa92507d07ac787bc73d06c4eec7239 before 2.6.20, at Seife's request and with Pavel's acceptance. We had much too much of the "two steps forward, one step back" dance with ACPI a few years ago, which is the reason that rule got installed (and which is why it's ACPI-only: in some other subsystems we accept the fact that sometimes we don't know how to fix some hardware issue, but the new situation is at least better than the old one). I agree that it can be aggravating to know that you can fix a problem for some people, but then being limited by the fact that it breaks for others. But beign able to *rely* on something that used to work is just too important, and with ACPI, you can never make a good judgement of which way works better (since it really just depends on some random firmware issues that we have zero visibility into). Also, quite often, it may *seem* like something fixes more boxes than it breaks, but it's because people report *breakage* only, and then a few months later it turns out that it's exactly the other way around: now it's a hundred people who report breakage with the *new* code, and the reason people thought it fixed more than it broke was that the people for whom the old code worked fine obviously never reported it! So this is why "a single regression is considered more important than ten fixes" - because a single regressionr report tends to actually be just the first indication of a lot of people who simply haven't tested the new code yet! People for whom the old code is broken are more likely to test new things. So I'd just suggest changing the default back to PM_DISK_SHUTDOWN (but leave the "pm_ops->enter" testing in place - ie not reverting the other commits in the series). The series actually preserves the 2.6.20/21 behavior. By defaulting back to PM_DISK_SHUTDOWN, we'll cause some users for whom 2.6.20 and 2.6.21 work to report this change as a regression, so please let me avoid making this decision (I'm not the maintainer of the hibernation code after all). The problem is that we don't know about regressions until somebody reports them and if that happens after two affected kernel releases, what should we do? I think that one of the reasons people (guilty) don't report problems with suspend and hibernate is that it's been a problem on and off and when it breaks people don't bother to chase it, they just don't use it unless it's critical, or they install suspend2. I only suggest that if 'platform' is more correct use that, don't change it again. Then fix platform. -- Bill Davidsen <[EMAIL PROTECTED]> "We have more to fear from the bungling of the incompetent than from the machinations of the wicked." - from Slashdot - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
Rafael J. Wysocki wrote: On Friday, 11 May 2007 18:30, Linus Torvalds wrote: On Fri, 11 May 2007, Rafael J. Wysocki wrote: We're working on fixing the breakage, but currently it's difficult, because none of my testboxes has problems with the 'platform' hibernation and I cannot reproduce the reported issues. The rule for anything ACPI-related has been: no regressions. It doesn't matter if something fixes 10 boxes, if it breaks a single one, it's going to get reverted. [Well, I think I should stop explaining decisions that weren't mine. Yet, I feel responsible for patches that I sign-off.] Just to clarify, the change in question isn't new. It was introduced by the commit 9185cfa92507d07ac787bc73d06c4eec7239 before 2.6.20, at Seife's request and with Pavel's acceptance. We had much too much of the two steps forward, one step back dance with ACPI a few years ago, which is the reason that rule got installed (and which is why it's ACPI-only: in some other subsystems we accept the fact that sometimes we don't know how to fix some hardware issue, but the new situation is at least better than the old one). I agree that it can be aggravating to know that you can fix a problem for some people, but then being limited by the fact that it breaks for others. But beign able to *rely* on something that used to work is just too important, and with ACPI, you can never make a good judgement of which way works better (since it really just depends on some random firmware issues that we have zero visibility into). Also, quite often, it may *seem* like something fixes more boxes than it breaks, but it's because people report *breakage* only, and then a few months later it turns out that it's exactly the other way around: now it's a hundred people who report breakage with the *new* code, and the reason people thought it fixed more than it broke was that the people for whom the old code worked fine obviously never reported it! So this is why a single regression is considered more important than ten fixes - because a single regressionr report tends to actually be just the first indication of a lot of people who simply haven't tested the new code yet! People for whom the old code is broken are more likely to test new things. So I'd just suggest changing the default back to PM_DISK_SHUTDOWN (but leave the pm_ops-enter testing in place - ie not reverting the other commits in the series). The series actually preserves the 2.6.20/21 behavior. By defaulting back to PM_DISK_SHUTDOWN, we'll cause some users for whom 2.6.20 and 2.6.21 work to report this change as a regression, so please let me avoid making this decision (I'm not the maintainer of the hibernation code after all). The problem is that we don't know about regressions until somebody reports them and if that happens after two affected kernel releases, what should we do? I think that one of the reasons people (guilty) don't report problems with suspend and hibernate is that it's been a problem on and off and when it breaks people don't bother to chase it, they just don't use it unless it's critical, or they install suspend2. I only suggest that if 'platform' is more correct use that, don't change it again. Then fix platform. -- Bill Davidsen [EMAIL PROTECTED] We have more to fear from the bungling of the incompetent than from the machinations of the wicked. - from Slashdot - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
I agree that we should keep the "platform" default, as it went in 2 releases ago (nearly 6 months) without any reported failures until this one -- and it fixed a longstanding issue documented on many machines. We should debug Qi's failure like any other. We are actually in better shape on this one than others because we already know something that works around it. Qi, Please open a bug report here: http://bugzilla.kernel.org/enter_bug.cgi?product=ACPI in the Power-Off category. There are some other open poweroff bugs and maybe we'll find a common thread. Please attach the output from acpidump and dmesg -s64000. thanks, -Len - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
Hi! > > Just to clarify, the change in question isn't new. It was introduced by the > > commit 9185cfa92507d07ac787bc73d06c4eec7239 before 2.6.20, at Seife's > > request and with Pavel's acceptance. > > Ok, if it's that old, we migt as leave it in. Clearly there weren't many > regressions, and this isn't a case of other monsters lurking behind a lack > of testers. Ok, so what is the result? "platform" is the correct default, because it is as the spec said. Both were default in recent history, and neither is too horrible. So I'd prefer "platform" to be default, as it is correct. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
On Fri, 11 May 2007, Rafael J. Wysocki wrote: > > Just to clarify, the change in question isn't new. It was introduced by the > commit 9185cfa92507d07ac787bc73d06c4eec7239 before 2.6.20, at Seife's > request and with Pavel's acceptance. Ok, if it's that old, we migt as leave it in. Clearly there weren't many regressions, and this isn't a case of other monsters lurking behind a lack of testers. Linus - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
On Friday, 11 May 2007 18:30, Linus Torvalds wrote: > > On Fri, 11 May 2007, Rafael J. Wysocki wrote: > > > > We're working on fixing the breakage, but currently it's difficult, because > > none of my testboxes has problems with the 'platform' hibernation and I > > cannot reproduce the reported issues. > > The rule for anything ACPI-related has been: no regressions. > > It doesn't matter if something fixes 10 boxes, if it breaks a single one, > it's going to get reverted. [Well, I think I should stop explaining decisions that weren't mine. Yet, I feel responsible for patches that I sign-off.] Just to clarify, the change in question isn't new. It was introduced by the commit 9185cfa92507d07ac787bc73d06c4eec7239 before 2.6.20, at Seife's request and with Pavel's acceptance. > We had much too much of the "two steps forward, one step back" dance with > ACPI a few years ago, which is the reason that rule got installed (and > which is why it's ACPI-only: in some other subsystems we accept the fact > that sometimes we don't know how to fix some hardware issue, but the new > situation is at least better than the old one). > > I agree that it can be aggravating to know that you can fix a problem for > some people, but then being limited by the fact that it breaks for others. > But beign able to *rely* on something that used to work is just too > important, and with ACPI, you can never make a good judgement of which way > works better (since it really just depends on some random firmware issues > that we have zero visibility into). > > Also, quite often, it may *seem* like something fixes more boxes than it > breaks, but it's because people report *breakage* only, and then a few > months later it turns out that it's exactly the other way around: now it's > a hundred people who report breakage with the *new* code, and the reason > people thought it fixed more than it broke was that the people for whom > the old code worked fine obviously never reported it! > > So this is why "a single regression is considered more important than ten > fixes" - because a single regressionr report tends to actually be just the > first indication of a lot of people who simply haven't tested the new code > yet! People for whom the old code is broken are more likely to test new > things. > > So I'd just suggest changing the default back to PM_DISK_SHUTDOWN (but > leave the "pm_ops->enter" testing in place - ie not reverting the other > commits in the series). The series actually preserves the 2.6.20/21 behavior. By defaulting back to PM_DISK_SHUTDOWN, we'll cause some users for whom 2.6.20 and 2.6.21 work to report this change as a regression, so please let me avoid making this decision (I'm not the maintainer of the hibernation code after all). The problem is that we don't know about regressions until somebody reports them and if that happens after two affected kernel releases, what should we do? Rafael - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
On Fri, 11 May 2007, Rafael J. Wysocki wrote: > > We're working on fixing the breakage, but currently it's difficult, because > none of my testboxes has problems with the 'platform' hibernation and I > cannot reproduce the reported issues. The rule for anything ACPI-related has been: no regressions. It doesn't matter if something fixes 10 boxes, if it breaks a single one, it's going to get reverted. We had much too much of the "two steps forward, one step back" dance with ACPI a few years ago, which is the reason that rule got installed (and which is why it's ACPI-only: in some other subsystems we accept the fact that sometimes we don't know how to fix some hardware issue, but the new situation is at least better than the old one). I agree that it can be aggravating to know that you can fix a problem for some people, but then being limited by the fact that it breaks for others. But beign able to *rely* on something that used to work is just too important, and with ACPI, you can never make a good judgement of which way works better (since it really just depends on some random firmware issues that we have zero visibility into). Also, quite often, it may *seem* like something fixes more boxes than it breaks, but it's because people report *breakage* only, and then a few months later it turns out that it's exactly the other way around: now it's a hundred people who report breakage with the *new* code, and the reason people thought it fixed more than it broke was that the people for whom the old code worked fine obviously never reported it! So this is why "a single regression is considered more important than ten fixes" - because a single regressionr report tends to actually be just the first indication of a lot of people who simply haven't tested the new code yet! People for whom the old code is broken are more likely to test new things. So I'd just suggest changing the default back to PM_DISK_SHUTDOWN (but leave the "pm_ops->enter" testing in place - ie not reverting the other commits in the series). The patch would look something like this. Coywolf, does this fix it for you? Linus --- kernel/power/disk.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/power/disk.c b/kernel/power/disk.c index b5f0543..f6aa06e 100644 --- a/kernel/power/disk.c +++ b/kernel/power/disk.c @@ -60,9 +60,9 @@ void hibernation_set_ops(struct hibernation_ops *ops) } mutex_lock(_mutex); hibernation_ops = ops; - if (ops) - hibernation_mode = HIBERNATION_PLATFORM; - else if (hibernation_mode == HIBERNATION_PLATFORM) + + /* Turn off HIBERNATION_PLATFORM if we no longer have any platform ops */ + if (!ops && hibernation_mode == HIBERNATION_PLATFORM) hibernation_mode = HIBERNATION_SHUTDOWN; mutex_unlock(_mutex); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
On Friday, 11 May 2007 10:36, Coywolf Qi Hunt wrote:
> Hello,
>
> (This patch is merged in 2.6.20 as commit
> 9185cfa92507d07ac787bc73d06c4eec7239)
>
> With this patch, my desktop no longer powers off after hibernate(8).
> It just reboots.
>
> This user land fix can restore the old behavior:
> echo shutdown > /sys/power/disk
>
> The commit causes user land breakage. Is this behavior on purpose?
The change to using the 'platform' mode by default was made because some
notebooks didn't suspend and resume correctly in the 'shutdown' mode.
We're working on fixing the breakage, but currently it's difficult, because
none of my testboxes has problems with the 'platform' hibernation and I
cannot reproduce the reported issues.
BTW, which kernel have you tested?
Greetings,
Rafael
> On 02/11/06, Len Brown <[EMAIL PROTECTED]> wrote:
> > Applied.
> >
> > thanks,
> > -Len
> >
> > On Wednesday 01 November 2006 07:23, Rafael J. Wysocki wrote:
> > > It has been reported that on some systems the functionality after a resume
> > > from disk is limited if the system is simply powered off during the
> > > suspend
> > > instead of using the ACPI S4 suspend (aka platform mode).
> > >
> > > Unfortunately the default is currently to power off the system during the
> > > suspend so the users of these systems experience problems after the resume
> > > if they don't switch to the platform mode explicitly. This patch makes
> > > swsusp
> > > use the platform mode by default to avoid such situations.
> > >
> > > Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
> > > ---
> > > kernel/power/disk.c |8 +---
> > > kernel/power/main.c |2 +-
> > > 2 files changed, 6 insertions(+), 4 deletions(-)
> > >
> > > Index: linux-2.6.19-rc4-mm1/kernel/power/main.c
> > > ===
> > > --- linux-2.6.19-rc4-mm1.orig/kernel/power/main.c
> > > +++ linux-2.6.19-rc4-mm1/kernel/power/main.c
> > > @@ -29,7 +29,7 @@
> > > DECLARE_MUTEX(pm_sem);
> > >
> > > struct pm_ops *pm_ops;
> > > -suspend_disk_method_t pm_disk_mode = PM_DISK_SHUTDOWN;
> > > +suspend_disk_method_t pm_disk_mode = PM_DISK_PLATFORM;
> > >
> > > /**
> > > * pm_set_ops - Set the global power method table.
> > > Index: linux-2.6.19-rc4-mm1/kernel/power/disk.c
> > > ===
> > > --- linux-2.6.19-rc4-mm1.orig/kernel/power/disk.c
> > > +++ linux-2.6.19-rc4-mm1/kernel/power/disk.c
> > > @@ -62,9 +62,11 @@ static void power_down(suspend_disk_meth
> > >
> > > switch(mode) {
> > > case PM_DISK_PLATFORM:
> > > - kernel_shutdown_prepare(SYSTEM_SUSPEND_DISK);
> > > - error = pm_ops->enter(PM_SUSPEND_DISK);
> > > - break;
> > > + if (pm_ops && pm_ops->enter) {
> > > + kernel_shutdown_prepare(SYSTEM_SUSPEND_DISK);
> > > + error = pm_ops->enter(PM_SUSPEND_DISK);
> > > + break;
> > > + }
> > > case PM_DISK_SHUTDOWN:
> > > kernel_power_off();
> > > break;
> > > -
--
If you don't have the time to read,
you don't have the time or the tools to write.
- Stephen King
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
Hello,
(This patch is merged in 2.6.20 as commit
9185cfa92507d07ac787bc73d06c4eec7239)
With this patch, my desktop no longer powers off after hibernate(8).
It just reboots.
This user land fix can restore the old behavior:
echo shutdown > /sys/power/disk
The commit causes user land breakage. Is this behavior on purpose?
thanks,
Qi
On 02/11/06, Len Brown <[EMAIL PROTECTED]> wrote:
Applied.
thanks,
-Len
On Wednesday 01 November 2006 07:23, Rafael J. Wysocki wrote:
> It has been reported that on some systems the functionality after a resume
> from disk is limited if the system is simply powered off during the suspend
> instead of using the ACPI S4 suspend (aka platform mode).
>
> Unfortunately the default is currently to power off the system during the
> suspend so the users of these systems experience problems after the resume
> if they don't switch to the platform mode explicitly. This patch makes swsusp
> use the platform mode by default to avoid such situations.
>
> Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
> ---
> kernel/power/disk.c |8 +---
> kernel/power/main.c |2 +-
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> Index: linux-2.6.19-rc4-mm1/kernel/power/main.c
> ===
> --- linux-2.6.19-rc4-mm1.orig/kernel/power/main.c
> +++ linux-2.6.19-rc4-mm1/kernel/power/main.c
> @@ -29,7 +29,7 @@
> DECLARE_MUTEX(pm_sem);
>
> struct pm_ops *pm_ops;
> -suspend_disk_method_t pm_disk_mode = PM_DISK_SHUTDOWN;
> +suspend_disk_method_t pm_disk_mode = PM_DISK_PLATFORM;
>
> /**
> * pm_set_ops - Set the global power method table.
> Index: linux-2.6.19-rc4-mm1/kernel/power/disk.c
> ===
> --- linux-2.6.19-rc4-mm1.orig/kernel/power/disk.c
> +++ linux-2.6.19-rc4-mm1/kernel/power/disk.c
> @@ -62,9 +62,11 @@ static void power_down(suspend_disk_meth
>
> switch(mode) {
> case PM_DISK_PLATFORM:
> - kernel_shutdown_prepare(SYSTEM_SUSPEND_DISK);
> - error = pm_ops->enter(PM_SUSPEND_DISK);
> - break;
> + if (pm_ops && pm_ops->enter) {
> + kernel_shutdown_prepare(SYSTEM_SUSPEND_DISK);
> + error = pm_ops->enter(PM_SUSPEND_DISK);
> + break;
> + }
> case PM_DISK_SHUTDOWN:
> kernel_power_off();
> break;
> -
--
Qi Yong
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
Hello,
(This patch is merged in 2.6.20 as commit
9185cfa92507d07ac787bc73d06c4eec7239)
With this patch, my desktop no longer powers off after hibernate(8).
It just reboots.
This user land fix can restore the old behavior:
echo shutdown /sys/power/disk
The commit causes user land breakage. Is this behavior on purpose?
thanks,
Qi
On 02/11/06, Len Brown [EMAIL PROTECTED] wrote:
Applied.
thanks,
-Len
On Wednesday 01 November 2006 07:23, Rafael J. Wysocki wrote:
It has been reported that on some systems the functionality after a resume
from disk is limited if the system is simply powered off during the suspend
instead of using the ACPI S4 suspend (aka platform mode).
Unfortunately the default is currently to power off the system during the
suspend so the users of these systems experience problems after the resume
if they don't switch to the platform mode explicitly. This patch makes swsusp
use the platform mode by default to avoid such situations.
Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED]
---
kernel/power/disk.c |8 +---
kernel/power/main.c |2 +-
2 files changed, 6 insertions(+), 4 deletions(-)
Index: linux-2.6.19-rc4-mm1/kernel/power/main.c
===
--- linux-2.6.19-rc4-mm1.orig/kernel/power/main.c
+++ linux-2.6.19-rc4-mm1/kernel/power/main.c
@@ -29,7 +29,7 @@
DECLARE_MUTEX(pm_sem);
struct pm_ops *pm_ops;
-suspend_disk_method_t pm_disk_mode = PM_DISK_SHUTDOWN;
+suspend_disk_method_t pm_disk_mode = PM_DISK_PLATFORM;
/**
* pm_set_ops - Set the global power method table.
Index: linux-2.6.19-rc4-mm1/kernel/power/disk.c
===
--- linux-2.6.19-rc4-mm1.orig/kernel/power/disk.c
+++ linux-2.6.19-rc4-mm1/kernel/power/disk.c
@@ -62,9 +62,11 @@ static void power_down(suspend_disk_meth
switch(mode) {
case PM_DISK_PLATFORM:
- kernel_shutdown_prepare(SYSTEM_SUSPEND_DISK);
- error = pm_ops-enter(PM_SUSPEND_DISK);
- break;
+ if (pm_ops pm_ops-enter) {
+ kernel_shutdown_prepare(SYSTEM_SUSPEND_DISK);
+ error = pm_ops-enter(PM_SUSPEND_DISK);
+ break;
+ }
case PM_DISK_SHUTDOWN:
kernel_power_off();
break;
-
--
Qi Yong
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
On Friday, 11 May 2007 10:36, Coywolf Qi Hunt wrote:
Hello,
(This patch is merged in 2.6.20 as commit
9185cfa92507d07ac787bc73d06c4eec7239)
With this patch, my desktop no longer powers off after hibernate(8).
It just reboots.
This user land fix can restore the old behavior:
echo shutdown /sys/power/disk
The commit causes user land breakage. Is this behavior on purpose?
The change to using the 'platform' mode by default was made because some
notebooks didn't suspend and resume correctly in the 'shutdown' mode.
We're working on fixing the breakage, but currently it's difficult, because
none of my testboxes has problems with the 'platform' hibernation and I
cannot reproduce the reported issues.
BTW, which kernel have you tested?
Greetings,
Rafael
On 02/11/06, Len Brown [EMAIL PROTECTED] wrote:
Applied.
thanks,
-Len
On Wednesday 01 November 2006 07:23, Rafael J. Wysocki wrote:
It has been reported that on some systems the functionality after a resume
from disk is limited if the system is simply powered off during the
suspend
instead of using the ACPI S4 suspend (aka platform mode).
Unfortunately the default is currently to power off the system during the
suspend so the users of these systems experience problems after the resume
if they don't switch to the platform mode explicitly. This patch makes
swsusp
use the platform mode by default to avoid such situations.
Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED]
---
kernel/power/disk.c |8 +---
kernel/power/main.c |2 +-
2 files changed, 6 insertions(+), 4 deletions(-)
Index: linux-2.6.19-rc4-mm1/kernel/power/main.c
===
--- linux-2.6.19-rc4-mm1.orig/kernel/power/main.c
+++ linux-2.6.19-rc4-mm1/kernel/power/main.c
@@ -29,7 +29,7 @@
DECLARE_MUTEX(pm_sem);
struct pm_ops *pm_ops;
-suspend_disk_method_t pm_disk_mode = PM_DISK_SHUTDOWN;
+suspend_disk_method_t pm_disk_mode = PM_DISK_PLATFORM;
/**
* pm_set_ops - Set the global power method table.
Index: linux-2.6.19-rc4-mm1/kernel/power/disk.c
===
--- linux-2.6.19-rc4-mm1.orig/kernel/power/disk.c
+++ linux-2.6.19-rc4-mm1/kernel/power/disk.c
@@ -62,9 +62,11 @@ static void power_down(suspend_disk_meth
switch(mode) {
case PM_DISK_PLATFORM:
- kernel_shutdown_prepare(SYSTEM_SUSPEND_DISK);
- error = pm_ops-enter(PM_SUSPEND_DISK);
- break;
+ if (pm_ops pm_ops-enter) {
+ kernel_shutdown_prepare(SYSTEM_SUSPEND_DISK);
+ error = pm_ops-enter(PM_SUSPEND_DISK);
+ break;
+ }
case PM_DISK_SHUTDOWN:
kernel_power_off();
break;
-
--
If you don't have the time to read,
you don't have the time or the tools to write.
- Stephen King
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
On Fri, 11 May 2007, Rafael J. Wysocki wrote: We're working on fixing the breakage, but currently it's difficult, because none of my testboxes has problems with the 'platform' hibernation and I cannot reproduce the reported issues. The rule for anything ACPI-related has been: no regressions. It doesn't matter if something fixes 10 boxes, if it breaks a single one, it's going to get reverted. We had much too much of the two steps forward, one step back dance with ACPI a few years ago, which is the reason that rule got installed (and which is why it's ACPI-only: in some other subsystems we accept the fact that sometimes we don't know how to fix some hardware issue, but the new situation is at least better than the old one). I agree that it can be aggravating to know that you can fix a problem for some people, but then being limited by the fact that it breaks for others. But beign able to *rely* on something that used to work is just too important, and with ACPI, you can never make a good judgement of which way works better (since it really just depends on some random firmware issues that we have zero visibility into). Also, quite often, it may *seem* like something fixes more boxes than it breaks, but it's because people report *breakage* only, and then a few months later it turns out that it's exactly the other way around: now it's a hundred people who report breakage with the *new* code, and the reason people thought it fixed more than it broke was that the people for whom the old code worked fine obviously never reported it! So this is why a single regression is considered more important than ten fixes - because a single regressionr report tends to actually be just the first indication of a lot of people who simply haven't tested the new code yet! People for whom the old code is broken are more likely to test new things. So I'd just suggest changing the default back to PM_DISK_SHUTDOWN (but leave the pm_ops-enter testing in place - ie not reverting the other commits in the series). The patch would look something like this. Coywolf, does this fix it for you? Linus --- kernel/power/disk.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/power/disk.c b/kernel/power/disk.c index b5f0543..f6aa06e 100644 --- a/kernel/power/disk.c +++ b/kernel/power/disk.c @@ -60,9 +60,9 @@ void hibernation_set_ops(struct hibernation_ops *ops) } mutex_lock(pm_mutex); hibernation_ops = ops; - if (ops) - hibernation_mode = HIBERNATION_PLATFORM; - else if (hibernation_mode == HIBERNATION_PLATFORM) + + /* Turn off HIBERNATION_PLATFORM if we no longer have any platform ops */ + if (!ops hibernation_mode == HIBERNATION_PLATFORM) hibernation_mode = HIBERNATION_SHUTDOWN; mutex_unlock(pm_mutex); - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
On Friday, 11 May 2007 18:30, Linus Torvalds wrote: On Fri, 11 May 2007, Rafael J. Wysocki wrote: We're working on fixing the breakage, but currently it's difficult, because none of my testboxes has problems with the 'platform' hibernation and I cannot reproduce the reported issues. The rule for anything ACPI-related has been: no regressions. It doesn't matter if something fixes 10 boxes, if it breaks a single one, it's going to get reverted. [Well, I think I should stop explaining decisions that weren't mine. Yet, I feel responsible for patches that I sign-off.] Just to clarify, the change in question isn't new. It was introduced by the commit 9185cfa92507d07ac787bc73d06c4eec7239 before 2.6.20, at Seife's request and with Pavel's acceptance. We had much too much of the two steps forward, one step back dance with ACPI a few years ago, which is the reason that rule got installed (and which is why it's ACPI-only: in some other subsystems we accept the fact that sometimes we don't know how to fix some hardware issue, but the new situation is at least better than the old one). I agree that it can be aggravating to know that you can fix a problem for some people, but then being limited by the fact that it breaks for others. But beign able to *rely* on something that used to work is just too important, and with ACPI, you can never make a good judgement of which way works better (since it really just depends on some random firmware issues that we have zero visibility into). Also, quite often, it may *seem* like something fixes more boxes than it breaks, but it's because people report *breakage* only, and then a few months later it turns out that it's exactly the other way around: now it's a hundred people who report breakage with the *new* code, and the reason people thought it fixed more than it broke was that the people for whom the old code worked fine obviously never reported it! So this is why a single regression is considered more important than ten fixes - because a single regressionr report tends to actually be just the first indication of a lot of people who simply haven't tested the new code yet! People for whom the old code is broken are more likely to test new things. So I'd just suggest changing the default back to PM_DISK_SHUTDOWN (but leave the pm_ops-enter testing in place - ie not reverting the other commits in the series). The series actually preserves the 2.6.20/21 behavior. By defaulting back to PM_DISK_SHUTDOWN, we'll cause some users for whom 2.6.20 and 2.6.21 work to report this change as a regression, so please let me avoid making this decision (I'm not the maintainer of the hibernation code after all). The problem is that we don't know about regressions until somebody reports them and if that happens after two affected kernel releases, what should we do? Rafael - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
On Fri, 11 May 2007, Rafael J. Wysocki wrote: Just to clarify, the change in question isn't new. It was introduced by the commit 9185cfa92507d07ac787bc73d06c4eec7239 before 2.6.20, at Seife's request and with Pavel's acceptance. Ok, if it's that old, we migt as leave it in. Clearly there weren't many regressions, and this isn't a case of other monsters lurking behind a lack of testers. Linus - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
Hi! Just to clarify, the change in question isn't new. It was introduced by the commit 9185cfa92507d07ac787bc73d06c4eec7239 before 2.6.20, at Seife's request and with Pavel's acceptance. Ok, if it's that old, we migt as leave it in. Clearly there weren't many regressions, and this isn't a case of other monsters lurking behind a lack of testers. Ok, so what is the result? platform is the correct default, because it is as the spec said. Both were default in recent history, and neither is too horrible. So I'd prefer platform to be default, as it is correct. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] swsusp: Use platform mode by default
I agree that we should keep the platform default, as it went in 2 releases ago (nearly 6 months) without any reported failures until this one -- and it fixed a longstanding issue documented on many machines. We should debug Qi's failure like any other. We are actually in better shape on this one than others because we already know something that works around it. Qi, Please open a bug report here: http://bugzilla.kernel.org/enter_bug.cgi?product=ACPI in the Power-Off category. There are some other open poweroff bugs and maybe we'll find a common thread. Please attach the output from acpidump and dmesg -s64000. thanks, -Len - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
