public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ACPI: dock: Don't eval _STA on every show_docked sysfs read
@ 2009-01-20 11:18 Holger Macht
  2009-01-27 21:04 ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: Holger Macht @ 2009-01-20 11:18 UTC (permalink / raw)
  To: linux-acpi; +Cc: stable, Len Brown, Li, Shaohua, Daniel Smolik, akpm

Some devices trigger a DEVICE_CHECK on every evalutation of _STA. This
can also be seen in commit 8b59560a3baf2e7c24e0fb92ea5d09eca92805db
(ACPI: dock: avoid check _STA method).  If an undock is processed, the
dock driver sends a uevent and userspace might read the show_docked
property in sysfs. This causes an evaluation of _STA of the particular
device which causes the dock driver to immediately dock again.

In any case, evaluation of _STA (show_docked) does not necessarily mean
that we are docked, so check with the internal device structure.

http://bugzilla.kernel.org/show_bug.cgi?id=12360

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

diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
index 5b30b8d..afd5db3 100644
--- a/drivers/acpi/dock.c
+++ b/drivers/acpi/dock.c
@@ -855,10 +855,14 @@ fdd_out:
 static ssize_t show_docked(struct device *dev,
 			   struct device_attribute *attr, char *buf)
 {
+	struct acpi_device *tmp;
+
 	struct dock_station *dock_station = *((struct dock_station **)
 		dev->platform_data);
-	return snprintf(buf, PAGE_SIZE, "%d\n", dock_present(dock_station));
 
+	if (ACPI_SUCCESS(acpi_bus_get_device(dock_station->handle, &tmp)))
+		return snprintf(buf, PAGE_SIZE, "1\n");
+	return snprintf(buf, PAGE_SIZE, "0\n");
 }
 static DEVICE_ATTR(docked, S_IRUGO, show_docked, NULL);
 


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

* Re: [PATCH] ACPI: dock: Don't eval _STA on every show_docked sysfs read
  2009-01-20 11:18 [PATCH] ACPI: dock: Don't eval _STA on every show_docked sysfs read Holger Macht
@ 2009-01-27 21:04 ` Andrew Morton
  2009-02-06 16:47   ` Holger Macht
  2009-02-07  3:11   ` Len Brown
  0 siblings, 2 replies; 6+ messages in thread
From: Andrew Morton @ 2009-01-27 21:04 UTC (permalink / raw)
  To: hmacht; +Cc: linux-acpi, stable, lenb, Li, shaohua.li, marvin

On Tue, 20 Jan 2009 12:18:24 +0100
Holger Macht <hmacht@suse.de> wrote:

> Cc: stable@kernel.org

The patch applied OK to 2.6.28 but not to any earlier kernel.  If we
decide that it should be backported to 2.6.27 or earlier, a new patch
might be needed for that.

> Some devices trigger a DEVICE_CHECK on every evalutation of _STA. This
> can also be seen in commit 8b59560a3baf2e7c24e0fb92ea5d09eca92805db
> (ACPI: dock: avoid check _STA method).  If an undock is processed, the
> dock driver sends a uevent and userspace might read the show_docked
> property in sysfs. This causes an evaluation of _STA of the particular
> device which causes the dock driver to immediately dock again.
> 
> In any case, evaluation of _STA (show_docked) does not necessarily mean
> that we are docked, so check with the internal device structure.
> 
> http://bugzilla.kernel.org/show_bug.cgi?id=12360
> 
> Signed-off-by: Holger Macht <hmacht@suse.de>
> ---
> 
> diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
> index 5b30b8d..afd5db3 100644
> --- a/drivers/acpi/dock.c
> +++ b/drivers/acpi/dock.c
> @@ -855,10 +855,14 @@ fdd_out:
>  static ssize_t show_docked(struct device *dev,
>  			   struct device_attribute *attr, char *buf)
>  {
> +	struct acpi_device *tmp;
> +
>  	struct dock_station *dock_station = *((struct dock_station **)
>  		dev->platform_data);
> -	return snprintf(buf, PAGE_SIZE, "%d\n", dock_present(dock_station));
>  
> +	if (ACPI_SUCCESS(acpi_bus_get_device(dock_station->handle, &tmp)))
> +		return snprintf(buf, PAGE_SIZE, "1\n");
> +	return snprintf(buf, PAGE_SIZE, "0\n");
>  }
>  static DEVICE_ATTR(docked, S_IRUGO, show_docked, NULL);

That seems a little overwrought.  Won't plain old strcpy suffice?


--- a/drivers/acpi/dock.c~acpi-dock-dont-eval-_sta-on-every-show_docked-sysfs-read-simplification
+++ a/drivers/acpi/dock.c
@@ -861,8 +861,8 @@ static ssize_t show_docked(struct device
 		dev->platform_data);
 
 	if (ACPI_SUCCESS(acpi_bus_get_device(dock_station->handle, &tmp)))
-		return snprintf(buf, PAGE_SIZE, "1\n");
-	return snprintf(buf, PAGE_SIZE, "0\n");
+		return strcpy(buf, "1\n");
+	return strcpy(buf, "0\n");
 }
 static DEVICE_ATTR(docked, S_IRUGO, show_docked, NULL);
 
_


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

* Re: [PATCH] ACPI: dock: Don't eval _STA on every show_docked sysfs read
  2009-01-27 21:04 ` Andrew Morton
