[PATCH] call drv->shutdown at rmmod

[PATCH] call drv->shutdown at rmmod

[Eric W. Biederman]

At the kexec BOF at OSDL there was some discussion on calling the
device shutdown method at module remove time, in addition to calling
it during reboot.  The driver was the observation that the primary
source of problems in booting linux from linux are drivers with bad
or missing drv->shutdown() routines.  The hope is this will increase
the testing so people can get it right and kexec can become more
useful.  In addition to making normal reboots more reliable.

The following patch is an implementation of that idea it calls drv->shutdown()
before calling drv->remove().  If drv->shutdown() is implemented.

In addition the driver model documentation in 2.6.0-test3 is badly out of
date.  So I have attached a minor correction which at least mentions
drv->shutdown().

Eric


diff -uNr linux-2.6.0-test3/Documentation/driver-model/driver.txt linux-2.6.0-test3-shutdown_before_remove/Documentation/driver-model/driver.txt
--- linux-2.6.0-test3/Documentation/driver-model/driver.txt	Mon Jul 14 03:34:33 2003
+++ linux-2.6.0-test3-shutdown_before_remove/Documentation/driver-model/driver.txt	Wed Aug 13 12:51:49 2003
@@ -16,10 +16,10 @@
         int     (*probe)        (struct device * dev);
         int     (*remove)       (struct device * dev);
 
+	void    (*shutdown)     (struct device * dev);
+
         int     (*suspend)      (struct device * dev, u32 state, u32 level);
         int     (*resume)       (struct device * dev, u32 level);
-
-        void    (*release)      (struct device_driver * drv);
 };
 
 
@@ -194,6 +194,18 @@
 
 If the device is still present, it should quiesce the device and place
 it into a supported low-power state.
+
+
+	void    (*shutdown)     (struct device * dev);
+
+shutdown is called to quiescent a device before a reboot, or before
+the device is removed.  A device is quiescent if all on going
+transactions are stopped, and it is not setup to spontaneously
+generate new ones.  In addition the device should be in a state
+that it is reasonable for the drivers initialization code can get it
+working again.  shutdown is a separate case from remove because on a
+reboot the data structures do not need to be freed, and not freeing
+them increases the robustness of a reboot.
 
 	int	(*suspend)	(struct device * dev, u32 state, u32 level);
 
