Re: 2.6.19-rc3: known unfixed regressions (v3)

2006-10-30 Thread Michael S. Tsirkin
Quoting r. Adrian Bunk [EMAIL PROTECTED]:
 Subject: T60 stops triggering any ACPI events
 References : http://lkml.org/lkml/2006/10/4/425
  http://lkml.org/lkml/2006/10/16/262
  http://bugzilla.kernel.org/show_bug.cgi?id=7408
 Submitter  : Michael S. Tsirkin [EMAIL PROTECTED]
 Status : unknown

OK, I spent half a night with git-bisect, and the patch that triggers this issue
seems to be this:

commit d7dd8fd9557840162b724a8ac1366dd78a12dff
Author: Andrew Morton [EMAIL PROTECTED] 
[PATCH] blockdev.c: check driver layer errors

Reset to d7dd8fd9557840162b724a8ac1366dd78a12dff seems to hide part of the issue
(I have ACPI after kernel build, but not after suspend/resume).  Both reverting
this patch, and reset to the parent of this patch seem to solve (or at least,
hide) both problems for me (no ACPI after suspend/resume and no ACPI after
kernel build).

I am currently running on 2.6.19-rc3 minus
d7dd8fd9557840162b724a8ac1366dd78a12dff, and in a full day of use I have not
observed any issues yet. 2.6.19-rc3 without reverting
d7dd8fd9557840162b724a8ac1366dd78a12dff stops receiving ACPI events after some
use (sometimes after suspend/resume, sometimes after kernel build stress).  Now,
what does this tell us? Andrew, any idea?


Martin, could you test whether reverting this helps you, too, by chance?
Here's a patch to apply for testing this.

---

commit 658488b7577b7b2242372c43f081f55e2d274615
Author: Michael S. Tsirkin [EMAIL PROTECTED]
Date:   Mon Oct 30 01:28:40 2006 +0200

Revert [PATCH] blockdev.c: check driver layer errors

This reverts commit 4d7dd8fd9557840162b724a8ac1366dd78a12dff.

diff --git a/fs/block_dev.c b/fs/block_dev.c
index bc8f27c..b15ad29 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -545,11 +545,11 @@ static struct kobject *bdev_get_holder(s
return kobject_get(bdev-bd_disk-holder_dir);
 }
 
-static int add_symlink(struct kobject *from, struct kobject *to)
+static void add_symlink(struct kobject *from, struct kobject *to)
 {
if (!from || !to)
-   return 0;
-   return sysfs_create_link(from, to, kobject_name(to));
+   return;
+   sysfs_create_link(from, to, kobject_name(to));
 }
 
 static void del_symlink(struct kobject *from, struct kobject *to)