@ 2009-02-06 16:47   ` Holger Macht
  2009-02-06 21:33     ` Andrew Morton
  2009-02-07  3:11   ` Len Brown
  1 sibling, 1 reply; 6+ messages in thread
From: Holger Macht @ 2009-02-06 16:47 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-acpi, stable, lenb, Li, shaohua.li, marvin

[-- Attachment #1: Type: text/plain, Size: 442 bytes --]

Hi,

sorry for the delay...

On Tue, 2009-01-27 at 13:04 -0800, Andrew Morton wrote:
> On Tue, 20 Jan 2009 12:18:24 +0100
> Holger Macht <hmacht@suse.de> wrote:
> 
> > Cc: stable@kernel.org
> 
> The patch applied OK to 2.6.28 but not to any earlier kernel.  If we
> decide that it should be backported to 2.6.27 or earlier, a new patch
> might be needed for that.

Patch that applies to stable kernel from today is attached.

Thanks,
	Holger

[-- Attachment #2: acpi-dock-dont-eval-STA-on-every-show-docked-sysfs-read.patch --]
[-- Type: text/x-patch, Size: 1325 bytes --]

Some devices trigger a DEVICE_CHECK on every evalutation of _STA. This
can also be seen in commit 8b59560a3baf2e7c24e0fb92ea5d09eca92805db
(acpi: dock: avoid check _STA method).  If an undock is processed, the
dock driver sends a uevent and userspace might read the show_docked
property in sysfs. This causes an evaluation of _STA of the particular
device which causes the dock driver to immediately dock again.

In any case, evaluation of _STA (show_docked) does not necessarily mean
that we are docked, so check with the internal device structure.

http://bugzilla.kernel.org/show_bug.cgi?id=12360

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

diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
index 25d2161..7290e54 100644
--- a/drivers/acpi/dock.c
+++ b/drivers/acpi/dock.c
@@ -691,8 +691,14 @@ fdd_out:
 static ssize_t show_docked(struct device *dev,
 			   struct device_attribute *attr, char *buf)
 {
-	return snprintf(buf, PAGE_SIZE, "%d\n", dock_present(dock_station));
+	struct acpi_device *tmp;
+
+	struct dock_station *dock_station = *((struct dock_station **)
+		dev->platform_data);
 
+	if (ACPI_SUCCESS(acpi_bus_get_device(dock_station->handle, &tmp)))
+		return snprintf(buf, PAGE_SIZE, "1\n");
+	return snprintf(buf, PAGE_SIZE, "0\n");
 }
 static DEVICE_ATTR(docked, S_IRUGO, show_docked, NULL);
 

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

* Re: [PATCH] ACPI: dock: Don't eval _STA on every show_docked sysfs read
  2009-02-06 16:47   ` Holger Macht
