Re: [PATCH] ACPI: Set flag DOCK_UNDOCKING when triggered via sysfs

[Kristen Carlson Accardi <kristen.c.accardi@intel.com>]

On Wed, 12 Mar 2008 11:40:35 +0100
Holger Macht <hmacht@suse.de> wrote:

> On Di 11. Mär - 22:35:22, Andrew Morton wrote:
> > On Wed, 12 Mar 2008 01:07:27 +0100 Holger Macht <hmacht@suse.de>
> > wrote:
> > 
> > > begin_undock() is only called when triggered via a acpi notify
> > > handler (pressing the undock button on the dock station), but
> > > complete_undock() is always called after the eject. So if a
> > > undock is triggered through a sysfs write, the flag
> > > DOCK_UNDOCKING has to be set for the dock station, too. Otherwise
> > > this will freeze the system hard.
> > > 
> > 
> > We prefer not to make systems freeze hard.
> > 
> > > ---
> > > 
> > > diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
> > > index 307cef6..fa44fb9 100644
> > > --- a/drivers/acpi/dock.c
> > > +++ b/drivers/acpi/dock.c
> > > @@ -710,6 +710,7 @@ static ssize_t write_undock(struct device
> > > *dev, struct device_attribute *attr, if (!count)
> > >  		return -EINVAL;
> > >  
> > > +	begin_undock(dock_station);
> > >  	ret = handle_eject_request(dock_station,
> > > ACPI_NOTIFY_EJECT_REQUEST); return ret ? ret: count;
> > >  }
> > 
> > I assume that this is needed in 2.6.24.x as well?
> 
> I tested this, and no, I weren't able to reproduce on 2.6.24.
> 
> Moreover, it seems that the freeze is related to some libata issue
> because it only freezes when a drive is in the dock station.
> 
> Nevertheless, I think the patch is correct. Maybe there's a race
> involved somewhere I'm not aware of. I really like to wait for
> Kristen to comment before pushing it into more trees.
> 
> Thanks,
> 	Holger
> 

Hum, I'm surprised this caused a freeze - you are sure this fixes your
problem?  This flag is never checked anywhere in the dock code, and if
it's not set, it will not matter, it will still do the undock.  The
flags file is exported to user space via sysfs though, so maybe because
the flag isn't being set, some user space script is doing something it
shouldn't?  The patch does seem correct though, so might as well add it.

Acked-by:  Kristen Carlson Accardi <kristen.c.accardi@intel.com>