diff -uNr linux-2.6.0-test3/drivers/base/bus.c linux-2.6.0-test3-shutdown_before_remove/drivers/base/bus.c
--- linux-2.6.0-test3/drivers/base/bus.c	Mon Jul 14 03:31:58 2003
+++ linux-2.6.0-test3-shutdown_before_remove/drivers/base/bus.c	Wed Aug 13 12:52:20 2003
@@ -350,6 +350,8 @@
 	if (drv) {
 		sysfs_remove_link(&drv->kobj,dev->kobj.name);
 		list_del_init(&dev->driver_list);
+		if (drv->shutdown)
+			drv->shutdown(dev);
 		if (drv->remove)
 			drv->remove(dev);
 		dev->driver = NULL;

Re: [PATCH] call drv->shutdown at rmmod

[Christoph Hellwig]

On Thu, Aug 14, 2003 at 01:06:45AM -0600, Eric W. Biederman wrote:
> 
> At the kexec BOF at OSDL there was some discussion on calling the
> device shutdown method at module remove time, in addition to calling
> it during reboot.  The driver was the observation that the primary
> source of problems in booting linux from linux are drivers with bad
> or missing drv->shutdown() routines.  The hope is this will increase
> the testing so people can get it right and kexec can become more
> useful.  In addition to making normal reboots more reliable.
> 
> The following patch is an implementation of that idea it calls drv->shutdown()
> before calling drv->remove().  If drv->shutdown() is implemented.

Sounds really confusing.  And having shutdown maybe called before remove
but not always sounds like a design mistake.

Why do we have shutdown at all?  Can't we just call ->remove on shutdown
so the device always get's into proper unitialized state on shutdown, too?

Re: [PATCH] call drv->shutdown at rmmod

[Russell King]

On Thu, Aug 14, 2003 at 08:54:43AM +0100, Christoph Hellwig wrote:
> Why do we have shutdown at all?  Can't we just call ->remove on shutdown
> so the device always get's into proper unitialized state on shutdown, too?

That's likely to remove the keyboard driver, and some people like
to configure their box so that ctrl-alt-del halts the system, and
a further ctrl-alt-del reboots the system once halted.

There are also some bus drivers which want to know the difference
between "device (driver) was removed" and "device was shutdown",
eg, for setting bus-specific stuff back into a state where the
machine can be soft-rebooted.

With the shutdown callback, drivers get the option whether to
handle this event or not.  Combining it with remove gives them no
option what so ever, and bus drivers loose this knowledge.

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: [PATCH] call drv->shutdown at rmmod

[Eric W. Biederman]

Russell King <rmk@arm.linux.org.uk> writes:

> On Thu, Aug 14, 2003 at 08:54:43AM +0100, Christoph Hellwig wrote:
> > Sounds really confusing.  And having shutdown maybe called before remove
> > but not always sounds like a design mistake.

My patch always called shutdown before remove, but only if the methods
are implemented.  Mandating that shutdown and remove are implemented 
is not something I am changing with this patch.

> > Why do we have shutdown at all?  Can't we just call ->remove on shutdown
> > so the device always get's into proper unitialized state on shutdown, too?
> 
> That's likely to remove the keyboard driver, and some people like
> to configure their box so that ctrl-alt-del halts the system, and
> a further ctrl-alt-del reboots the system once halted.

Hmm.  That is a very weird case.  Semantically the keyboard driver
and everything else should be removed in any case.

After device_shutdown is called it is improper for any driver
to be handling interrupts because the are supposed to be in a quiescent
state.  And if they are not it is likely to break a soft reboot.

> There are also some bus drivers which want to know the difference
> between "device (driver) was removed" and "device was shutdown",
> eg, for setting bus-specific stuff back into a state where the
> machine can be soft-rebooted.
> 
> With the shutdown callback, drivers get the option whether to
> handle this event or not.  Combining it with remove gives them no
> option what so ever, and bus drivers loose this knowledge.

Reasonable.  And they still get that knowledge with the way I implemented
my patch.  They just get to shutdown first.

Russell do you have any objects to always calling shutdown before
remove?   I don't think this looses knowledge and in most cases it should
work, but if there are bus devices were we need to do things significantly
differently I am open to other solutions.

All I really care about is that we get something that is simple enough
that device driver authors can get it right.  And according to my limited
testing we don't have that right now.

Eric

Re: [PATCH] call drv->shutdown at rmmod

[Russell King]

On Thu, Aug 14, 2003 at 09:50:05AM -0600, Eric W. Biederman wrote:
> Russell King <rmk@arm.linux.org.uk> writes:
> > That's likely to remove the keyboard driver, and some people like
> > to configure their box so that ctrl-alt-del halts the system, and
> > a further ctrl-alt-del reboots the system once halted.
> 
> Hmm.  That is a very weird case.  Semantically the keyboard driver
> and everything else should be removed in any case.

I don't view it as "really weird" - it's something I've always done
with 2.2 and 2.4, and in fact the first thing I do when I get a machine
is to modify the inittab to halt the machine on ctrl-alt-del.

It's far safer than hitting ctrl-alt-del and trying to power the machine
off at the exact moment it reboots.

However, sometimes you want it to reboot, and in this case its far
simpler to wait for the machine to halt, and then use ctrl-alt-del
again.  It's something that's been supported by both the kernel and
init for eons.

> After device_shutdown is called it is improper for any driver
> to be handling interrupts because the are supposed to be in a quiescent
> state.  And if they are not it is likely to break a soft reboot.

I guess then this is another bug we need to add to the list of bugs
introduced during 2.5 into 2.6 then...

If it is changing, then someone needs to get that information into
davej's document.

> Russell do you have any objects to always calling shutdown before
> remove?   I don't think this looses knowledge and in most cases it should
> work, but if there are bus devices were we need to do things significantly
> differently I am open to other solutions.

The way I'm treating ->shutdown at present is that it is the final call
to the device driver.  Once this call has been made, the bus driver
puts the bus into the correct state for reboot, and the device driver
must not attempt to access it.

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: [PATCH] call drv->shutdown at rmmod

[Eric W. Biederman]

Russell King <rmk@arm.linux.org.uk> writes:

> On Thu, Aug 14, 2003 at 09:50:05AM -0600, Eric W. Biederman wrote:
> > Russell King <rmk@arm.linux.org.uk> writes:
> > > That's likely to remove the keyboard driver, and some people like
> > > to configure their box so that ctrl-alt-del halts the system, and
> > > a further ctrl-alt-del reboots the system once halted.
> > 
> > Hmm.  That is a very weird case.  Semantically the keyboard driver
> > and everything else should be removed in any case.
> 
> I don't view it as "really weird" - it's something I've always done
> with 2.2 and 2.4, and in fact the first thing I do when I get a machine
> is to modify the inittab to halt the machine on ctrl-alt-del.
>
> It's far safer than hitting ctrl-alt-del and trying to power the machine
> off at the exact moment it reboots.
> 
> However, sometimes you want it to reboot, and in this case its far
> simpler to wait for the machine to halt, and then use ctrl-alt-del
> again.  It's something that's been supported by both the kernel and
> init for eons.

This sounds reasonable.  Something that needs to happen is we need
to distinguish clearly, between the semantics of a halt and reboot.

For a soft reboot to be safe we need to shutdown all of the drivers.

However what does it mean to call halt?  
There are two possibilities. 
-  A frozen in amber state where the kernel just sits in a busy loop,
   and possibly the system is powered off, nothing new can be stared
   but a few remaining things continue on.
-  Everything happens like a reboot except we don't transition to
   any other code.

So to be clear the cases we have to get semantics straight for are.

	case LINUX_REBOOT_CMD_RESTART:
        /* This is a reboot and things are fairly clear */

	case LINUX_REBOOT_CMD_RESTART2:
        /* This is the reboot case and it is fairly clear what needs to
         * happen there.  Of the two this case is biased towards
         * returning to the firmware if there need to be any differences.
         */

	case LINUX_REBOOT_CMD_HALT:
        /* this is the generic halt case and the most confusing.


	case LINUX_REBOOT_CMD_POWER_OFF:
        /* This is the power off case and similarly confusing. */

I think my vote goes for a frozen in amber state where the drivers
do nothing except for the little bit of code in machine_halt or
machine_power_off, which are practically noops, on x86.  And as long
as input is event interrupt driven you can do something with it.

I like it because doing absolutely nothing is very much KISS and easy
to get right.

> > After device_shutdown is called it is improper for any driver
> > to be handling interrupts because the are supposed to be in a quiescent
> > state.  And if they are not it is likely to break a soft reboot.
> 
> I guess then this is another bug we need to add to the list of bugs
> introduced during 2.5 into 2.6 then...

If the kernel should just go into some form of busy loop when halt
is called, I agree that calling device_shutdown on halt is a bug.  
If a halt is just a reboot where we don't do anything afterwards then
2.6 is correct, and the init example is unsupportable.

Possibly what is required is another case of sys_reboot so we can have
both semantics but that feels like over kill.

> If it is changing, then someone needs to get that information into
> davej's document.

I will have to look, I am not up to speed on that one.

> > Russell do you have any objects to always calling shutdown before
> > remove?   I don't think this looses knowledge and in most cases it should
> > work, but if there are bus devices were we need to do things significantly
> > differently I am open to other solutions.
> 
> The way I'm treating ->shutdown at present is that it is the final call
> to the device driver.  Once this call has been made, the bus driver
> puts the bus into the correct state for reboot, and the device driver
> must not attempt to access it.

I agree with that interpretation of ->shutdown.  And that is why
I contend it is wrong to access the keyboard controller after
device_shutdown is called.   Because device_shutdown calls ->shutdown
on every device.  In that case all ->remove could legitimately do in
remove is to free the device data structures.

I think calling ->shutdown, ->remove when a device goes away or when
we remove the module is compatible with the current semantics of
->shutdown.  Although we might need to update a driver or two of the
set that has already implemented a ->shutdown method.  But I don't
think that case is terribly likely to cause problems in practice.

Eric

[PATCH] don't call device_shutdown on halt.

[Eric W. Biederman]

For a halt quiescing devices is overkill, historically wrong, and
error prone when the system is halted.  The only drivers that should
care are drivers for devices that do the wrong thing when power is
removed.

diff -uNr linux-2.6.0-test3/kernel/sys.c linux-2.6.0-test3-no_device_shutdown/kernel/sys.c
--- linux-2.6.0-test3/kernel/sys.c	Tue Jul 29 14:48:17 2003
+++ linux-2.6.0-test3-no_device_shutdown/kernel/sys.c	Sun Aug 17 22:04:18 2003
@@ -423,7 +423,6 @@
 	case LINUX_REBOOT_CMD_HALT:
 		notifier_call_chain(&reboot_notifier_list, SYS_HALT, NULL);
 		system_running = 0;
-		device_shutdown();
 		printk(KERN_EMERG "System halted.\n");
 		machine_halt();
 		unlock_kernel();
@@ -433,7 +432,6 @@
 	case LINUX_REBOOT_CMD_POWER_OFF:
 		notifier_call_chain(&reboot_notifier_list, SYS_POWER_OFF, NULL);
 		system_running = 0;
-		device_shutdown();
 		printk(KERN_EMERG "Power down.\n");
 		machine_power_off();
 		unlock_kernel();

Re: [PATCH] call drv->shutdown at rmmod

[Russell King]

On Thu, Aug 14, 2003 at 09:06:05AM +0100, Russell King wrote:
> That's likely to remove the keyboard driver, and some people like
> to configure their box so that ctrl-alt-del halts the system, and
> a further ctrl-alt-del reboots the system once halted.

Several people have asked me how to set this up, so I'll give the info
here.

- change inittab to call shutdown with -h instead of -r
- telinit Q

I think there is a catch on x86 though - I think shutdown -h powers the
machine down rather than halting it.  The redhat initscripts seem to
need a /halt file to prevent poweroff.  Maybe calling /sbin/halt
instead of shutdown from the initscripts would work better in those
cases...

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: [PATCH] call drv->shutdown at rmmod

[Patrick Mochel]

[ I'm curious as to why you didn't cc me in the first place.. ] 

> At the kexec BOF at OSDL there was some discussion on calling the
> device shutdown method at module remove time, in addition to calling
> it during reboot.  The driver was the observation that the primary
> source of problems in booting linux from linux are drivers with bad
> or missing drv->shutdown() routines.  The hope is this will increase
> the testing so people can get it right and kexec can become more
> useful.  In addition to making normal reboots more reliable.
> 
> The following patch is an implementation of that idea it calls drv->shutdown()
> before calling drv->remove().  If drv->shutdown() is implemented.

I like the idea behind the patch, but we shouldn't be calling it 
unconditionally. We're bound to run into some suprises that could really 
kill us this late in the 2.6 game. 

I do think it should go in, as long as there is a flag telling the core 
whether or not to call shutdown for that particular device. I think it 
could also be extended to include a power state, so the core could suspend 
the device before removing the module. 

The default would always be 'Do Nothing', but with a per-device sysfs 
file, a developer/user/gui app could be used to set the policy from user 
space. 



	-pat

Re: [PATCH] call drv->shutdown at rmmod

[Eric W. Biederman]

Patrick Mochel <mochel@osdl.org> writes:

> [ I'm curious as to why you didn't cc me in the first place.. ] 

My apologies, I meant to.  Somehow I got Pavel Macheck instead of
Patrick Mochel.   The names are only superficially similar but mistakes
happen.

> > At the kexec BOF at OSDL there was some discussion on calling the
> > device shutdown method at module remove time, in addition to calling
> > it during reboot.  The driver was the observation that the primary
> > source of problems in booting linux from linux are drivers with bad
> > or missing drv->shutdown() routines.  The hope is this will increase
> > the testing so people can get it right and kexec can become more
> > useful.  In addition to making normal reboots more reliable.
> > 
> > The following patch is an implementation of that idea it calls drv->shutdown()
> 
> > before calling drv->remove().  If drv->shutdown() is implemented.
> 
> I like the idea behind the patch, but we shouldn't be calling it 
> unconditionally. We're bound to run into some suprises that could really 
> kill us this late in the 2.6 game. 
> 
> I do think it should go in, as long as there is a flag telling the core 
> whether or not to call shutdown for that particular device. I think it 
> could also be extended to include a power state, so the core could suspend 
> the device before removing the module. 

Assuming the device driver can get a device out of the suspend state
when the module is loaded.  This has been one of the biggest problem areas
with the e100 driver.  It's init code cannot bring the device out of a low
power state.

> The default would always be 'Do Nothing', but with a per-device sysfs 
> file, a developer/user/gui app could be used to set the policy from user 
> space. 

Possibly.  But this is getting complicated.  And while I do support things
being complicated enough to handle the problem this part of the API feels
like it is excessively complicated.  Which is why I was trying to simply
it by always calling shutdown on a device before we remove it.

Eric

Re: [PATCH] call drv->shutdown at rmmod

[Patrick Mochel]

> Assuming the device driver can get a device out of the suspend state
> when the module is loaded.  This has been one of the biggest problem areas
> with the e100 driver.  It's init code cannot bring the device out of a low
> power state.

You're assuming that a driver can always bring a device out a shutdown
state. That's one of the things we talked about at OLS, and the other half
of the justification behind such a feature, not just making sure the
device is queisced. Your argument against my suggestion are some of the
same arguments for a feature like you're introducing. 

We have similar goals - introduce device power state code paths into 
common operations (e.g module removal). Doing so gets the code paths 
tested, which will help us ultimately achieve our utopian goals. And, it 
can be done in one shot. 

> > The default would always be 'Do Nothing', but with a per-device sysfs 
> > file, a developer/user/gui app could be used to set the policy from user 
> > space. 
> 
> Possibly.  But this is getting complicated.  And while I do support things
> being complicated enough to handle the problem this part of the API feels
> like it is excessively complicated.  Which is why I was trying to simply
> it by always calling shutdown on a device before we remove it.

Eh? This is complicated? especially compared your overall goal, kexec? Let 
me explain again: 

I won't take the patch to unconditionally shutdown devices on module 
removal, so you need some sort of flag. But, you must have some way to set 
the flag, which is what sysfs is designed for. While you're at it, we 
might as well make it an integer value, rather than a pure binary one, and 
conditionally attempt to set the power state if the user says so. 

It's actually pretty simple, and if it is confusing, then sit back and 
wait, and I'll provide documentation when I submit the patch. 



	Pat

Re: [PATCH] call drv->shutdown at rmmod

[Eric W. Biederman]

Patrick Mochel <mochel@osdl.org> writes:

> > Assuming the device driver can get a device out of the suspend state
> > when the module is loaded.  This has been one of the biggest problem areas
> > with the e100 driver.  It's init code cannot bring the device out of a low
> > power state.
> 
> You're assuming that a driver can always bring a device out a shutdown
> state. 

I am assuming that is a requirement that it is possible for software
to bring a device out of the state is place in by the ->shutdown method.
->shutdown is called on the reboot path and if it results in a pure
software only reboot (i.e. the reset line on the motherboard is not toggled),
then the device driver for the OS loaded after the reboot must reinitialize it.

> That's one of the things we talked about at OLS, and the other half
> of the justification behind such a feature, not just making sure the
> device is queisced. Your argument against my suggestion are some of the
> same arguments for a feature like you're introducing. 

Yes.  My point is that I don't especially like having an additional argument
as that introduces more conditional control flow cases.  My experience
suggests unconditional control flow is more likely to be implemented properly.
 
> We have similar goals - introduce device power state code paths into 
> common operations (e.g module removal). Doing so gets the code paths 
> tested, which will help us ultimately achieve our utopian goals. And, it 
> can be done in one shot. 

If it can be done reasonably I am for it.

> > > The default would always be 'Do Nothing', but with a per-device sysfs 
> > > file, a developer/user/gui app could be used to set the policy from user 
> > > space. 
> > 
> > Possibly.  But this is getting complicated.  And while I do support things
> > being complicated enough to handle the problem this part of the API feels
> > like it is excessively complicated.  Which is why I was trying to simply
> > it by always calling shutdown on a device before we remove it.
> 
> Eh? This is complicated? especially compared your overall goal, kexec? Let 
> me explain again: 

Except that kexec suffers from the same class of issues a cooperative
multitasking (every component must do the right thing), it is quite
simple.  All of my code paths are written assuming they are the slow
uncommon path, so they place the smallest possible burden on the rest
of the kernel.

> I won't take the patch to unconditionally shutdown devices on module 
> removal, so you need some sort of flag. But, you must have some way to set 
> the flag, which is what sysfs is designed for. While you're at it, we 
> might as well make it an integer value, rather than a pure binary one, and 
> conditionally attempt to set the power state if the user says so. 
> 
> It's actually pretty simple, and if it is confusing, then sit back and 
> wait, and I'll provide documentation when I submit the patch. 

I think I now see where you are coming from.  Instead of changing the
production code paths.  Introducing a set of testing code paths to
assist with driver testing.  At least that is the feel I am getting.

Eric

Re: [PATCH] call drv->shutdown at rmmod

[Benjamin Herrenschmidt]

> You're assuming that a driver can always bring a device out a shutdown
> state. That's one of the things we talked about at OLS, and the other half
> of the justification behind such a feature, not just making sure the
> device is queisced. Your argument against my suggestion are some of the
> same arguments for a feature like you're introducing. 

There is a problem of semantics here. Is shutdown() supposed to shutdown
the hardware device (ie. low power) or just the driver ? If yes, then
it's duplicate of the PM callbacks. My understanding of the shutdown()
callback is that it was more than "stop driver activity, put device into
idle state" to prepare for a shutdown/reboot (though we do also sleep
IDE drives in this case, but this is because of that nasty cache flush
issue).

The problem with kexec is just that. What it needs isn't low power devices,
it needs device back in "idle" state, but if possible powered up (or at
least in whatever state the driver found them on boot). The most important
thing is to actually stop pending bus mastering activities.

On PPC, we have a name for that which comes from Open Firmware (since we
need to ask the firmware to stop bus mastering & idle devices the same way
when we take over it and before we get control of the system memory) and
it's called "quiesce".

Ben.

Re: [PATCH] call drv->shutdown at rmmod

[Eric W. Biederman]

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> > You're assuming that a driver can always bring a device out a shutdown
> > state. That's one of the things we talked about at OLS, and the other half
> > of the justification behind such a feature, not just making sure the
> > device is queisced. Your argument against my suggestion are some of the
> > same arguments for a feature like you're introducing. 
> 
> There is a problem of semantics here. Is shutdown() supposed to shutdown
> the hardware device (ie. low power) or just the driver ? If yes, then
> it's duplicate of the PM callbacks. My understanding of the shutdown()
> callback is that it was more than "stop driver activity, put device into
> idle state" to prepare for a shutdown/reboot (though we do also sleep
> IDE drives in this case, but this is because of that nasty cache flush
> issue).
> 
> The problem with kexec is just that. What it needs isn't low power devices,
> it needs device back in "idle" state, but if possible powered up (or at
> least in whatever state the driver found them on boot). The most important
> thing is to actually stop pending bus mastering activities.
> 
> On PPC, we have a name for that which comes from Open Firmware (since we
> need to ask the firmware to stop bus mastering & idle devices the same way
> when we take over it and before we get control of the system memory) and
> it's called "quiesce".

