Re: [PATCH 0/23] reboot-fixes
Hi. On Fri, 2005-08-05 at 07:45, Pavel Machek wrote: > Hi! > > > > > > > Good question. I'm not certain if Pavel intended to add > > > > > > device_suspend(PMSG_FREEZE) to the reboot path. It was > > > > > > there in only one instance. Pavel comments talk only about > > > > > > the suspend path. > > > > > > > > > > Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path. > > > > > > > > Why? > > > > > > Many bioses are broken; if you leave hardware active during reboot, > > > they'll hang during reboot. It is so common problem that I think that > > > only sane solution is make hardware quiet before reboot. > > > > Sorry for my slow reply. > > > > If I remember correctly PMSG_FREEZE was intended solely for stopping > > activity when suspend to disk implementations are about to do their > > Well, I think that PMSG_FREEZE can be handy when we want to stop > activity for other reasons, too... > > > atomic copies. I thought that ide reacts to this message by putting a > > hold on queues, but doesn't otherwise do anything to prepare a drive for > > a restart. If that's true, using FREEZE here isn't going to stop drives > > from doing their emergency shutdown actions. Don't we need PMSG_SUSPEND > > instead? > > Spinning disk down is not neccessary for reboot. Users will be angry > if we do it before reboot... Yes, but I understood (perhaps wrongly) that we were discussing the shutdown path. Nevertheless, for rebooting, you don't want to simply freeze the queue - you want to flush the queue and tell the drive to flush too. For freeze, you may well flush the queue, but you might not necessarily force the drive to flush its queue too. Regards, Nigel -- Evolution. Enumerate the requirements. Consider the interdependencies. Calculate the probabilities. - 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 0/23] reboot-fixes
Hi! > > > > > Good question. I'm not certain if Pavel intended to add > > > > > device_suspend(PMSG_FREEZE) to the reboot path. It was > > > > > there in only one instance. Pavel comments talk only about > > > > > the suspend path. > > > > > > > > Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path. > > > > > > Why? > > > > Many bioses are broken; if you leave hardware active during reboot, > > they'll hang during reboot. It is so common problem that I think that > > only sane solution is make hardware quiet before reboot. > > Sorry for my slow reply. > > If I remember correctly PMSG_FREEZE was intended solely for stopping > activity when suspend to disk implementations are about to do their Well, I think that PMSG_FREEZE can be handy when we want to stop activity for other reasons, too... > atomic copies. I thought that ide reacts to this message by putting a > hold on queues, but doesn't otherwise do anything to prepare a drive for > a restart. If that's true, using FREEZE here isn't going to stop drives > from doing their emergency shutdown actions. Don't we need PMSG_SUSPEND > instead? Spinning disk down is not neccessary for reboot. Users will be angry if we do it before reboot... Pavel -- if you have sharp zaurus hardware you don't need... you know my address - 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 0/23] reboot-fixes
Hi! Good question. I'm not certain if Pavel intended to add device_suspend(PMSG_FREEZE) to the reboot path. It was there in only one instance. Pavel comments talk only about the suspend path. Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path. Why? Many bioses are broken; if you leave hardware active during reboot, they'll hang during reboot. It is so common problem that I think that only sane solution is make hardware quiet before reboot. Sorry for my slow reply. If I remember correctly PMSG_FREEZE was intended solely for stopping activity when suspend to disk implementations are about to do their Well, I think that PMSG_FREEZE can be handy when we want to stop activity for other reasons, too... atomic copies. I thought that ide reacts to this message by putting a hold on queues, but doesn't otherwise do anything to prepare a drive for a restart. If that's true, using FREEZE here isn't going to stop drives from doing their emergency shutdown actions. Don't we need PMSG_SUSPEND instead? Spinning disk down is not neccessary for reboot. Users will be angry if we do it before reboot... Pavel -- if you have sharp zaurus hardware you don't need... you know my address - 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 0/23] reboot-fixes
Hi. On Fri, 2005-08-05 at 07:45, Pavel Machek wrote: Hi! Good question. I'm not certain if Pavel intended to add device_suspend(PMSG_FREEZE) to the reboot path. It was there in only one instance. Pavel comments talk only about the suspend path. Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path. Why? Many bioses are broken; if you leave hardware active during reboot, they'll hang during reboot. It is so common problem that I think that only sane solution is make hardware quiet before reboot. Sorry for my slow reply. If I remember correctly PMSG_FREEZE was intended solely for stopping activity when suspend to disk implementations are about to do their Well, I think that PMSG_FREEZE can be handy when we want to stop activity for other reasons, too... atomic copies. I thought that ide reacts to this message by putting a hold on queues, but doesn't otherwise do anything to prepare a drive for a restart. If that's true, using FREEZE here isn't going to stop drives from doing their emergency shutdown actions. Don't we need PMSG_SUSPEND instead? Spinning disk down is not neccessary for reboot. Users will be angry if we do it before reboot... Yes, but I understood (perhaps wrongly) that we were discussing the shutdown path. Nevertheless, for rebooting, you don't want to simply freeze the queue - you want to flush the queue and tell the drive to flush too. For freeze, you may well flush the queue, but you might not necessarily force the drive to flush its queue too. Regards, Nigel -- Evolution. Enumerate the requirements. Consider the interdependencies. Calculate the probabilities. - 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 0/23] reboot-fixes
Hi. On Thu, 2005-07-28 at 08:54, Pavel Machek wrote: > Hi! > > > > > Good question. I'm not certain if Pavel intended to add > > > > device_suspend(PMSG_FREEZE) to the reboot path. It was > > > > there in only one instance. Pavel comments talk only about > > > > the suspend path. > > > > > > Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path. > > > > Why? > > Many bioses are broken; if you leave hardware active during reboot, > they'll hang during reboot. It is so common problem that I think that > only sane solution is make hardware quiet before reboot. Sorry for my slow reply. If I remember correctly PMSG_FREEZE was intended solely for stopping activity when suspend to disk implementations are about to do their atomic copies. I thought that ide reacts to this message by putting a hold on queues, but doesn't otherwise do anything to prepare a drive for a restart. If that's true, using FREEZE here isn't going to stop drives from doing their emergency shutdown actions. Don't we need PMSG_SUSPEND instead? Regards, Nigel -- Evolution. Enumerate the requirements. Consider the interdependencies. Calculate the probabilities. - 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 0/23] reboot-fixes
Hi. On Thu, 2005-07-28 at 08:54, Pavel Machek wrote: Hi! Good question. I'm not certain if Pavel intended to add device_suspend(PMSG_FREEZE) to the reboot path. It was there in only one instance. Pavel comments talk only about the suspend path. Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path. Why? Many bioses are broken; if you leave hardware active during reboot, they'll hang during reboot. It is so common problem that I think that only sane solution is make hardware quiet before reboot. Sorry for my slow reply. If I remember correctly PMSG_FREEZE was intended solely for stopping activity when suspend to disk implementations are about to do their atomic copies. I thought that ide reacts to this message by putting a hold on queues, but doesn't otherwise do anything to prepare a drive for a restart. If that's true, using FREEZE here isn't going to stop drives from doing their emergency shutdown actions. Don't we need PMSG_SUSPEND instead? Regards, Nigel -- Evolution. Enumerate the requirements. Consider the interdependencies. Calculate the probabilities. - 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 0/23] reboot-fixes
Pavel Machek <[EMAIL PROTECTED]> writes: > I always thought that device_shutdown is different phase -- the one > with interrupts disabled... At the end of device_shutdown interrupts are disabled because we shutdown the interrupt controllers. I don't think we have a phase where the interrupts are disabled, except for the code that runs after device_shutdown, and the bootup code that happens before interrupts are enabled. Eric - 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 0/23] reboot-fixes
Pavel Machek <[EMAIL PROTECTED]> writes: > Hi! > >> So unless you are really ambitious I'd like to take >> device_suspend(PMSG_FREEZE) out of the reboot path for >> 2.6.13, put in -mm where people can bang on it for a bit >> and see that it is coming and delay the merge with the stable >> branch until the bugs with common hardware have been sorted out. > > Works for me. I may feel ambitious, but I don't feel like debugging > every reboot problem or every strange architecture for 2.6.13 :-). Curses. Foiled again! I have failed to pass the buck. Eric - 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 0/23] reboot-fixes
Pavel Machek <[EMAIL PROTECTED]> writes: > I always thought that device_shutdown is different phase -- the one > with interrupts disabled... Nope. device_shutdown runs before interrupts are disabled. Eric - 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 0/23] reboot-fixes
Hi! > So unless you are really ambitious I'd like to take > device_suspend(PMSG_FREEZE) out of the reboot path for > 2.6.13, put in -mm where people can bang on it for a bit > and see that it is coming and delay the merge with the stable > branch until the bugs with common hardware have been sorted out. Works for me. I may feel ambitious, but I don't feel like debugging every reboot problem or every strange architecture for 2.6.13 :-). Pavel -- teflon -- maybe it is a trademark, but it should not be. - 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 0/23] reboot-fixes
Hi! > >> > Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path. > >> > >> Considering how many device drivers that are likely broken, I disagree. > >> Especially since Andrew seems to have trivially found a machine where it > >> doesn't work. > > > > I'm not sure if we want to do that for 2.6.13, but long term, we > > should just tell drivers to FREEZE instead of inventing reboot > > notifier lists and similar uglynesses... > > Then as part of the patch device_shutdown should disappear. It > is silly to have two functions that want to achieve the same > thing. I always thought that device_shutdown is different phase -- the one with interrupts disabled... Pavel -- teflon -- maybe it is a trademark, but it should not be. - 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 0/23] reboot-fixes
Hi! Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path. Considering how many device drivers that are likely broken, I disagree. Especially since Andrew seems to have trivially found a machine where it doesn't work. I'm not sure if we want to do that for 2.6.13, but long term, we should just tell drivers to FREEZE instead of inventing reboot notifier lists and similar uglynesses... Then as part of the patch device_shutdown should disappear. It is silly to have two functions that want to achieve the same thing. I always thought that device_shutdown is different phase -- the one with interrupts disabled... Pavel -- teflon -- maybe it is a trademark, but it should not be. - 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 0/23] reboot-fixes
Hi! So unless you are really ambitious I'd like to take device_suspend(PMSG_FREEZE) out of the reboot path for 2.6.13, put in -mm where people can bang on it for a bit and see that it is coming and delay the merge with the stable branch until the bugs with common hardware have been sorted out. Works for me. I may feel ambitious, but I don't feel like debugging every reboot problem or every strange architecture for 2.6.13 :-). Pavel -- teflon -- maybe it is a trademark, but it should not be. - 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 0/23] reboot-fixes
Pavel Machek [EMAIL PROTECTED] writes: I always thought that device_shutdown is different phase -- the one with interrupts disabled... Nope. device_shutdown runs before interrupts are disabled. Eric - 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 0/23] reboot-fixes
Pavel Machek [EMAIL PROTECTED] writes: Hi! So unless you are really ambitious I'd like to take device_suspend(PMSG_FREEZE) out of the reboot path for 2.6.13, put in -mm where people can bang on it for a bit and see that it is coming and delay the merge with the stable branch until the bugs with common hardware have been sorted out. Works for me. I may feel ambitious, but I don't feel like debugging every reboot problem or every strange architecture for 2.6.13 :-). Curses. Foiled again! I have failed to pass the buck. Eric - 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 0/23] reboot-fixes
Pavel Machek [EMAIL PROTECTED] writes: I always thought that device_shutdown is different phase -- the one with interrupts disabled... At the end of device_shutdown interrupts are disabled because we shutdown the interrupt controllers. I don't think we have a phase where the interrupts are disabled, except for the code that runs after device_shutdown, and the bootup code that happens before interrupts are enabled. Eric - 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 0/23] reboot-fixes
Pavel Machek <[EMAIL PROTECTED]> writes: > Hi! > >> > Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path. >> >> Considering how many device drivers that are likely broken, I disagree. >> Especially since Andrew seems to have trivially found a machine where it >> doesn't work. > > I'm not sure if we want to do that for 2.6.13, but long term, we > should just tell drivers to FREEZE instead of inventing reboot > notifier lists and similar uglynesses... Then as part of the patch device_shutdown should disappear. It is silly to have two functions that want to achieve the same thing. Right now the device driver model is ugly and over complicated in that case and it needs to be simplified so driver writers have a chance of getting it correct. device_suspend(PMSG_FREEZE) feels more reusable than device_shutdown so long term it feels right. But a simple question is why can't you simply use the shutdown method instead of extending the suspend method in the drivers. Eric - 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 0/23] reboot-fixes
Pavel Machek <[EMAIL PROTECTED]> writes: > Hi! > > Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path. > >> My gut feel is the device_suspend calls are the right direction >> as it allows us to remove code from the drivers and possible >> kill device_shutdown completely. >> >> But this close to 2.6.13 I'm not certain what the correct solution >> is. With this we have had issues with both ide and the e1000. >> But those are among the few drivers that do anything in either >> device_shutdown() or the reboot_notifier. > .. >> Looking at it more closely the code is confusing because >> FREEZE and SUSPEND are actually the same message, and in >> addition to what shutdown does they place the device in > > Not in -mm; I was finally able to fix that one. Cool. >> My gut feel is that device_suspend(PMSG_FREEZE) should be >> removed from kernel_restart until is a different message >> from PMSG_SUSPEND at which point it should be equivalent >> to device_shutdown and we can remove that case. > > PMSG_FREEZE != PMSG_SUSPEND in current -mm, but I'm not sure if we can > push that to 2.6.13... Currently device_suspend(PMSG_FREEZE) in the reboot path breaks the e1000 and the ide driver. Which is common enough hardware it effectively breaks reboots in 2.6.13 despite the fact that nearly everything thing else works. To make device_suspend(PMSG_FREEZE) solid in the reboot path I think it would take pushing and stabilizing all of PMSG_FREEZE != PMSG_SUSPEND. It will certainly take a bunch of digging to make certain reboots keep working. Since the number of drivers that implement either a reboot notifier or a shutdown method is small the it is conceivable we could track down all of the serious issues before 2.6.13. However I'm not ready to take point on the bug hunt. So unless you are really ambitious I'd like to take device_suspend(PMSG_FREEZE) out of the reboot path for 2.6.13, put in -mm where people can bang on it for a bit and see that it is coming and delay the merge with the stable branch until the bugs with common hardware have been sorted out. Eric - 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 0/23] reboot-fixes
Pavel Machek <[EMAIL PROTECTED]> wrote: > > > Good question. I'm not certain if Pavel intended to add > > device_suspend(PMSG_FREEZE) to the reboot path. It was > > there in only one instance. Pavel comments talk only about > > the suspend path. > > Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path. Why? - 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 0/23] reboot-fixes
Hi! > > > Good question. I'm not certain if Pavel intended to add > > > device_suspend(PMSG_FREEZE) to the reboot path. It was > > > there in only one instance. Pavel comments talk only about > > > the suspend path. > > > > Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path. > > Why? Many bioses are broken; if you leave hardware active during reboot, they'll hang during reboot. It is so common problem that I think that only sane solution is make hardware quiet before reboot. Pavel -- teflon -- maybe it is a trademark, but it should not be. - 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 0/23] reboot-fixes
Hi! > > Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path. > > Considering how many device drivers that are likely broken, I disagree. > Especially since Andrew seems to have trivially found a machine where it > doesn't work. I'm not sure if we want to do that for 2.6.13, but long term, we should just tell drivers to FREEZE instead of inventing reboot notifier lists and similar uglynesses... Pavel -- teflon -- maybe it is a trademark, but it should not be. - 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 0/23] reboot-fixes
On Thu, 28 Jul 2005, Pavel Machek wrote: > > Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path. Considering how many device drivers that are likely broken, I disagree. Especially since Andrew seems to have trivially found a machine where it doesn't work. 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 0/23] reboot-fixes
Hi!
> >> > My fairly ordinary x86 test box gets stuck during reboot on the
> >> > wait_for_completion() in ide_do_drive_cmd():
> >>
> >> Hmm. The only thing I can think of is someone started adding calls
> >> to device_suspend() before device_shutdown(). Not understanding
> >> where it was a good idea I made certain the calls were in there
> >> consistently.
> >>
> >> Andrew can you remove the call to device_suspend from kernel_restart
> >> and see if this still happens?
> >
> > yup, that fixes it.
> >
> > --- devel/kernel/sys.c~a2005-07-27 10:36:06.0 -0700
> > +++ devel-akpm/kernel/sys.c 2005-07-27 10:36:26.0 -0700
> > @@ -371,7 +371,6 @@ void kernel_restart(char *cmd)
> > {
> > notifier_call_chain(_notifier_list, SYS_RESTART, cmd);
> > system_state = SYSTEM_RESTART;
> > - device_suspend(PMSG_FREEZE);
> > device_shutdown();
> > if (!cmd) {
> > printk(KERN_EMERG "Restarting system.\n");
> > _
> >
> >
> > Presumably it unfixes Pavel's patch?
>
> Good question. I'm not certain if Pavel intended to add
> device_suspend(PMSG_FREEZE) to the reboot path. It was
> there in only one instance. Pavel comments talk only about
> the suspend path.
Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path.
> My gut feel is the device_suspend calls are the right direction
> as it allows us to remove code from the drivers and possible
> kill device_shutdown completely.
>
> But this close to 2.6.13 I'm not certain what the correct solution
> is. With this we have had issues with both ide and the e1000.
> But those are among the few drivers that do anything in either
> device_shutdown() or the reboot_notifier.
..
> Looking at it more closely the code is confusing because
> FREEZE and SUSPEND are actually the same message, and in
> addition to what shutdown does they place the device in
Not in -mm; I was finally able to fix that one.
> My gut feel is that device_suspend(PMSG_FREEZE) should be
> removed from kernel_restart until is a different message
> from PMSG_SUSPEND at which point it should be equivalent
> to device_shutdown and we can remove that case.
PMSG_FREEZE != PMSG_SUSPEND in current -mm, but I'm not sure if we can
push that to 2.6.13...
Pavel
--
teflon -- maybe it is a trademark, but it should not be.
-
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 0/23] reboot-fixes
Andrew Morton <[EMAIL PROTECTED]> writes: > [EMAIL PROTECTED] (Eric W. Biederman) wrote: > > By "fixed" do you mean Tony Luck's patch which I added to rc3-mm2? If so, > do you think that's needed for 2.6.13? Getting patches into e100[0] is a > bit of an exercise :( That is what I was referring to. Eric - 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 0/23] reboot-fixes
[EMAIL PROTECTED] (Eric W. Biederman) wrote:
>
> Andrew Morton <[EMAIL PROTECTED]> writes:
>
> > [EMAIL PROTECTED] (Eric W. Biederman) wrote:
> >>
> >> Andrew Morton <[EMAIL PROTECTED]> writes:
> >>
> >> > My fairly ordinary x86 test box gets stuck during reboot on the
> >> > wait_for_completion() in ide_do_drive_cmd():
> >>
> >> Hmm. The only thing I can think of is someone started adding calls
> >> to device_suspend() before device_shutdown(). Not understanding
> >> where it was a good idea I made certain the calls were in there
> >> consistently.
> >>
> >> Andrew can you remove the call to device_suspend from kernel_restart
> >> and see if this still happens?
> >
> > yup, that fixes it.
> >
> > --- devel/kernel/sys.c~a2005-07-27 10:36:06.0 -0700
> > +++ devel-akpm/kernel/sys.c 2005-07-27 10:36:26.0 -0700
> > @@ -371,7 +371,6 @@ void kernel_restart(char *cmd)
> > {
> > notifier_call_chain(_notifier_list, SYS_RESTART, cmd);
> > system_state = SYSTEM_RESTART;
> > - device_suspend(PMSG_FREEZE);
> > device_shutdown();
> > if (!cmd) {
> > printk(KERN_EMERG "Restarting system.\n");
> > _
> >
> >
> > Presumably it unfixes Pavel's patch?
>
> Good question. I'm not certain if Pavel intended to add
> device_suspend(PMSG_FREEZE) to the reboot path. It was
> there in only one instance. Pavel comments talk only about
> the suspend path.
>
> My gut feel is the device_suspend calls are the right direction
> as it allows us to remove code from the drivers and possible
> kill device_shutdown completely.
>
> But this close to 2.6.13 I'm not certain what the correct solution
> is. With this we have had issues with both ide and the e1000.
> But those are among the few drivers that do anything in either
> device_shutdown() or the reboot_notifier.
>
> The e1000 has been fixed.
By "fixed" do you mean Tony Luck's patch which I added to rc3-mm2? If so,
do you think that's needed for 2.6.13? Getting patches into e100[0] is a
bit of an exercise :(
-
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 0/23] reboot-fixes
[EMAIL PROTECTED] (Eric W. Biederman) writes: > We really need to get a consistent set of requirements > for the device driver authors so they have a chance of > catching up. I meant to say. We really need to get a simple and stable set of requirement for the device driver authors so they have a chance of catching up and implementing it correctly. Eric - 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 0/23] reboot-fixes
Andrew Morton <[EMAIL PROTECTED]> writes:
> [EMAIL PROTECTED] (Eric W. Biederman) wrote:
>>
>> Andrew Morton <[EMAIL PROTECTED]> writes:
>>
>> > My fairly ordinary x86 test box gets stuck during reboot on the
>> > wait_for_completion() in ide_do_drive_cmd():
>>
>> Hmm. The only thing I can think of is someone started adding calls
>> to device_suspend() before device_shutdown(). Not understanding
>> where it was a good idea I made certain the calls were in there
>> consistently.
>>
>> Andrew can you remove the call to device_suspend from kernel_restart
>> and see if this still happens?
>
> yup, that fixes it.
>
> --- devel/kernel/sys.c~a 2005-07-27 10:36:06.0 -0700
> +++ devel-akpm/kernel/sys.c 2005-07-27 10:36:26.0 -0700
> @@ -371,7 +371,6 @@ void kernel_restart(char *cmd)
> {
> notifier_call_chain(_notifier_list, SYS_RESTART, cmd);
> system_state = SYSTEM_RESTART;
> - device_suspend(PMSG_FREEZE);
> device_shutdown();
> if (!cmd) {
> printk(KERN_EMERG "Restarting system.\n");
> _
>
>
> Presumably it unfixes Pavel's patch?
Good question. I'm not certain if Pavel intended to add
device_suspend(PMSG_FREEZE) to the reboot path. It was
there in only one instance. Pavel comments talk only about
the suspend path.
My gut feel is the device_suspend calls are the right direction
as it allows us to remove code from the drivers and possible
kill device_shutdown completely.
But this close to 2.6.13 I'm not certain what the correct solution
is. With this we have had issues with both ide and the e1000.
But those are among the few drivers that do anything in either
device_shutdown() or the reboot_notifier.
The e1000 has been fixed. Can we fix the ide driver?
Looking at it more closely the code is confusing because
FREEZE and SUSPEND are actually the same message, and in
addition to what shutdown does they place the device in
a low power state. That worries me because last I checked
the e1000 driver could not bring itself out of the low power
state.
Are the semantic differences to great to make this interesting?
Is this additional suspend what is causing some of the oddball
power off bug reports?
My gut feel is that device_suspend(PMSG_FREEZE) should be
removed from kernel_restart until is a different message
from PMSG_SUSPEND at which point it should be equivalent
to device_shutdown and we can remove that case.
We really need to get a consistent set of requirements
for the device driver authors so they have a chance of
catching up.
Eric
-
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 0/23] reboot-fixes
[EMAIL PROTECTED] (Eric W. Biederman) wrote:
>
> Andrew Morton <[EMAIL PROTECTED]> writes:
>
> > My fairly ordinary x86 test box gets stuck during reboot on the
> > wait_for_completion() in ide_do_drive_cmd():
>
> Hmm. The only thing I can think of is someone started adding calls
> to device_suspend() before device_shutdown(). Not understanding
> where it was a good idea I made certain the calls were in there
> consistently.
>
> Andrew can you remove the call to device_suspend from kernel_restart
> and see if this still happens?
yup, that fixes it.
--- devel/kernel/sys.c~a2005-07-27 10:36:06.0 -0700
+++ devel-akpm/kernel/sys.c 2005-07-27 10:36:26.0 -0700
@@ -371,7 +371,6 @@ void kernel_restart(char *cmd)
{
notifier_call_chain(_notifier_list, SYS_RESTART, cmd);
system_state = SYSTEM_RESTART;
- device_suspend(PMSG_FREEZE);
device_shutdown();
if (!cmd) {
printk(KERN_EMERG "Restarting system.\n");
_
Presumably it unfixes Pavel's patch?
From: Pavel Machek <[EMAIL PROTECTED]>
Without this patch, Linux provokes emergency disk shutdowns and
similar nastiness. It was in SuSE kernels for some time, IIRC.
Signed-off-by: Pavel Machek <[EMAIL PROTECTED]>
Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
---
include/linux/pm.h | 33 +
kernel/sys.c |3 +++
2 files changed, 24 insertions(+), 12 deletions(-)
diff -puN kernel/sys.c~properly-stop-devices-before-poweroff kernel/sys.c
--- 25/kernel/sys.c~properly-stop-devices-before-poweroff 2005-06-25
14:47:00.0 -0700
+++ 25-akpm/kernel/sys.c2005-06-25 14:50:53.0 -0700
@@ -405,6 +405,7 @@ asmlinkage long sys_reboot(int magic1, i
case LINUX_REBOOT_CMD_HALT:
notifier_call_chain(_notifier_list, SYS_HALT, NULL);
system_state = SYSTEM_HALT;
+ device_suspend(PMSG_SUSPEND);
device_shutdown();
printk(KERN_EMERG "System halted.\n");
machine_halt();
@@ -415,6 +416,7 @@ asmlinkage long sys_reboot(int magic1, i
case LINUX_REBOOT_CMD_POWER_OFF:
notifier_call_chain(_notifier_list, SYS_POWER_OFF, NULL);
system_state = SYSTEM_POWER_OFF;
+ device_suspend(PMSG_SUSPEND);
device_shutdown();
printk(KERN_EMERG "Power down.\n");
machine_power_off();
@@ -431,6 +433,7 @@ asmlinkage long sys_reboot(int magic1, i
notifier_call_chain(_notifier_list, SYS_RESTART, buffer);
system_state = SYSTEM_RESTART;
+ device_suspend(PMSG_FREEZE);
device_shutdown();
printk(KERN_EMERG "Restarting system with command '%s'.\n",
buffer);
machine_restart(buffer);
diff -puN include/linux/pm.h~properly-stop-devices-before-poweroff
include/linux/pm.h
--- 25/include/linux/pm.h~properly-stop-devices-before-poweroff 2005-06-25
14:47:00.0 -0700
+++ 25-akpm/include/linux/pm.h 2005-06-25 14:47:00.0 -0700
@@ -103,7 +103,8 @@ extern int pm_active;
/*
* Register a device with power management
*/
-struct pm_dev __deprecated *pm_register(pm_dev_t type, unsigned long id,
pm_callback callback);
+struct pm_dev __deprecated *
+pm_register(pm_dev_t type, unsigned long id, pm_callback callback);
/*
* Unregister a device with power management
@@ -190,17 +191,18 @@ typedef u32 __bitwise pm_message_t;
/*
* There are 4 important states driver can be in:
* ON -- driver is working
- * FREEZE -- stop operations and apply whatever policy is applicable to a
suspended driver
- * of that class, freeze queues for block like IDE does, drop
packets for
- * ethernet, etc... stop DMA engine too etc... so a consistent image
can be
- * saved; but do not power any hardware down.
- * SUSPEND - like FREEZE, but hardware is doing as much powersaving as
possible. Roughly
- * pci D3.
+ * FREEZE -- stop operations and apply whatever policy is applicable to a
+ * suspended driver of that class, freeze queues for block like IDE
+ * does, drop packets for ethernet, etc... stop DMA engine too etc...
+ * so a consistent image can be saved; but do not power any hardware
+ * down.
+ * SUSPEND - like FREEZE, but hardware is doing as much powersaving as
+ * possible. Roughly pci D3.
*
- * Unfortunately, current drivers only recognize numeric values 0 (ON) and 3
(SUSPEND).
- * We'll need to fix the drivers. So yes, putting 3 to all diferent defines is
intentional,
- * and will go away as soon as drivers are fixed. Also note that typedef is
neccessary,
- * we'll probably want to switch to
+ * Unfortunately, current drivers only recognize numeric values 0 (ON) and 3
+ * (SUSPEND). We'll need to fix the drivers. So yes, putting 3 to all
different
+ * defines is
Re: [PATCH 0/23] reboot-fixes
Andrew Morton <[EMAIL PROTECTED]> writes: > My fairly ordinary x86 test box gets stuck during reboot on the > wait_for_completion() in ide_do_drive_cmd(): Hmm. The only thing I can think of is someone started adding calls to device_suspend() before device_shutdown(). Not understanding where it was a good idea I made certain the calls were in there consistently. Andrew can you remove the call to device_suspend from kernel_restart and see if this still happens? I would suspect interrupts of being disabled but it looks like kgdb is working and I think that requires an interrupt to notice new characters. Eric - 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 0/23] reboot-fixes
[EMAIL PROTECTED] (Eric W. Biederman) writes: > Andrew Morton <[EMAIL PROTECTED]> writes: > >> My fairly ordinary x86 test box gets stuck during reboot on the >> wait_for_completion() in ide_do_drive_cmd(): > > Hmm. The only thing I can think of is someone started adding calls > to device_suspend() before device_shutdown(). Not understanding > where it was a good idea I made certain the calls were in there > consistently. > > Andrew can you remove the call to device_suspend from kernel_restart > and see if this still happens? > > I would suspect interrupts of being disabled but it looks like > kgdb is working and I think that requires an interrupt to notice > new characters. Looking at it the device_suspend calls should be safe but in case we need to follow it up the device_suspend calls in sys_reboot were initially introduced in: commit 620b03276488c3cf103caf1e326bd21f00d3df84 Author: Pavel Machek <[EMAIL PROTECTED]> Date: Sat Jun 25 14:55:11 2005 -0700 [PATCH] properly stop devices before poweroff Without this patch, Linux provokes emergency disk shutdowns and similar nastiness. It was in SuSE kernels for some time, IIRC. Signed-off-by: Pavel Machek <[EMAIL PROTECTED]> Signed-off-by: Andrew Morton <[EMAIL PROTECTED]> Signed-off-by: Linus Torvalds <[EMAIL PROTECTED]> Eric - 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 0/23] reboot-fixes
My fairly ordinary x86 test box gets stuck during reboot on the wait_for_completion() in ide_do_drive_cmd(): (gdb) thread 73 [Switching to thread 73 (Thread 2906)]#0 ide_do_drive_cmd (drive=0xc072dd10, rq=0x0, action=ide_wait) at drivers/ide/ide-io.c:1671 1671rq->waiting = NULL; (gdb) bt #0 ide_do_drive_cmd (drive=0xc072dd10, rq=0x0, action=ide_wait) at drivers/ide/ide-io.c:1671 #1 0xc02e64c0 in ide_diag_taskfile (drive=0xc072dd10, args=0xcc0c1e20, data_size=0, buf=0x0) at drivers/ide/ide-taskfile.c:503 #2 0xc02e64e6 in ide_raw_taskfile (drive=0x0, args=0x0, buf=0x0) at drivers/ide/ide-taskfile.c:508 #3 0xc02eab6d in do_idedisk_flushcache (drive=0x0) at drivers/ide/ide-disk.c:831 #4 0xc02eb232 in ide_cacheflush_p (drive=0xc072dd10) at drivers/ide/ide-disk.c:1027 #5 0xc02eb2e4 in ide_device_shutdown (dev=0xc072ddf4) at drivers/ide/ide-disk.c:1083 #6 0xc02af75c in device_shutdown () at drivers/base/power/shutdown.c:45 #7 0xc0128bfe in kernel_restart (cmd=0x0) at kernel/sys.c:375 #8 0xc0128dea in sys_reboot (magic1=-18751827, magic2=672274793, cmd=19088743, arg=0x0) at kernel/sys.c:484 #9 0xc0102ba3 in sysenter_past_esp () at arch/i386/kernel/semaphore.c:177 - 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 0/23] reboot-fixes
My fairly ordinary x86 test box gets stuck during reboot on the wait_for_completion() in ide_do_drive_cmd(): (gdb) thread 73 [Switching to thread 73 (Thread 2906)]#0 ide_do_drive_cmd (drive=0xc072dd10, rq=0x0, action=ide_wait) at drivers/ide/ide-io.c:1671 1671rq-waiting = NULL; (gdb) bt #0 ide_do_drive_cmd (drive=0xc072dd10, rq=0x0, action=ide_wait) at drivers/ide/ide-io.c:1671 #1 0xc02e64c0 in ide_diag_taskfile (drive=0xc072dd10, args=0xcc0c1e20, data_size=0, buf=0x0) at drivers/ide/ide-taskfile.c:503 #2 0xc02e64e6 in ide_raw_taskfile (drive=0x0, args=0x0, buf=0x0) at drivers/ide/ide-taskfile.c:508 #3 0xc02eab6d in do_idedisk_flushcache (drive=0x0) at drivers/ide/ide-disk.c:831 #4 0xc02eb232 in ide_cacheflush_p (drive=0xc072dd10) at drivers/ide/ide-disk.c:1027 #5 0xc02eb2e4 in ide_device_shutdown (dev=0xc072ddf4) at drivers/ide/ide-disk.c:1083 #6 0xc02af75c in device_shutdown () at drivers/base/power/shutdown.c:45 #7 0xc0128bfe in kernel_restart (cmd=0x0) at kernel/sys.c:375 #8 0xc0128dea in sys_reboot (magic1=-18751827, magic2=672274793, cmd=19088743, arg=0x0) at kernel/sys.c:484 #9 0xc0102ba3 in sysenter_past_esp () at arch/i386/kernel/semaphore.c:177 - 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 0/23] reboot-fixes
[EMAIL PROTECTED] (Eric W. Biederman) writes: Andrew Morton [EMAIL PROTECTED] writes: My fairly ordinary x86 test box gets stuck during reboot on the wait_for_completion() in ide_do_drive_cmd(): Hmm. The only thing I can think of is someone started adding calls to device_suspend() before device_shutdown(). Not understanding where it was a good idea I made certain the calls were in there consistently. Andrew can you remove the call to device_suspend from kernel_restart and see if this still happens? I would suspect interrupts of being disabled but it looks like kgdb is working and I think that requires an interrupt to notice new characters. Looking at it the device_suspend calls should be safe but in case we need to follow it up the device_suspend calls in sys_reboot were initially introduced in: commit 620b03276488c3cf103caf1e326bd21f00d3df84 Author: Pavel Machek [EMAIL PROTECTED] Date: Sat Jun 25 14:55:11 2005 -0700 [PATCH] properly stop devices before poweroff Without this patch, Linux provokes emergency disk shutdowns and similar nastiness. It was in SuSE kernels for some time, IIRC. Signed-off-by: Pavel Machek [EMAIL PROTECTED] Signed-off-by: Andrew Morton [EMAIL PROTECTED] Signed-off-by: Linus Torvalds [EMAIL PROTECTED] Eric - 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 0/23] reboot-fixes
Andrew Morton [EMAIL PROTECTED] writes: My fairly ordinary x86 test box gets stuck during reboot on the wait_for_completion() in ide_do_drive_cmd(): Hmm. The only thing I can think of is someone started adding calls to device_suspend() before device_shutdown(). Not understanding where it was a good idea I made certain the calls were in there consistently. Andrew can you remove the call to device_suspend from kernel_restart and see if this still happens? I would suspect interrupts of being disabled but it looks like kgdb is working and I think that requires an interrupt to notice new characters. Eric - 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 0/23] reboot-fixes
[EMAIL PROTECTED] (Eric W. Biederman) wrote:
Andrew Morton [EMAIL PROTECTED] writes:
My fairly ordinary x86 test box gets stuck during reboot on the
wait_for_completion() in ide_do_drive_cmd():
Hmm. The only thing I can think of is someone started adding calls
to device_suspend() before device_shutdown(). Not understanding
where it was a good idea I made certain the calls were in there
consistently.
Andrew can you remove the call to device_suspend from kernel_restart
and see if this still happens?
yup, that fixes it.
--- devel/kernel/sys.c~a2005-07-27 10:36:06.0 -0700
+++ devel-akpm/kernel/sys.c 2005-07-27 10:36:26.0 -0700
@@ -371,7 +371,6 @@ void kernel_restart(char *cmd)
{
notifier_call_chain(reboot_notifier_list, SYS_RESTART, cmd);
system_state = SYSTEM_RESTART;
- device_suspend(PMSG_FREEZE);
device_shutdown();
if (!cmd) {
printk(KERN_EMERG Restarting system.\n);
_
Presumably it unfixes Pavel's patch?
From: Pavel Machek [EMAIL PROTECTED]
Without this patch, Linux provokes emergency disk shutdowns and
similar nastiness. It was in SuSE kernels for some time, IIRC.
Signed-off-by: Pavel Machek [EMAIL PROTECTED]
Signed-off-by: Andrew Morton [EMAIL PROTECTED]
---
include/linux/pm.h | 33 +
kernel/sys.c |3 +++
2 files changed, 24 insertions(+), 12 deletions(-)
diff -puN kernel/sys.c~properly-stop-devices-before-poweroff kernel/sys.c
--- 25/kernel/sys.c~properly-stop-devices-before-poweroff 2005-06-25
14:47:00.0 -0700
+++ 25-akpm/kernel/sys.c2005-06-25 14:50:53.0 -0700
@@ -405,6 +405,7 @@ asmlinkage long sys_reboot(int magic1, i
case LINUX_REBOOT_CMD_HALT:
notifier_call_chain(reboot_notifier_list, SYS_HALT, NULL);
system_state = SYSTEM_HALT;
+ device_suspend(PMSG_SUSPEND);
device_shutdown();
printk(KERN_EMERG System halted.\n);
machine_halt();
@@ -415,6 +416,7 @@ asmlinkage long sys_reboot(int magic1, i
case LINUX_REBOOT_CMD_POWER_OFF:
notifier_call_chain(reboot_notifier_list, SYS_POWER_OFF, NULL);
system_state = SYSTEM_POWER_OFF;
+ device_suspend(PMSG_SUSPEND);
device_shutdown();
printk(KERN_EMERG Power down.\n);
machine_power_off();
@@ -431,6 +433,7 @@ asmlinkage long sys_reboot(int magic1, i
notifier_call_chain(reboot_notifier_list, SYS_RESTART, buffer);
system_state = SYSTEM_RESTART;
+ device_suspend(PMSG_FREEZE);
device_shutdown();
printk(KERN_EMERG Restarting system with command '%s'.\n,
buffer);
machine_restart(buffer);
diff -puN include/linux/pm.h~properly-stop-devices-before-poweroff
include/linux/pm.h
--- 25/include/linux/pm.h~properly-stop-devices-before-poweroff 2005-06-25
14:47:00.0 -0700
+++ 25-akpm/include/linux/pm.h 2005-06-25 14:47:00.0 -0700
@@ -103,7 +103,8 @@ extern int pm_active;
/*
* Register a device with power management
*/
-struct pm_dev __deprecated *pm_register(pm_dev_t type, unsigned long id,
pm_callback callback);
+struct pm_dev __deprecated *
+pm_register(pm_dev_t type, unsigned long id, pm_callback callback);
/*
* Unregister a device with power management
@@ -190,17 +191,18 @@ typedef u32 __bitwise pm_message_t;
/*
* There are 4 important states driver can be in:
* ON -- driver is working
- * FREEZE -- stop operations and apply whatever policy is applicable to a
suspended driver
- * of that class, freeze queues for block like IDE does, drop
packets for
- * ethernet, etc... stop DMA engine too etc... so a consistent image
can be
- * saved; but do not power any hardware down.
- * SUSPEND - like FREEZE, but hardware is doing as much powersaving as
possible. Roughly
- * pci D3.
+ * FREEZE -- stop operations and apply whatever policy is applicable to a
+ * suspended driver of that class, freeze queues for block like IDE
+ * does, drop packets for ethernet, etc... stop DMA engine too etc...
+ * so a consistent image can be saved; but do not power any hardware
+ * down.
+ * SUSPEND - like FREEZE, but hardware is doing as much powersaving as
+ * possible. Roughly pci D3.
*
- * Unfortunately, current drivers only recognize numeric values 0 (ON) and 3
(SUSPEND).
- * We'll need to fix the drivers. So yes, putting 3 to all diferent defines is
intentional,
- * and will go away as soon as drivers are fixed. Also note that typedef is
neccessary,
- * we'll probably want to switch to
+ * Unfortunately, current drivers only recognize numeric values 0 (ON) and 3
+ * (SUSPEND). We'll need to fix the drivers. So yes, putting 3 to all
different
+ * defines is intentional,
Re: [PATCH 0/23] reboot-fixes
Andrew Morton [EMAIL PROTECTED] writes:
[EMAIL PROTECTED] (Eric W. Biederman) wrote:
Andrew Morton [EMAIL PROTECTED] writes:
My fairly ordinary x86 test box gets stuck during reboot on the
wait_for_completion() in ide_do_drive_cmd():
Hmm. The only thing I can think of is someone started adding calls
to device_suspend() before device_shutdown(). Not understanding
where it was a good idea I made certain the calls were in there
consistently.
Andrew can you remove the call to device_suspend from kernel_restart
and see if this still happens?
yup, that fixes it.
--- devel/kernel/sys.c~a 2005-07-27 10:36:06.0 -0700
+++ devel-akpm/kernel/sys.c 2005-07-27 10:36:26.0 -0700
@@ -371,7 +371,6 @@ void kernel_restart(char *cmd)
{
notifier_call_chain(reboot_notifier_list, SYS_RESTART, cmd);
system_state = SYSTEM_RESTART;
- device_suspend(PMSG_FREEZE);
device_shutdown();
if (!cmd) {
printk(KERN_EMERG Restarting system.\n);
_
Presumably it unfixes Pavel's patch?
Good question. I'm not certain if Pavel intended to add
device_suspend(PMSG_FREEZE) to the reboot path. It was
there in only one instance. Pavel comments talk only about
the suspend path.
My gut feel is the device_suspend calls are the right direction
as it allows us to remove code from the drivers and possible
kill device_shutdown completely.
But this close to 2.6.13 I'm not certain what the correct solution
is. With this we have had issues with both ide and the e1000.
But those are among the few drivers that do anything in either
device_shutdown() or the reboot_notifier.
The e1000 has been fixed. Can we fix the ide driver?
Looking at it more closely the code is confusing because
FREEZE and SUSPEND are actually the same message, and in
addition to what shutdown does they place the device in
a low power state. That worries me because last I checked
the e1000 driver could not bring itself out of the low power
state.
Are the semantic differences to great to make this interesting?
Is this additional suspend what is causing some of the oddball
power off bug reports?
My gut feel is that device_suspend(PMSG_FREEZE) should be
removed from kernel_restart until is a different message
from PMSG_SUSPEND at which point it should be equivalent
to device_shutdown and we can remove that case.
We really need to get a consistent set of requirements
for the device driver authors so they have a chance of
catching up.
Eric
-
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 0/23] reboot-fixes
[EMAIL PROTECTED] (Eric W. Biederman) writes: We really need to get a consistent set of requirements for the device driver authors so they have a chance of catching up. I meant to say. We really need to get a simple and stable set of requirement for the device driver authors so they have a chance of catching up and implementing it correctly. Eric - 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 0/23] reboot-fixes
[EMAIL PROTECTED] (Eric W. Biederman) wrote:
Andrew Morton [EMAIL PROTECTED] writes:
[EMAIL PROTECTED] (Eric W. Biederman) wrote:
Andrew Morton [EMAIL PROTECTED] writes:
My fairly ordinary x86 test box gets stuck during reboot on the
wait_for_completion() in ide_do_drive_cmd():
Hmm. The only thing I can think of is someone started adding calls
to device_suspend() before device_shutdown(). Not understanding
where it was a good idea I made certain the calls were in there
consistently.
Andrew can you remove the call to device_suspend from kernel_restart
and see if this still happens?
yup, that fixes it.
--- devel/kernel/sys.c~a2005-07-27 10:36:06.0 -0700
+++ devel-akpm/kernel/sys.c 2005-07-27 10:36:26.0 -0700
@@ -371,7 +371,6 @@ void kernel_restart(char *cmd)
{
notifier_call_chain(reboot_notifier_list, SYS_RESTART, cmd);
system_state = SYSTEM_RESTART;
- device_suspend(PMSG_FREEZE);
device_shutdown();
if (!cmd) {
printk(KERN_EMERG Restarting system.\n);
_
Presumably it unfixes Pavel's patch?
Good question. I'm not certain if Pavel intended to add
device_suspend(PMSG_FREEZE) to the reboot path. It was
there in only one instance. Pavel comments talk only about
the suspend path.
My gut feel is the device_suspend calls are the right direction
as it allows us to remove code from the drivers and possible
kill device_shutdown completely.
But this close to 2.6.13 I'm not certain what the correct solution
is. With this we have had issues with both ide and the e1000.
But those are among the few drivers that do anything in either
device_shutdown() or the reboot_notifier.
The e1000 has been fixed.
By fixed do you mean Tony Luck's patch which I added to rc3-mm2? If so,
do you think that's needed for 2.6.13? Getting patches into e100[0] is a
bit of an exercise :(
-
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 0/23] reboot-fixes
Andrew Morton [EMAIL PROTECTED] writes: [EMAIL PROTECTED] (Eric W. Biederman) wrote: By fixed do you mean Tony Luck's patch which I added to rc3-mm2? If so, do you think that's needed for 2.6.13? Getting patches into e100[0] is a bit of an exercise :( That is what I was referring to. Eric - 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 0/23] reboot-fixes
Hi!
My fairly ordinary x86 test box gets stuck during reboot on the
wait_for_completion() in ide_do_drive_cmd():
Hmm. The only thing I can think of is someone started adding calls
to device_suspend() before device_shutdown(). Not understanding
where it was a good idea I made certain the calls were in there
consistently.
Andrew can you remove the call to device_suspend from kernel_restart
and see if this still happens?
yup, that fixes it.
--- devel/kernel/sys.c~a2005-07-27 10:36:06.0 -0700
+++ devel-akpm/kernel/sys.c 2005-07-27 10:36:26.0 -0700
@@ -371,7 +371,6 @@ void kernel_restart(char *cmd)
{
notifier_call_chain(reboot_notifier_list, SYS_RESTART, cmd);
system_state = SYSTEM_RESTART;
- device_suspend(PMSG_FREEZE);
device_shutdown();
if (!cmd) {
printk(KERN_EMERG Restarting system.\n);
_
Presumably it unfixes Pavel's patch?
Good question. I'm not certain if Pavel intended to add
device_suspend(PMSG_FREEZE) to the reboot path. It was
there in only one instance. Pavel comments talk only about
the suspend path.
Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path.
My gut feel is the device_suspend calls are the right direction
as it allows us to remove code from the drivers and possible
kill device_shutdown completely.
But this close to 2.6.13 I'm not certain what the correct solution
is. With this we have had issues with both ide and the e1000.
But those are among the few drivers that do anything in either
device_shutdown() or the reboot_notifier.
..
Looking at it more closely the code is confusing because
FREEZE and SUSPEND are actually the same message, and in
addition to what shutdown does they place the device in
Not in -mm; I was finally able to fix that one.
My gut feel is that device_suspend(PMSG_FREEZE) should be
removed from kernel_restart until is a different message
from PMSG_SUSPEND at which point it should be equivalent
to device_shutdown and we can remove that case.
PMSG_FREEZE != PMSG_SUSPEND in current -mm, but I'm not sure if we can
push that to 2.6.13...
Pavel
--
teflon -- maybe it is a trademark, but it should not be.
-
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 0/23] reboot-fixes
On Thu, 28 Jul 2005, Pavel Machek wrote: Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path. Considering how many device drivers that are likely broken, I disagree. Especially since Andrew seems to have trivially found a machine where it doesn't work. 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 0/23] reboot-fixes
Hi! Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path. Considering how many device drivers that are likely broken, I disagree. Especially since Andrew seems to have trivially found a machine where it doesn't work. I'm not sure if we want to do that for 2.6.13, but long term, we should just tell drivers to FREEZE instead of inventing reboot notifier lists and similar uglynesses... Pavel -- teflon -- maybe it is a trademark, but it should not be. - 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 0/23] reboot-fixes
Hi! Good question. I'm not certain if Pavel intended to add device_suspend(PMSG_FREEZE) to the reboot path. It was there in only one instance. Pavel comments talk only about the suspend path. Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path. Why? Many bioses are broken; if you leave hardware active during reboot, they'll hang during reboot. It is so common problem that I think that only sane solution is make hardware quiet before reboot. Pavel -- teflon -- maybe it is a trademark, but it should not be. - 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 0/23] reboot-fixes
Pavel Machek [EMAIL PROTECTED] wrote: Good question. I'm not certain if Pavel intended to add device_suspend(PMSG_FREEZE) to the reboot path. It was there in only one instance. Pavel comments talk only about the suspend path. Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path. Why? - 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 0/23] reboot-fixes
Pavel Machek [EMAIL PROTECTED] writes: Hi! Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path. My gut feel is the device_suspend calls are the right direction as it allows us to remove code from the drivers and possible kill device_shutdown completely. But this close to 2.6.13 I'm not certain what the correct solution is. With this we have had issues with both ide and the e1000. But those are among the few drivers that do anything in either device_shutdown() or the reboot_notifier. .. Looking at it more closely the code is confusing because FREEZE and SUSPEND are actually the same message, and in addition to what shutdown does they place the device in Not in -mm; I was finally able to fix that one. Cool. My gut feel is that device_suspend(PMSG_FREEZE) should be removed from kernel_restart until is a different message from PMSG_SUSPEND at which point it should be equivalent to device_shutdown and we can remove that case. PMSG_FREEZE != PMSG_SUSPEND in current -mm, but I'm not sure if we can push that to 2.6.13... Currently device_suspend(PMSG_FREEZE) in the reboot path breaks the e1000 and the ide driver. Which is common enough hardware it effectively breaks reboots in 2.6.13 despite the fact that nearly everything thing else works. To make device_suspend(PMSG_FREEZE) solid in the reboot path I think it would take pushing and stabilizing all of PMSG_FREEZE != PMSG_SUSPEND. It will certainly take a bunch of digging to make certain reboots keep working. Since the number of drivers that implement either a reboot notifier or a shutdown method is small the it is conceivable we could track down all of the serious issues before 2.6.13. However I'm not ready to take point on the bug hunt. So unless you are really ambitious I'd like to take device_suspend(PMSG_FREEZE) out of the reboot path for 2.6.13, put in -mm where people can bang on it for a bit and see that it is coming and delay the merge with the stable branch until the bugs with common hardware have been sorted out. Eric - 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 0/23] reboot-fixes
Pavel Machek [EMAIL PROTECTED] writes: Hi! Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path. Considering how many device drivers that are likely broken, I disagree. Especially since Andrew seems to have trivially found a machine where it doesn't work. I'm not sure if we want to do that for 2.6.13, but long term, we should just tell drivers to FREEZE instead of inventing reboot notifier lists and similar uglynesses... Then as part of the patch device_shutdown should disappear. It is silly to have two functions that want to achieve the same thing. Right now the device driver model is ugly and over complicated in that case and it needs to be simplified so driver writers have a chance of getting it correct. device_suspend(PMSG_FREEZE) feels more reusable than device_shutdown so long term it feels right. But a simple question is why can't you simply use the shutdown method instead of extending the suspend method in the drivers. Eric - 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 0/23] reboot-fixes
Hi! > The reboot code paths seems to be suffering from 15 years of people > only looking at the code when it breaks. The result is there > are several code paths in which different callers expect different > semantics from the same functions, and a fair amount of imperfect > inline replication of code. > > For a year or more every time I fix one bug in the bug fix reveals yet > another bug. In an attempt to end the cycle of bug fixes revealing > yet more bugs I have generated a series of patches to clean up > the semantics along the reboot path. > > With the callers all agreeing on what to expect from the functions > they call it should at least be possible to kill bugs without > more showing up because of the bug fix. > > My primary approach is to factor sys_reboot into several smaller > functions and provide those functions for the general kernel > consumers instead of the architecture dependent restart and > halt hooks. > > I don't expect this to noticeably fix any bugs along the > main code paths but magic sysrq and several of the more obscure > code paths should work much more reliably. It looks good to me. Good ammount of cruft really accumulated there... Pavel -- teflon -- maybe it is a trademark, but it should not be. - 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/
[PATCH 0/23] reboot-fixes
The reboot code paths seems to be suffering from 15 years of people only looking at the code when it breaks. The result is there are several code paths in which different callers expect different semantics from the same functions, and a fair amount of imperfect inline replication of code. For a year or more every time I fix one bug in the bug fix reveals yet another bug. In an attempt to end the cycle of bug fixes revealing yet more bugs I have generated a series of patches to clean up the semantics along the reboot path. With the callers all agreeing on what to expect from the functions they call it should at least be possible to kill bugs without more showing up because of the bug fix. My primary approach is to factor sys_reboot into several smaller functions and provide those functions for the general kernel consumers instead of the architecture dependent restart and halt hooks. I don't expect this to noticeably fix any bugs along the main code paths but magic sysrq and several of the more obscure code paths should work much more reliably. Eric - 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/
[PATCH 0/23] reboot-fixes
The reboot code paths seems to be suffering from 15 years of people only looking at the code when it breaks. The result is there are several code paths in which different callers expect different semantics from the same functions, and a fair amount of imperfect inline replication of code. For a year or more every time I fix one bug in the bug fix reveals yet another bug. In an attempt to end the cycle of bug fixes revealing yet more bugs I have generated a series of patches to clean up the semantics along the reboot path. With the callers all agreeing on what to expect from the functions they call it should at least be possible to kill bugs without more showing up because of the bug fix. My primary approach is to factor sys_reboot into several smaller functions and provide those functions for the general kernel consumers instead of the architecture dependent restart and halt hooks. I don't expect this to noticeably fix any bugs along the main code paths but magic sysrq and several of the more obscure code paths should work much more reliably. Eric - 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 0/23] reboot-fixes
Hi! The reboot code paths seems to be suffering from 15 years of people only looking at the code when it breaks. The result is there are several code paths in which different callers expect different semantics from the same functions, and a fair amount of imperfect inline replication of code. For a year or more every time I fix one bug in the bug fix reveals yet another bug. In an attempt to end the cycle of bug fixes revealing yet more bugs I have generated a series of patches to clean up the semantics along the reboot path. With the callers all agreeing on what to expect from the functions they call it should at least be possible to kill bugs without more showing up because of the bug fix. My primary approach is to factor sys_reboot into several smaller functions and provide those functions for the general kernel consumers instead of the architecture dependent restart and halt hooks. I don't expect this to noticeably fix any bugs along the main code paths but magic sysrq and several of the more obscure code paths should work much more reliably. It looks good to me. Good ammount of cruft really accumulated there... Pavel -- teflon -- maybe it is a trademark, but it should not be. - 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/
