Re: [PATCH] libata: Forcing PIO0 mode on reset must not freeze system

Holger Macht
2008-02-11T11:27:26Z

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