Even if kexec is not brought into the picture the devices need to be quiesed on
reboot.  On x86 and probably other architectures there are 2 ways a reboot can go.
1) The firmware when it regains control toggles the motherboard reset
   line resetting all of the devices, so nothing we do really makes a difference.
2) The firmware when it regains control tweaks a few things and
   pretends it was never out of control, and restarts the boot
   process.

When the firmware does not toggle the motherboard reset line during a
reboot the firmware case is exactly equivalent to the kexec one.

So shutdown needs to quiese things.

Eric

Re: [PATCH] call drv->shutdown at rmmod

[Benjamin Herrenschmidt]

> Even if kexec is not brought into the picture the devices need to be quiesed on
> reboot.  On x86 and probably other architectures there are 2 ways a reboot can go.
> 1) The firmware when it regains control toggles the motherboard reset
>    line resetting all of the devices, so nothing we do really makes a difference.
> 2) The firmware when it regains control tweaks a few things and
>    pretends it was never out of control, and restarts the boot
>    process.
> 
> When the firmware does not toggle the motherboard reset line during a
> reboot the firmware case is exactly equivalent to the kexec one.
> 
> So shutdown needs to quiese things.

Yup, my point was mostly to say that quiescing for shutdown and putting
in low power state are 2 different things, and that the "shutdown" name
for the callback is a bit misleading...