@ 2009-02-06 21:33     ` Andrew Morton
  0 siblings, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2009-02-06 21:33 UTC (permalink / raw)
  To: hmacht; +Cc: linux-acpi, stable, lenb, Li, shaohua.li, marvin

On Fri, 06 Feb 2009 17:47:56 +0100
Holger Macht <hmacht@suse.de> wrote:

> Hi,
> 
> sorry for the delay...
> 
> On Tue, 2009-01-27 at 13:04 -0800, Andrew Morton wrote:
> > On Tue, 20 Jan 2009 12:18:24 +0100
> > Holger Macht <hmacht@suse.de> wrote:
> > 
> > > Cc: stable@kernel.org
> > 
> > The patch applied OK to 2.6.28 but not to any earlier kernel.  If we
> > decide that it should be backported to 2.6.27 or earlier, a new patch
> > might be needed for that.
> 
> Patch that applies to stable kernel from today is attached.
> 

ok, thanks.

But this won't be going into -stable until the mainline version gets
merged.

Len, where are we up to with that?

Thanks.

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

* Re: [PATCH] ACPI: dock: Don't eval _STA on every show_docked sysfs read
  2009-01-27 21:04 ` Andrew Morton
  2009-02-06 16:47   ` Holger Macht
@ 2009-02-07  3:11   ` Len Brown
  2009-02-07  3:18     ` Andrew Morton
  1 sibling, 1 reply; 6+ messages in thread
From: Len Brown @ 2009-02-07  3:11 UTC (permalink / raw)
  To: Andrew Morton; +Cc: hmacht, linux-acpi, stable, Li, shaohua.li, marvin



--
Len Brown, Intel Open Source Technology Center

On Tue, 27 Jan 2009, Andrew Morton wrote:

> On Tue, 20 Jan 2009 12:18:24 +0100
> Holger Macht <hmacht@suse.de> wrote:
> 
> > Cc: stable@kernel.org
> 
> The patch applied OK to 2.6.28 but not to any earlier kernel.  If we
> decide that it should be backported to 2.6.27 or earlier, a new patch
> might be needed for that.
> 
> > Some devices trigger a DEVICE_CHECK on every evalutation of _STA. This
> > can also be seen in commit 8b59560a3baf2e7c24e0fb92ea5d09eca92805db
> > (ACPI: dock: avoid check _STA method).  If an undock is processed, the
> > dock driver sends a uevent and userspace might read the show_docked
> > property in sysfs. This causes an evaluation of _STA of the particular
> > device which causes the dock driver to immediately dock again.
> > 
> > In any case, evaluation of _STA (show_docked) does not necessarily mean
> > that we are docked, so check with the internal device structure.
> > 
> > http://bugzilla.kernel.org/show_bug.cgi?id=12360
> > 
> > Signed-off-by: Holger Macht <hmacht@suse.de>
> > ---
> > 
> > diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
> > index 5b30b8d..afd5db3 100644
> > --- a/drivers/acpi/dock.c
> > +++ b/drivers/acpi/dock.c
> > @@ -855,10 +855,14 @@ fdd_out:
> >  static ssize_t show_docked(struct device *dev,
> >  			   struct device_attribute *attr, char *buf)
> >  {
> > +	struct acpi_device *tmp;
> > +
> >  	struct dock_station *dock_station = *((struct dock_station **)
> >  		dev->platform_data);
> > -	return snprintf(buf, PAGE_SIZE, "%d\n", dock_present(dock_station));
> >  
> > +	if (ACPI_SUCCESS(acpi_bus_get_device(dock_station->handle, &tmp)))
> > +		return snprintf(buf, PAGE_SIZE, "1\n");
> > +	return snprintf(buf, PAGE_SIZE, "0\n");
> >  }
> >  static DEVICE_ATTR(docked, S_IRUGO, show_docked, NULL);
> 
> That seems a little overwrought.  Won't plain old strcpy suffice?
> 
> 
> --- a/drivers/acpi/dock.c~acpi-dock-dont-eval-_sta-on-every-show_docked-sysfs-read-simplification
> +++ a/drivers/acpi/dock.c
> @@ -861,8 +861,8 @@ static ssize_t show_docked(struct device
>  		dev->platform_data);
>  
>  	if (ACPI_SUCCESS(acpi_bus_get_device(dock_station->handle, &tmp)))
> -		return snprintf(buf, PAGE_SIZE, "1\n");
> -	return snprintf(buf, PAGE_SIZE, "0\n");
> +		return strcpy(buf, "1\n");
> +	return strcpy(buf, "0\n");
>  }
>  static DEVICE_ATTR(docked, S_IRUGO, show_docked, NULL);
>  
> _
> 

sure, strcpy is fine b/c the src and dest are known to not be in danger of 
overlow.  applied to acpi misc branch.

thanks,
-Len


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

* Re: [PATCH] ACPI: dock: Don't eval _STA on every show_docked sysfs read
  2009-02-07  3:11   ` Len Brown