@@ -650,38 +650,30 @@ static void free_bd_holder(struct bd_hol
  * If there is no matching entry with @bo in @bdev-bd_holder_list,
  * add @bo to the list, create symlinks.
  *
- * Returns 0 if symlinks are created or already there.
- * Returns -ve if something fails and @bo can be freed.
+ * Returns 1 if @bo was added to the list.
+ * Returns 0 if @bo wasn't used by any reason and should be freed.
  */
 static int add_bd_holder(struct block_device *bdev, struct bd_holder *bo)
 {
struct bd_holder *tmp;
-   int ret;
 
if (!bo)
-   return -EINVAL;
+   return 0;
 
list_for_each_entry(tmp, bdev-bd_holder_list, list) {
if (tmp-sdir == bo-sdir) {
tmp-count++;
-   /* We've already done what we need to do here. */
-   free_bd_holder(bo);
return 0;
}
}
 
if (!bd_holder_grab_dirs(bdev, bo))
-   return -EBUSY;
+   return 0;
 
-   ret = add_symlink(bo-sdir, bo-sdev);
-   if (ret == 0) {
-   ret = add_symlink(bo-hdir, bo-hdev);
-   if (ret)
-   del_symlink(bo-sdir, bo-sdev);
-   }
-   if (ret == 0)
-   list_add_tail(bo-list, bdev-bd_holder_list);
-   return ret;
+   add_symlink(bo-sdir, bo-sdev);
+   add_symlink(bo-hdir, bo-hdev);
+   list_add_tail(bo-list, bdev-bd_holder_list);
+   return 1;
 }
 
 /**
@@ -751,9 +743,7 @@ static int bd_claim_by_kobject(struct bl
 
mutex_lock_nested(bdev-bd_mutex, BD_MUTEX_PARTITION);
res = bd_claim(bdev, holder);
-   if (res == 0)
-   res = add_bd_holder(bdev, bo);
-   if (res)
+   if (res || !add_bd_holder(bdev, bo))
free_bd_holder(bo);
mutex_unlock(bdev-bd_mutex);
 


-- 
MST
-
To unsubscribe from this list: send the line unsubscribe linux-acpi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

2006-10-30 Thread Luming Yu

updated version attached.

signed-off-by   [EMAIL PROTECTED]

[patch 1/6] video sysfs support: Add dev argument for backlight sys dev
drivers/video/backlight/backlight.c |7 +--
include/linux/backlight.h   |2 +-
2 files changed, 6 insertions(+), 3 deletions(-)

patch 2/6] Add display output class support
drivers/video/output.c |  129 +
include/linux/output.h |   42 +++
2 files changed, 171 insertions(+)

[patch 3/6] backlight and output sysfs support for acpi video driver
acpi/Kconfig   |2
acpi/video.c   |  257 +
video/Kconfig  |9 +
video/Makefile |1
4 files changed, 253 insertions(+), 16 deletions(-)

[patch 4/6] Add output class document
video-output.txt |   34 ++
1 file changed, 34 insertions(+)

[patch 5/6] fix comments style
video.c |   44 ++--
1 file changed, 22 insertions(+), 22 deletions(-)

[patch 6/6] fix compile time warning
video.c |4 +---
1 file changed, 1 insertion(+), 3 deletions(-)


On 10/30/06, Pavel Machek [EMAIL PROTECTED] wrote:

Hi!

  Some whitespace is not okay there...
  Pavel

 updated version.

 index 27597c5..1a18cdb 100644
 --- a/drivers/video/backlight/backlight.c
 +++ b/drivers/video/backlight/backlight.c
 @@ -190,7 +190,7 @@ static int fb_notifier_callback(struct n
   * Creates and registers new backlight class_device. Returns either an
   * ERR_PTR() or a pointer to the newly allocated device.
   */
 -struct backlight_device *backlight_device_register(const char *name, void 
*devdata,
 +struct backlight_device *backlight_device_register(const char *name,struct 
device *dev,  void *devdata,
  struct

80-columns, and fix the whitespace, please.

 --- /dev/null
 +++ b/drivers/video/output.c
 @@ -0,0 +1,110 @@
 +/*
 + * Video output switch support
 + */

I guess this one needs copyright/GPL.

 + struct output_device *new_dev;
 + int ret_code = 0;

Indentation is wrong where...

 + new_dev = (struct output_device *) kzalloc( sizeof(struct
output_device), GFP_KERNEL);

Cast should not be needed.

 + strlcpy(new_dev-class_dev.class_id, name, KOBJ_NAME_LEN);
 + class_set_devdata(new_dev-class_dev, devdata);
 + ret_code = class_device_register(new_dev-class_dev);
 + if (ret_code){

) {, please.

 + kfree (new_dev);

..and no space between kfree and its arguments.


 @@ -0,0 +1,27 @@
 +The output sysfs class driver is to provide video output abstract layer that 
can be used to hook platform specific methods to enable/disable video output 
device through common sysfs interface.
 +

80-columns, please. And some title would be nice.

 @@ -141,11 +144,11 @@ struct acpi_video_device_cap {
   u8 _ADR:1;  /*Return the unique ID */
   u8 _BCL:1;  /*Query list of brightness control levels 
supported */
   u8 _BCM:1;  /*Set the brightness level */
 + u8 _BQC:1;  /*Get current brightness level */
   u8 _DDC:1;  /*Return the EDID for this device */
   u8 _DCS:1;  /*Return status of output device */
   u8 _DGS:1;  /*Query graphics state */
   u8 _DSS:1;  /*Device state set */

It is nicer to have space between /* and comment.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 27597c5..1d97cdf 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -190,8 +190,10 @@ static int fb_notifier_callback(struct n
  * Creates and registers new backlight class_device. Returns either an
  * ERR_PTR() or a pointer to the newly allocated device.
  */
-struct backlight_device *backlight_device_register(const char *name, void *devdata,
-		   struct backlight_properties *bp)
+struct backlight_device *backlight_device_register(const char *name,
+	struct device *dev,
+	void *devdata,
+	struct backlight_properties *bp)
 {
 	int i, rc;
 	struct backlight_device *new_bd;
@@ -206,6 +208,7 @@ struct backlight_device *backlight_devic
 	new_bd-props = bp;
 	memset(new_bd-class_dev, 0, sizeof(new_bd-class_dev));
 	new_bd-class_dev.class = backlight_class;
+	new_bd-class_dev.dev = dev;
 	strlcpy(new_bd-class_dev.class_id, name, KOBJ_NAME_LEN);
 	class_set_devdata(new_bd-class_dev, devdata);
 
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 75e91f5..a5cf1be 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -54,7 +54,7 @@ struct backlight_device {
 };
 
 extern struct backlight_device 

Re: 2.6.19-rc3: known unfixed regressions (v3)

2006-10-30 Thread Jun'ichi Nomura

Hi Michael,

 2.6.19-rc3 without reverting
 d7dd8fd9557840162b724a8ac1366dd78a12dff stops receiving ACPI events after some
 use (sometimes after suspend/resume, sometimes after kernel build stress).  
Now,
 what does this tell us? Andrew, any idea?

The code is related to bd_claim_by_disk which is called when
device-mapper or md tries to mark the underlying devices
for exclusive use and creates symlinks from/to the devices
in sysfs. The patch added error handlings which weren't in
the original code.

I have no idea how it affects ACPI event handling.
Are you using dm and/or md on your machine?
Have you seen any unusual kernel messages or symptoms regarding
dm/md before the ACPI problem occurs?

Michael S. Tsirkin wrote:

Quoting r. Adrian Bunk [EMAIL PROTECTED]:

Subject: T60 stops triggering any ACPI events
References : http://lkml.org/lkml/2006/10/4/425
 http://lkml.org/lkml/2006/10/16/262
 http://bugzilla.kernel.org/show_bug.cgi?id=7408
Submitter  : Michael S. Tsirkin [EMAIL PROTECTED]
Status : unknown


OK, I spent half a night with git-bisect, and the patch that triggers this issue
seems to be this:

commit d7dd8fd9557840162b724a8ac1366dd78a12dff
Author: Andrew Morton [EMAIL PROTECTED] 
[PATCH] blockdev.c: check driver layer errors


Reset to d7dd8fd9557840162b724a8ac1366dd78a12dff seems to hide part of the issue
(I have ACPI after kernel build, but not after suspend/resume).  Both reverting
this patch, and reset to the parent of this patch seem to solve (or at least,
hide) both problems for me (no ACPI after suspend/resume and no ACPI after
kernel build).

I am currently running on 2.6.19-rc3 minus
d7dd8fd9557840162b724a8ac1366dd78a12dff, and in a full day of use I have not
observed any issues yet. 2.6.19-rc3 without reverting
d7dd8fd9557840162b724a8ac1366dd78a12dff stops receiving ACPI events after some
use (sometimes after suspend/resume, sometimes after kernel build stress).  Now,
what does this tell us? Andrew, any idea?


Martin, could you test whether reverting this helps you, too, by chance?
Here's a patch to apply for testing this.

---

commit 658488b7577b7b2242372c43f081f55e2d274615
Author: Michael S. Tsirkin [EMAIL PROTECTED]
Date:   Mon Oct 30 01:28:40 2006 +0200

Revert [PATCH] blockdev.c: check driver layer errors

This reverts commit 4d7dd8fd9557840162b724a8ac1366dd78a12dff.


Thanks,
--
Jun'ichi Nomura, NEC Corporation of America
-
To unsubscribe from this list: send the line unsubscribe linux-acpi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: 2.6.19-rc3: known unfixed regressions (v3)

2006-10-30 Thread Michael S. Tsirkin
Quoting r. Jun'ichi Nomura [EMAIL PROTECTED]:
 Subject: Re: 2.6.19-rc3: known unfixed regressions (v3)
 
 Hi Michael,
 
   2.6.19-rc3 without reverting
   d7dd8fd9557840162b724a8ac1366dd78a12dff stops receiving ACPI events after 
 some
   use (sometimes after suspend/resume, sometimes after kernel build stress). 
  Now,
   what does this tell us? Andrew, any idea?
 
 The code is related to bd_claim_by_disk which is called when
 device-mapper or md tries to mark the underlying devices
 for exclusive use and creates symlinks from/to the devices
 in sysfs. The patch added error handlings which weren't in
 the original code.
 
 I have no idea how it affects ACPI event handling.

It's a mystery. Probably exposes a bug somewhere?

 Are you using dm and/or md on your machine?

The .config is attached to bugzilla.

 Have you seen any unusual kernel messages or symptoms regarding
 dm/md before the ACPI problem occurs?

I haven't.

-- 
MST
-
To unsubscribe from this list: send the line unsubscribe linux-acpi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: 2.6.19-rc3: known unfixed regressions (v3)

2006-10-30 Thread Jun'ichi Nomura

Hi Michael,

Michael S. Tsirkin wrote:

The code is related to bd_claim_by_disk which is called when
device-mapper or md tries to mark the underlying devices
for exclusive use and creates symlinks from/to the devices
in sysfs. The patch added error handlings which weren't in
the original code.

I have no idea how it affects ACPI event handling.


It's a mystery. Probably exposes a bug somewhere?


Are you using dm and/or md on your machine?


The .config is attached to bugzilla.


OK, I found you disabled CONFIG_MD, which means neither
dm.ko nor md.ko was built.
Do you have any out-of-tree kernel modules which call either
bd_claim_by_kobject or bd_claim_by_disk?

If you aren't using either of them, I'm afraid reverting
the patch doesn't really solve your problem because the patched
code is called only from them.


Have you seen any unusual kernel messages or symptoms regarding
dm/md before the ACPI problem occurs?


I haven't.


Thanks,
--
Jun'ichi Nomura, NEC Corporation of America
-
To unsubscribe from this list: send the line unsubscribe linux-acpi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: 2.6.19-rc3: known unfixed regressions (v3)

2006-10-30 Thread Jun'ichi Nomura

Hi Linus,

Linus Torvalds wrote:
Actually, looking closer at the code, the patch seems to add _incorrect_ 
error handling.


For example, look at bd_claim_by_kobject(): if the bd_claim() inside of 
it succeeds, we used to always return success. Now, we don't necessarily 
do that: we may have done a _successful_ bd_claim() call, but then we 
return an error because something else failed, and now we're returning 
with from bd_claim_by_kobject() with the bd_claim() done, but with an 
error return (so the caller will _not_ call bd_release(), and the 
block_device will forever stay exclusive).


No?


You're right.

Now, exactly why acpi stops working as a result, I don't know, but maybe 
something else tries to get exclusive access to a swap partition, for 
example, and now fails, causing some acpi sequence to not be set up? 
Dunno.


So I suspect it should be reverted, but maybe somebody can see exactly 
what goes wrong here.


Please revert the patch. I'll fix the wrong error handling.

I'm not sure reverting the patch solves the ACPI problem
because Michael's kernel seems not having any user of
bd_claim_by_kobject.

Thanks,
--
Jun'ichi Nomura, NEC Corporation of America
-
To unsubscribe from this list: send the line unsubscribe linux-acpi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: 2.6.19-rc3: known unfixed regressions (v3)

2006-10-30 Thread Michael S. Tsirkin
Quoting r. Jun'ichi Nomura [EMAIL PROTECTED]:
 Subject: Re: 2.6.19-rc3: known unfixed regressions (v3)
 
 Hi Michael,
 
 Michael S. Tsirkin wrote:
  The code is related to bd_claim_by_disk which is called when
  device-mapper or md tries to mark the underlying devices
  for exclusive use and creates symlinks from/to the devices
  in sysfs. The patch added error handlings which weren't in
  the original code.
 
  I have no idea how it affects ACPI event handling.
  
  It's a mystery. Probably exposes a bug somewhere?
  
  Are you using dm and/or md on your machine?
  
  The .config is attached to bugzilla.
 
 OK, I found you disabled CONFIG_MD, which means neither
 dm.ko nor md.ko was built.
 Do you have any out-of-tree kernel modules which call either
 bd_claim_by_kobject or bd_claim_by_disk?

No, I don't have any out-of-tree modules.

 If you aren't using either of them, I'm afraid reverting
 the patch doesn't really solve your problem because the patched
 code is called only from them.

I agree this could be just papering over some issue.
The test results (of both git-bisect and reverting the patch) seem to be pretty
consistent so far though. Keep me posted if you rework the patch.

  Have you seen any unusual kernel messages or symptoms regarding
  dm/md before the ACPI problem occurs?
  
  I haven't.
 
 Thanks,

-- 
MST
-
To unsubscribe from this list: send the line unsubscribe linux-acpi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: 2.6.19-rc3: known unfixed regressions (v3)

2006-10-30 Thread Linus Torvalds


On Mon, 30 Oct 2006, Jun'ichi Nomura wrote:
 
 Please revert the patch. I'll fix the wrong error handling.
 
 I'm not sure reverting the patch solves the ACPI problem
 because Michael's kernel seems not having any user of
 bd_claim_by_kobject.

Yeah, doing a grep does seem to imply that there is no way that those 
changes could matter.

Michael, can you double-check? I think Jun'ichi is right - in your kernel, 
according to the config posted on bugzilla, I don't think there should be 
a single caller of bd_claim_by_disk, since CONFIG_MD is disabled.

So it does seem strange. But if you bisected to that patch, and it 
reliably does _not_ have problems with the patch reverted, maybe there is 
some strange preprocessor thing that makes grep not find the caller.

Michael, you also reported:

 Reset to d7dd8fd9557840162b724a8ac1366dd78a12dff seems to hide part of 
 the issue (I have ACPI after kernel build, but not after 
 suspend/resume).  Both reverting this patch, and reset to the parent of
 this patch seem to solve (or at least, hide) both problems for me (no 
 ACPI after suspend/resume and no ACPI after kernel build).

(where that d7dd8f.. is actually missing the initial 4 - I think you 
cut-and-pasted things incorrectly). 

So I wonder.. You still had ACPI working _after_ the kernel build even 
with that patch in place, and it seems that suspend/resume is the real 
issue. Martin Lorenz reports on the same bugzilla entry, and he only has 
problems with suspend/resume.

I assume that compile the kernel just triggers some magic ACPI event 
(probably fan-related due to heat), and I wonder if the bisection faked 
you out because once you get close enough the differences are small 
enough that the kernel compile is quick and the heat event doesn't 
actually trigger?

See what I'm saying? Maybe the act of bisecting itself changed the 
results, and then when you just revert the patch, you end up in the same 
situation: you only recompile a small part (you only recompile that 
particular file), and the problem doesn't occur, so you'd think that the 
revert fixed it.

If it's heat-related, it should probably trigger by anything that does a 
lot of CPU (and perhaps disk) accesses, not just kernel builds. It might 
be good to try to find another test-case for it than a kernel recompile, 
one that doesn't depend on how much changed in the kernel..

Linus
-
To unsubscribe from this list: send the line unsubscribe linux-acpi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[patch] acpi: use mutex instead of spinlock in dock driver

2006-10-30 Thread Kristen Carlson Accardi
http://bugzilla.kernel.org/show_bug.cgi?id=7303

Use a mutex instead of a spinlock for locking the
hotplug list because we need to call into the acpi
subsystem which might sleep.

Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED]
---
 drivers/acpi/dock.c |   16 
 1 file changed, 8 insertions(+), 8 deletions(-)

--- 2.6-git.orig/drivers/acpi/dock.c
+++ 2.6-git/drivers/acpi/dock.c
@@ -44,7 +44,7 @@ struct dock_station {
unsigned long last_dock_time;
u32 flags;
spinlock_t dd_lock;
-   spinlock_t hp_lock;
+   struct mutex hp_lock;
struct list_head dependent_devices;
struct list_head hotplug_devices;
 };
@@ -114,9 +114,9 @@ static void
 dock_add_hotplug_device(struct dock_station *ds,
struct dock_dependent_device *dd)
 {
-   spin_lock(ds-hp_lock);
+   mutex_lock(ds-hp_lock);
list_add_tail(dd-hotplug_list, ds-hotplug_devices);
-   spin_unlock(ds-hp_lock);
+   mutex_unlock(ds-hp_lock);
 }
 
 /**
@@ -130,9 +130,9 @@ static void
 dock_del_hotplug_device(struct dock_station *ds,
struct dock_dependent_device *dd)
 {
-   spin_lock(ds-hp_lock);
+   mutex_lock(ds-hp_lock);
list_del(dd-hotplug_list);
-   spin_unlock(ds-hp_lock);
+   mutex_unlock(ds-hp_lock);
 }
 
 /**
@@ -295,7 +295,7 @@ static void hotplug_dock_devices(struct 
 {
struct dock_dependent_device *dd;
 
-   spin_lock(ds-hp_lock);
+   mutex_lock(ds-hp_lock);
 
/*
 * First call driver specific hotplug functions
@@ -317,7 +317,7 @@ static void hotplug_dock_devices(struct 
else
dock_create_acpi_device(dd-handle);
}
-   spin_unlock(ds-hp_lock);
+   mutex_unlock(ds-hp_lock);
 }
 
 static void dock_event(struct dock_station *ds, u32 event, int num)
@@ -625,7 +625,7 @@ static int dock_add(acpi_handle handle)
INIT_LIST_HEAD(dock_station-dependent_devices);
INIT_LIST_HEAD(dock_station-hotplug_devices);
spin_lock_init(dock_station-dd_lock);
-   spin_lock_init(dock_station-hp_lock);
+   mutex_init(dock_station-hp_lock);
ATOMIC_INIT_NOTIFIER_HEAD(dock_notifier_list);
 
/* Find dependent devices */
-
To unsubscribe from this list: send the line unsubscribe linux-acpi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: 2.6.19-rc3: known unfixed regressions (v3)

2006-10-30 Thread Hugh Dickins
On Mon, 30 Oct 2006, Adrian Bunk wrote:
 
 Martin's original bug report stated now I loose ACPI events after 
 suspend/resume. not every time, but roughly 3 out of 4 times.
 This seems to support your theory.
 
 But considering that two people have independently reported this as a 
 2.6.19-rc regression for similar hardware (Michael for a T60 and Martin 
 for an X60), a problem in the kernel seems to be involved.

Add me to the list, on a T43p.  I believe it was happening in -rc1,
but seems worse with -rc3 and current.  But it's one of those things
which doesn't happen if you just go to test it out, only when you
need to suspend and resume for real.

Hugh 
-
To unsubscribe from this list: send the line unsubscribe linux-acpi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: 2.6.19-rc3: known unfixed regressions (v3)

2006-10-30 Thread Michael S. Tsirkin
Quoting r. Adrian Bunk [EMAIL PROTECTED]:
 Martin, Michael, can you send complete dmesg -s 100 for both 
 2.6.18.1 and a non-working 2.6.19-rc kernel after resume?
 I don't have high hopes, but perhaps looking at the dmesg and/or 
 diff'ing them might give a hint.

OK, I'll try to go back to this, just not today.

-- 
MST
-
To unsubscribe from this list: send the line unsubscribe linux-acpi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] acpi: use mutex instead of spinlock in dock driver

2006-10-30 Thread Len Brown
Applied.

thanks,
-Len

On Monday 30 October 2006 14:18, Kristen Carlson Accardi wrote:
 http://bugzilla.kernel.org/show_bug.cgi?id=7303
 
 Use a mutex instead of a spinlock for locking the
 hotplug list because we need to call into the acpi
 subsystem which might sleep.
 
 Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED]
 ---
  drivers/acpi/dock.c |   16 
  1 file changed, 8 insertions(+), 8 deletions(-)
 
 --- 2.6-git.orig/drivers/acpi/dock.c
 +++ 2.6-git/drivers/acpi/dock.c
 @@ -44,7 +44,7 @@ struct dock_station {
   unsigned long last_dock_time;
   u32 flags;
   spinlock_t dd_lock;
 - spinlock_t hp_lock;
 + struct mutex hp_lock;
   struct list_head dependent_devices;
   struct list_head hotplug_devices;
  };
 @@ -114,9 +114,9 @@ static void
  dock_add_hotplug_device(struct dock_station *ds,
   struct dock_dependent_device *dd)
  {
 - spin_lock(ds-hp_lock);
 + mutex_lock(ds-hp_lock);
   list_add_tail(dd-hotplug_list, ds-hotplug_devices);
 - spin_unlock(ds-hp_lock);
 + mutex_unlock(ds-hp_lock);
  }
  
  /**
 @@ -130,9 +130,9 @@ static void
  dock_del_hotplug_device(struct dock_station *ds,
   struct dock_dependent_device *dd)
  {
 - spin_lock(ds-hp_lock);
 + mutex_lock(ds-hp_lock);
   list_del(dd-hotplug_list);
 - spin_unlock(ds-hp_lock);
 + mutex_unlock(ds-hp_lock);
  }
  
  /**
 @@ -295,7 +295,7 @@ static void hotplug_dock_devices(struct 
  {
   struct dock_dependent_device *dd;
  
 - spin_lock(ds-hp_lock);
 + mutex_lock(ds-hp_lock);
  
   /*
* First call driver specific hotplug functions
 @@ -317,7 +317,7 @@ static void hotplug_dock_devices(struct 
   else
   dock_create_acpi_device(dd-handle);
   }
 - spin_unlock(ds-hp_lock);
 + mutex_unlock(ds-hp_lock);
  }
  
  static void dock_event(struct dock_station *ds, u32 event, int num)
 @@ -625,7 +625,7 @@ static int dock_add(acpi_handle handle)
   INIT_LIST_HEAD(dock_station-dependent_devices);
   INIT_LIST_HEAD(dock_station-hotplug_devices);
   spin_lock_init(dock_station-dd_lock);
 - spin_lock_init(dock_station-hp_lock);
 + mutex_init(dock_station-hp_lock);
   ATOMIC_INIT_NOTIFIER_HEAD(dock_notifier_list);
  
   /* Find dependent devices */
 -
 To unsubscribe from this list: send the line unsubscribe linux-acpi in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 
-
To unsubscribe from this list: send the line unsubscribe linux-acpi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2.6.19-rc3] (2/2) clean up add_bd_holder()

2006-10-30 Thread Jun'ichi Nomura
Hi,

This is an additional clean-up to [PATCH 2.6.19-rc3] (1/2)
fix bd_claim_by_kobject error handling.

add_bd_holder() is called from bd_claim_by_kobject to
put a given struct bd_holder in the list if there is no
matching entry.

There are 3 possible results of add_bd_holder():
  1. there is no matching entry and add the given one to the list
  2. there is matching entry, so just increment reference count of
 the existing one
  3. something failed during its course

1 and 2 are success. But for the case 2, someone has to free
the unused struct bd_holder.
Current 2.6.19-rc3 code frees it inside of add_bd_holder and
returns same value 0 for both cases 1 and 2.
However, it's natural and less error-prone if caller frees it
since it's allocated by the caller.

The attached patch separates the function in 2 parts to make things
clear. The patch depends on the previous fix.

Please consider to apply.

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America
add_bd_holder() is called from bd_claim_by_kobject to
put a given struct bd_holder in the list if there is no
matching entry.

There are 3 possible results of add_bd_holder():
  1. there is no matching entry and add the given one to the list
  2. there is matching entry, so just increment reference count of
 the existing one
  3. something failed during its course

1 and 2 are success. But for the case 2, someone has to free
the unused struct bd_holder.
Current 2.6.19-rc3 code frees it inside of add_bd_holder and
returns same value 0 for both cases 1 and 2.
However, it's natural and less error-prone if caller frees it
since it's allocated by the caller.

 fs/block_dev.c |   53 +++--
 1 file changed, 35 insertions(+), 18 deletions(-)

Signed-off-by: Jun'ichi Nomura [EMAIL PROTECTED]

--- linux-2.6.withfix/fs/block_dev.c	2006-10-30 15:38:21.0 -0500
+++ linux-2.6/fs/block_dev.c	2006-10-30 13:24:09.0 -0500
@@ -642,16 +642,38 @@ static void free_bd_holder(struct bd_hol
 }
 
 /**
+ * find_bd_holder - find matching struct bd_holder from the block device
+ *
+ * @bdev:	struct block device to be searched
+ * @bo:		target struct bd_holder
+ *
+ * Returns matching entry with @bo in @bdev-bd_holder_list.
+ * If found, increment the reference count and return the pointer.
+ * If not found, returns NULL.
+ */
+static int find_bd_holder(struct block_device *bdev, struct bd_holder *bo)
+{
+	struct bd_holder *tmp;
+
+	list_for_each_entry(tmp, bdev-bd_holder_list, list)
+		if (tmp-sdir == bo-sdir) {
+			tmp-count++;
+			return tmp;
+		}
+
+	return NULL;
+}
+
+/**
  * add_bd_holder - create sysfs symlinks for bd_claim() relationship
  *
  * @bdev:	block device to be bd_claimed
  * @bo:		preallocated and initialized by alloc_bd_holder()
  *
- * If there is no matching entry with @bo in @bdev-bd_holder_list,
- * add @bo to the list, create symlinks.
+ * Add @bo to @bdev-bd_holder_list, create symlinks.
  *
- * Returns 0 if symlinks are created or already there.
- * Returns -ve if something fails and @bo can be freed.
+ * Returns 0 if symlinks are created.
+ * Returns -ve if something fails.
  */
 static int add_bd_holder(struct block_device *bdev, struct bd_holder *bo)
 {
@@ -661,15 +683,6 @@ static int add_bd_holder(struct block_de
 	if (!bo)
 		return -EINVAL;
 
-	list_for_each_entry(tmp, bdev-bd_holder_list, list) {
-		if (tmp-sdir == bo-sdir) {
-			tmp-count++;
-			/* We've already done what we need to do here. */
-			free_bd_holder(bo);
-			return 0;
-		}
-	}
-
 	if (!bd_holder_grab_dirs(bdev, bo))
 		return -EBUSY;
 
@@ -740,7 +753,7 @@ static int bd_claim_by_kobject(struct bl
 struct kobject *kobj)
 {
 	int res;
-	struct bd_holder *bo;
+	struct bd_holder *bo, *found;
 
 	if (!kobj)
 		return -EINVAL;
@@ -752,11 +765,15 @@ static int bd_claim_by_kobject(struct bl
 	mutex_lock_nested(bdev-bd_mutex, BD_MUTEX_PARTITION);
 	res = bd_claim(bdev, holder);
 	if (res == 0) {
-		res = add_bd_holder(bdev, bo);
-		if (res)
-			bd_release(bdev);
+		found = find_bd_holder(bdev, bo);
+		if (found == NULL) {
+			res = add_bd_holder(bdev, bo);
+			if (res)
+bd_release(bdev);
+		}
 	}
-	if (res)
+
+	if (res || found)
 		free_bd_holder(bo);
 	mutex_unlock(bdev-bd_mutex);
 


[PATCH 2.6.19-rc3] (1/2) fix bd_claim_by_kobject error handling

2006-10-30 Thread Jun'ichi Nomura
Hi,

Excuse me for the long Cc list.
I kept people in the thread below in Cc.
http://marc.theaimsgroup.com/?l=linux-kernelm=116221664710826w=2

The patch below fixes bd_claim_by_kobject to release bdev correctly
in case that bd_claim succeeds but following add_bd_holder fails.

If it happens, the caller takes it didn't bd_claim() where actually
it did. The bdev becomes no longer bd_claim-able from others.
I don't have any reproducible test case for it but it's a bug.
The bug is introduced in 2.6.18-rc1-mm2 and now in 2.6.19-rc3.

Please consider to apply.

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America
The patch below fixes bd_claim_by_kobject to release bdev correctly
in case that bd_claim succeeds but following add_bd_holder fails.

 fs/block_dev.c |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

Signed-off-by: Jun'ichi Nomura [EMAIL PROTECTED]

--- linux-2.6.orig/fs/block_dev.c	2006-10-30 10:07:16.0 -0500
+++ linux-2.6/fs/block_dev.c	2006-10-30 15:38:21.0 -0500
@@ -751,8 +751,11 @@ static int bd_claim_by_kobject(struct bl
 
 	mutex_lock_nested(bdev-bd_mutex, BD_MUTEX_PARTITION);
 	res = bd_claim(bdev, holder);
-	if (res == 0)
+	if (res == 0) {
 		res = add_bd_holder(bdev, bo);
+		if (res)
+			bd_release(bdev);
+	}
 	if (res)
 		free_bd_holder(bo);
 	mutex_unlock(bdev-bd_mutex);


why use acpi for cpufreq?

2006-10-30 Thread Michael Blakeley
I've recently been bitten by 
http://bugzilla.kernel.org/show_bug.cgi?id=7157 - I also have an HP 
nc6400, with bios F.05, and can't downgrade to F.03 because my CPU only 
became supported in F.05. While I'm waiting to see if HP will fix this 
problem in a future bios rev, I wondered why cpufreq needs any bios 
information in the first place?


This is all well outside my technical expertise, so I'm sure that I'm 
misunderstanding the problem - but that's why I'm asking. Basically my 
question is: why can't cpufreq interrogate the CPU directly, in order to 
find out what sort of cpu frequency scaling it supports (SpeedStep in 
various flavors, PowerNow, and whatever else is out there in the wild)? 
Wouldn't that be more robust than relying on the bios to get it right?


As a side issue, how does Windows XP solve this problem? From what I can 
tell, Windows XP does use frequency-scaling with my CPU (according to 
the NHC software from http://www.pbus-167.com/).


Again, I'm sure that I'm misunderstanding the technical issues here, but 
I'd like to understand them. I'm also happy to post a summary on the 
acpi wiki, so that the question is less likely to come up again.


thanks,
-- Mike
-
To unsubscribe from this list: send the line unsubscribe linux-acpi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: 2.6.19-rc3-mm1 - udev doesn't work (was: ATI SATA controller not detected)

2006-10-30 Thread Rafael J. Wysocki
[Resending due to a network problem on my side.]

On Monday, 30 October 2006 21:57, Greg KH wrote:
 On Mon, Oct 30, 2006 at 09:48:33PM +0100, Rafael J. Wysocki wrote:
  On Monday, 30 October 2006 21:22, Greg KH wrote:
   On Mon, Oct 30, 2006 at 09:15:37PM +0100, Rafael J. Wysocki wrote:
Sorry, I was wrong.

The controller _is_ detected and handled properly, but udev is 
apparently
unable to create the special device files for SATA drives/partitions 
even
though CONFIG_SYSFS_DEPRECATED is set.
   
   This config option should not affect the block device sysfs files at all
   at this point in time.
   
   What does 'tree /sys/block/' show?
  
  I can't run 'tree', but 'ls' works somehow (can't mount the root fs).  The
  block device sysfs files seem to be present
 
 If they are there, then udev should work just fine.
 
   If the files show up there properly, udev should handle them just fine.
  
  It doesn't.
  
  Well, I can binary search for the offending patch if that helps.
 
 That would be very helpful, thanks.

It's one of these:

git-acpi.patch
git-acpi-fixup.patch
git-acpi-more-build-fixes.patch

Greetings,
Rafael


-- 
You never change things by fighting the existing reality.
R. Buckminster Fuller
-
To unsubscribe from this list: send the line unsubscribe linux-acpi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: 2.6.19-rc3-mm1 - udev doesn't work (was: ATI SATA controller not detected)

2006-10-30 Thread Rafael J. Wysocki
On Tuesday, 31 October 2006 08:40, Andrew Morton wrote:
 On Tue, 31 Oct 2006 08:29:28 +0100
 Rafael J. Wysocki [EMAIL PROTECTED] wrote:
 
  [Resending due to a network problem on my side.]
  
  On Monday, 30 October 2006 21:57, Greg KH wrote:
   On Mon, Oct 30, 2006 at 09:48:33PM +0100, Rafael J. Wysocki wrote:
On Monday, 30 October 2006 21:22, Greg KH wrote:
 On Mon, Oct 30, 2006 at 09:15:37PM +0100, Rafael J. Wysocki wrote:
  Sorry, I was wrong.
  
  The controller _is_ detected and handled properly, but udev is 
  apparently
  unable to create the special device files for SATA 
  drives/partitions even
  though CONFIG_SYSFS_DEPRECATED is set.
 
 This config option should not affect the block device sysfs files at 
 all
 at this point in time.
 
 What does 'tree /sys/block/' show?

I can't run 'tree', but 'ls' works somehow (can't mount the root fs).  
The
block device sysfs files seem to be present
   
   If they are there, then udev should work just fine.
   
 If the files show up there properly, udev should handle them just 
 fine.

It doesn't.

Well, I can binary search for the offending patch if that helps.
   
   That would be very helpful, thanks.
  
  It's one of these:
  
  git-acpi.patch
  git-acpi-fixup.patch
  git-acpi-more-build-fixes.patch
  
 
 You might need to resend the original report so the acpi guys can see it.

Okay, I will.

 Meanwhile, I'll have to drop the acpi tree.

Well, I'd prefer to find the offending commit within the tree, as the majority
of changes look pretty innocent.  Are the commits available somewhere as
individual patches?
-
To unsubscribe from this list: send the line unsubscribe linux-acpi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html