Ben.

Re: [PATCH] call drv->shutdown at rmmod

[Patrick Mochel]

> There is a problem of semantics here. Is shutdown() supposed to shutdown
> the hardware device (ie. low power) or just the driver ? If yes, then
> it's duplicate of the PM callbacks. My understanding of the shutdown()
> callback is that it was more than "stop driver activity, put device into
> idle state" to prepare for a shutdown/reboot (though we do also sleep
> IDE drives in this case, but this is because of that nasty cache flush
> issue).

You have it right - ->shutdown() is only supposed to queisce the device. 


	Pat

Re: [PATCH] call drv->shutdown at rmmod

[Randy.Dunlap]

On 14 Aug 2003 10:26:47 -0600 ebiederm@xmission.com (Eric W. Biederman) wrote:

| Patrick Mochel <mochel@osdl.org> writes:
| 
| > > At the kexec BOF at OSDL there was some discussion on calling the
| > > device shutdown method at module remove time, in addition to calling
| > > it during reboot.  The driver was the observation that the primary
| > > source of problems in booting linux from linux are drivers with bad
| > > or missing drv->shutdown() routines.  The hope is this will increase
| > > the testing so people can get it right and kexec can become more
| > > useful.  In addition to making normal reboots more reliable.
| > > 
| > > The following patch is an implementation of that idea it calls drv->shutdown()
| > 
| > > before calling drv->remove().  If drv->shutdown() is implemented.
| > 
| > I like the idea behind the patch, but we shouldn't be calling it 
| > unconditionally. We're bound to run into some suprises that could really 
| > kill us this late in the 2.6 game. 
| > 
| > I do think it should go in, as long as there is a flag telling the core 
| > whether or not to call shutdown for that particular device. I think it 
| > could also be extended to include a power state, so the core could suspend 
| > the device before removing the module. 
| 
| Assuming the device driver can get a device out of the suspend state
| when the module is loaded.  This has been one of the biggest problem areas
| with the e100 driver.  It's init code cannot bring the device out of a low
| power state.
| 
| > The default would always be 'Do Nothing', but with a per-device sysfs 
| > file, a developer/user/gui app could be used to set the policy from user 
| > space. 
| 
| Possibly.  But this is getting complicated.  And while I do support things
| being complicated enough to handle the problem this part of the API feels
| like it is excessively complicated.  Which is why I was trying to simply
| it by always calling shutdown on a device before we remove it.

Can it just be a (kernel hacking/debug) CONFIG_DRIVER_SHUTDOWN build
option?

--
~Randy