@ 2009-02-07  3:18     ` Andrew Morton
  0 siblings, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2009-02-07  3:18 UTC (permalink / raw)
  To: Len Brown; +Cc: hmacht, linux-acpi, stable, Li, shaohua.li, marvin

On Fri, 06 Feb 2009 22:11:15 -0500 (EST) Len Brown <lenb@kernel.org> wrote:

> 
> 
> --
> Len Brown, Intel Open Source Technology Center
> 
> On Tue, 27 Jan 2009, Andrew Morton wrote:
> 
> > On Tue, 20 Jan 2009 12:18:24 +0100
> > Holger Macht <hmacht@suse.de> wrote:
> > 
> > > Cc: stable@kernel.org
> > 
> > The patch applied OK to 2.6.28 but not to any earlier kernel.  If we
> > decide that it should be backported to 2.6.27 or earlier, a new patch
> > might be needed for that.
> > 
> > > Some devices trigger a DEVICE_CHECK on every evalutation of _STA. This
> > > can also be seen in commit 8b59560a3baf2e7c24e0fb92ea5d09eca92805db
> > > (ACPI: dock: avoid check _STA method).  If an undock is processed, the
> > > dock driver sends a uevent and userspace might read the show_docked
> > > property in sysfs. This causes an evaluation of _STA of the particular
> > > device which causes the dock driver to immediately dock again.
> > > 
> > > In any case, evaluation of _STA (show_docked) does not necessarily mean
> > > that we are docked, so check with the internal device structure.
> > > 
> > > http://bugzilla.kernel.org/show_bug.cgi?id=12360
> > > 
> > > Signed-off-by: Holger Macht <hmacht@suse.de>
> > > ---
> > > 
> > > diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
> > > index 5b30b8d..afd5db3 100644
> > > --- a/drivers/acpi/dock.c
> > > +++ b/drivers/acpi/dock.c
> > > @@ -855,10 +855,14 @@ fdd_out:
> > >  static ssize_t show_docked(struct device *dev,
> > >  			   struct device_attribute *attr, char *buf)
> > >  {
> > > +	struct acpi_device *tmp;
> > > +
> > >  	struct dock_station *dock_station = *((struct dock_station **)
> > >  		dev->platform_data);
> > > -	return snprintf(buf, PAGE_SIZE, "%d\n", dock_present(dock_station));
> > >  
> > > +	if (ACPI_SUCCESS(acpi_bus_get_device(dock_station->handle, &tmp)))
> > > +		return snprintf(buf, PAGE_SIZE, "1\n");
> > > +	return snprintf(buf, PAGE_SIZE, "0\n");
> > >  }
> > >  static DEVICE_ATTR(docked, S_IRUGO, show_docked, NULL);
> > 
> > That seems a little overwrought.  Won't plain old strcpy suffice?
> > 
> > 
> > --- a/drivers/acpi/dock.c~acpi-dock-dont-eval-_sta-on-every-show_docked-sysfs-read-simplification
> > +++ a/drivers/acpi/dock.c
> > @@ -861,8 +861,8 @@ static ssize_t show_docked(struct device
> >  		dev->platform_data);
> >  
> >  	if (ACPI_SUCCESS(acpi_bus_get_device(dock_station->handle, &tmp)))
> > -		return snprintf(buf, PAGE_SIZE, "1\n");
> > -	return snprintf(buf, PAGE_SIZE, "0\n");
> > +		return strcpy(buf, "1\n");
> > +	return strcpy(buf, "0\n");
> >  }
> >  static DEVICE_ATTR(docked, S_IRUGO, show_docked, NULL);
> >  
> > _
> > 
> 
> sure, strcpy is fine b/c the src and dest are known to not be in danger of 
> overlow.  applied to acpi misc branch.

argh, sorry, don't apply the strcpy() version - it's buggy.  It returns
the address of the string (and a compile warning) rather than the
amount-of-data-available.


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

end of thread, other threads:[~2009-02-07  4:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-20 11:18 [PATCH] ACPI: dock: Don't eval _STA on every show_docked sysfs read Holger Macht
2009-01-27 21:04 ` Andrew Morton
2009-02-06 16:47   ` Holger Macht
2009-02-06 21:33     ` Andrew Morton
2009-02-07  3:11   ` Len Brown
2009-02-07  3:18     ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox