Patch: linux-2.5.42/kernel/sys.c - warm reboot should not suspend devices
Quoted 6 lines · Eric W. Biederman
Hopefully I am not coming off harsh but I am a little cranky this morning, As before this change I thought things on the device side were pretty much under control they just needed a little stabilization and testing. And now someone possibly me has to go through every driver and write a shutdown method because someone figured calling free was expensive.
I think I've essentially refuted this in parts of previous
messages on this thread that Eric has basically ignored in his responses.
Interested readers should check the previous lkml messages
with this subject line. Of particular relevance to this issue, see my list of three advantages of the 2.4.x approach of not calling unnecessary device-specific shutdown sequences in my message at
http://marc.theaimsgroup.com/?l=linux-kernel&m=103481960911183&w=2,
where I pasted in a response on the same issue that I originally wroted in this thread to Eric Blade (after the paragraph that begins "Changing the interface will reduce coplexity as only the code that needs to be executed will be called."). Also of interest is Richard B. Johnson's example of BIOS reset code that shuts off the PCI bus before touching any RAM at
http://marc.theaimsgroup.com/?l=linux-kernel&m=103462697923792&w=2.
I'm willing to get into this in detail again upon request.
Otherwise, I think it would be a better use of everyone's time not to subject linux-kernel readers to an infinite loop.
Adam J. Richter __ ______________ 575 Oroville Road [email protected] \ / Milpitas, California 95035 +1 408 309-6081 | g g d r a s i l United States of America
"Free Software For The Rest Of Us."
Quoted 11 lines · Adam J. Richter
Quoted 6 lines · Eric W. Biederman
Hopefully I am not coming off harsh but I am a little cranky this morning, As before this change I thought things on the device side were pretty much under control they just needed a little stabilization and testing. And now someone possibly me has to go through every driver and write a shutdown method because someone figured calling free was expensive.
I think I've essentially refuted this in parts of previous
messages on this thread that Eric has basically ignored in his responses.
About free, and the interface I don't much care. Except that I care that it is clean, and that device_shutdown does not get neutered. My current complaint is now that the interface in 2.5.44 changed it is ill specified, and setup in a way so that the code will rarely get tested, and when it is tested it will be tested in a way that is likely to not expose problems.
But judging from my experience with etherboot, and by the bug reports I have coming in there is a lot of work where I need to get drivers to shut themselves down.
There are two cases on reboot.
A) eventually the BIOS toggles the machine Reset line, common but not guaranteed to happen.
B) Whatever we call on reboot does not toggle the reset line.
Quoted 4 lines
Interested readers should check the previous lkml messages
with this subject line. Of particular relevance to this issue, see my list of three advantages of the 2.4.x approach of not calling unnecessary device-specific shutdown sequences in my message at
Breaking it up is fine, but a real pain.
Quoted 5 lines
http://marc.theaimsgroup.com/?l=linux-kernel&m=103481960911183&w=2,
where I pasted in a response on the same issue that I originally wroted in this thread to Eric Blade (after the paragraph that begins "Changing the interface will reduce coplexity as only the code that needs to be executed will be called.").
Not calling the same code in rmmod is wrong because then the is hard to test. Without that the code only gets tested when someone is likely to toggle the reset line on the device anyway, trivially hiding bugs. running rmmod and insmod a bunch of times makes a good test for this kind of thing, and is likely to happen during driver development.
With respect to the embedded case, embedded machines are more likely to see this problem than desktop machines, on an ordinary reboot. But for code bloat I don't have a problem if there is a compile option that totally removes device_shutdown, to save space. So long as it works when it is enabled.
And I do agree that running the bare minimum can increase reliability in some cases as Eric Blade pointed. But I don't want an interface that is optimized for buggy drivers. We have the source so we can fix them.
Quoted 4 lines
Also of interest is Richard
B. Johnson's example of BIOS reset code that shuts off the PCI bus before touching any RAM at
http://marc.theaimsgroup.com/?l=linux-kernel&m=103462697923792&w=2.
Yes the BIOS work around because some OS's don't shut down their devices.
Quoted 3 lines
I'm willing to get into this in detail again upon request.
Otherwise, I think it would be a better use of everyone's time not to subject linux-kernel readers to an infinite loop.
Mostly I want a comment from Patrick Mochel why he made the change, and roughly what he was thinking. So I have a good idea about which code I need to dig into and send patches to fix. If he makes a good case for an independent shutdown, method I am fine with that, just every driver in the kernel needs to change, and that is a heck of a lot of work before 2.6. Otherwise we can go back to calling remove.
Eric
Quoted 7 lines
Mostly I want a comment from Patrick Mochel why he made the change, and roughly what he was thinking. So I have a good idea about which code I need to dig into and send patches to fix. If he makes a good case for an independent shutdown, method I am fine with that, just every driver in the kernel needs to change, and that is a heck of a lot of work before 2.6. Otherwise we can go back to calling remove.
The main problem is locking and refcounting on the device objects. ->remove() is removing objects from the device tree and freeing them. This is not good when we expect the list to remain intact while we iterate over it.
This is fine when a device is unplugged or a module is removed, but completely unnecessary during a power transition. Nothing is going away; we're just turning everything off. And, we don't we don't have to mess with getting the list traversal right, since we can assume it's intact.
In short, it's about the data structures, not the hardware. It is going to require modification to drivers, but the changes should be small and make the code cleaner. It can also happen gradually. There is going to be a lot of cleanup of drivers in the coming months as more things are converted to exploit the driver model, anyway.
In general, I agree with the patch that you sent later in the thread. I'll apply it, at least for now.
-pat
Quoted 12 lines · Patrick Mochel
Quoted 6 lines
Mostly I want a comment from Patrick Mochel why he made the change, and roughly what he was thinking. So I have a good idea about which code I need to dig into and send patches to fix. If he makes a good case for an independent shutdown, method I am fine with that, just every driver in the kernel needs to change, and that is a heck of a lot of work before 2.6. Otherwise we can go back to calling remove.
The main problem is locking and refcounting on the device objects. ->remove() is removing objects from the device tree and freeing them. This is not good when we expect the list to remain intact while we iterate over it.
Quoted 4 lines
This is fine when a device is unplugged or a module is removed, but completely unnecessary during a power transition. Nothing is going away; we're just turning everything off. And, we don't we don't have to mess with getting the list traversal right, since we can assume it's intact.
O.k. That is very good reason for making the change.
Quoted 8 lines
In short, it's about the data structures, not the hardware. It is going to require modification to drivers, but the changes should be small and make the code cleaner. It can also happen gradually. There is going to be a lot of cleanup of drivers in the coming months as more things are converted to exploit the driver model, anyway.
Quoted 1 line
In general, I agree with the patch that you sent later in the thread. I'll apply it, at least for now.
My big concern is with getting the shutdown path setup in a manner that works, and gets testing. When booting linux from linux with sys_kexec a lot of my problems come back to some device driver not getting shutdown.
Question, is there a method from the class shutdown code that we can/should call, during reboot. I just have this memory that for network interfaces simply downing the interface tends to put it in a quiescent state. And I am wondering if that might be a general thing we can take advantage of. Though if the class remove methods modify the data structures I guess that is out.
Eric
Quoted 7 lines
Question, is there a method from the class shutdown code that we can/should call, during reboot. I just have this memory that for network interfaces simply downing the interface tends to put it in a quiescent state. And I am wondering if that might be a general thing we can take advantage of. Though if the class remove methods modify the data structures I guess that is out.
No. Bringing down the network interface, at least, can be done from userspace.
-pat