LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] ACPI: Set flag DOCK_UNDOCKING when triggered via sysfs
@ 2008-03-12  0:07 Holger Macht
  2008-03-12  5:35 ` Andrew Morton
  0 siblings, 1 reply; 5+ messages in thread
From: Holger Macht @ 2008-03-12  0:07 UTC (permalink / raw)
  To: linux-acpi; +Cc: linux-kernel, Kristen Carlson Accardi, Len Brown

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.

Signed-off-by: Holger Macht <hmacht@suse.de>
---

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;
 }


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ACPI: Set flag DOCK_UNDOCKING when triggered via sysfs
  2008-03-12  0:07 [PATCH] ACPI: Set flag DOCK_UNDOCKING when triggered via sysfs Holger Macht
@ 2008-03-12  5:35 ` Andrew Morton
  2008-03-12 10:40   ` Holger Macht
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2008-03-12  5:35 UTC (permalink / raw)
  To: Holger Macht
  Cc: linux-acpi, linux-kernel, Kristen Carlson Accardi, Len Brown, stable

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?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ACPI: Set flag DOCK_UNDOCKING when triggered via sysfs
  2008-03-12  5:35 ` Andrew Morton
@ 2008-03-12 10:40   ` Holger Macht
  2008-03-13 17:27     ` Kristen Carlson Accardi
  0 siblings, 1 reply; 5+ messages in thread
From: Holger Macht @ 2008-03-12 10:40 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-acpi, linux-kernel, Kristen Carlson Accardi, Len Brown, stable

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ACPI: Set flag DOCK_UNDOCKING when triggered via sysfs
  2008-03-12 10:40   ` Holger Macht
@ 2008-03-13 17:27     ` Kristen Carlson Accardi
  2008-03-13 18:14       ` Holger Macht
  0 siblings, 1 reply; 5+ messages in thread
From: Kristen Carlson Accardi @ 2008-03-13 17:27 UTC (permalink / raw)
  To: Holger Macht; +Cc: Andrew Morton, linux-acpi, linux-kernel, Len Brown, stable

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>



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ACPI: Set flag DOCK_UNDOCKING when triggered via sysfs
  2008-03-13 17:27     ` Kristen Carlson Accardi
@ 2008-03-13 18:14       ` Holger Macht
  0 siblings, 0 replies; 5+ messages in thread
From: Holger Macht @ 2008-03-13 18:14 UTC (permalink / raw)
  To: Kristen Carlson Accardi
  Cc: Andrew Morton, linux-acpi, linux-kernel, Len Brown, stable

On Thu 13. Mar - 10:27:57, Kristen Carlson Accardi wrote:
> 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.

No, now I know that it doesn't seem to be the root cause. It definitely
helps here, so it seems to be a race.

My userspace does not read the flags file and is doing something bad in
turn of it. The freeze is caused by HAL, which reads /dev/sr0 after
docking, but just before libata detaches the device. We're currently about
to fix this:

 http://lkml.org/lkml/2008/3/11/418

However, it seems that the patch is not perfect yet :-(

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

Thanks,
	Holger

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2008-03-13 18:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-03-12  0:07 [PATCH] ACPI: Set flag DOCK_UNDOCKING when triggered via sysfs Holger Macht
2008-03-12  5:35 ` Andrew Morton
2008-03-12 10:40   ` Holger Macht
2008-03-13 17:27     ` Kristen Carlson Accardi
2008-03-13 18:14       ` Holger Macht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).