Messages in this thread | | | Date | Mon, 11 Feb 2008 12:28:02 +0100 | From | Holger Macht <> | Subject | Re: [PATCH] libata: Forcing PIO0 mode on reset must not freeze system |
| |
On Mon 11. Feb - 20:16:38, Tejun Heo wrote: > Hello, > > Holger Macht wrote: > >> In the above example, even the reset sequence itself can cause hang if > >> the hardware is implemented slightly differently. The reason why > >> set_piomode() locks up but reset sequence doesn't is simple dumb luck. > >> I think the proper fix is to tell libata to detach the cdrom before > >> undocking. > > > > Wouldn't the proper fix be to call ata_acpi_handle_hotplug _somewhere_? > > (which is currently called nowhere AFAICS) > > It should be called via ata_acpi_{ap|dev}_notify() callbacks installed > via acpi_install_notify_handler(). Can you add dump_stack() in the > function and verify that it actually is being called? It could be that > the method is called too late or libata takes too long to actually > unplug the device. Hmmm... It seems what ata_acpi_handle_hotplug() does > isn't enough for undock. It probably should request detaching the > device instead of just notifying hotplug event. Anyways, please lemme > know whether and when the function is called.
I already checked, it's never called AFAICS. And I couldn't find a place where it should be installed, otherwise, I would have sent a patch. The dock driver already calls the notify methods on devices in the dock station before doing the real undock.
> > Anyway, kernel hackers keep telling me that the kernel should just do the > > right thing. Shouldn't userspace never be able to freeze the system? > > Yeah, I think most things should be done automatically but it's true > that somethings are a bit awkward to handle in kernel. Also, if you're > root, you can almost always crash the machine from userland. > > > It's completely ok for me to handle this from userspace, if that's the > > position of the libata developers. > > Let's see whether we can fix the ACPI handler first.
Yes,
> > In this case, we should change the dock driver to default to > > immediate_undock=false, because otherwise it's far too risky to freeze the > > system. > > I'm not too familiar with how docks work. Can you please explain > briefly what immediate_undock is?
immediate_undock=1: User presses undock button on the dock station, dock driver calls ACPI undock method immediately.
immediate_undock=0: User presses undock button on the dock station, dock driver throws uevent and waits for userland to undock the system via sysfs.
immediate_undock is currently set to 1 by default.
Regards, Holger
| |