This just hit:
[ 7.856000] usbcore: registered new interface driver usbhid
[ 7.860000] BUG: at kernel/mutex.c:311 __mutex_trylock_slowpath()
[ 7.868000] [<c0104584>] show_trace_log_lvl+0x1a/0x30
[ 7.872000] [<c0105196>] show_trace+0x12/0x14
[ 7.876000] [<c0105222>] dump_stack+0x15/0x17
[ 7.880000] [<c03cd4c8>] mutex_trylock+0x56/0x15a
[ 7.888000] [<c03172c5>] hdaps_mousedev_poll+0x10/0xcb
[ 7.892000] [<c011f8c5>] run_timer_softirq+0x10e/0x16f
[ 7.896000] [<c011cabc>] __do_softirq+0x5d/0xc0
[ 7.900000] [<c0105c88>] do_softirq+0x6e/0xf0
[ 7.904000] [<c011ca22>] irq_exit+0x3e/0x7b
[ 7.912000] [<c0105da7>] do_IRQ+0x9d/0xb2
[ 7.916000] [<c01040fa>] common_interrupt+0x2e/0x34
[ 7.920000] [<c0118e5f>] printk+0x1b/0x1d
[ 7.924000] [<c02f2f96>] usb_register_driver+0xa0/0xe5
[ 7.928000] [<c0525018>] hid_init+0x28/0x51
[ 7.932000] [<c050e6c4>] kernel_init+0xbc/0x23e
[ 7.940000] [<c010420f>] kernel_thread_helper+0x7/0x10
[ 7.944000] =======================
[ 7.948000] drivers/hid/usbhid/hid-core.c: v2.6:USB HID core driver
Looks like it's triggered by the HDAPS driver.
--
Mathematics is the supreme nostalgia of our time.
On Fri, 11 May 2007 19:21:15 -0500
Matt Mackall <[email protected]> wrote:
> This just hit:
>
> [ 7.856000] usbcore: registered new interface driver usbhid
> [ 7.860000] BUG: at kernel/mutex.c:311 __mutex_trylock_slowpath()
> [ 7.868000] [<c0104584>] show_trace_log_lvl+0x1a/0x30
> [ 7.872000] [<c0105196>] show_trace+0x12/0x14
> [ 7.876000] [<c0105222>] dump_stack+0x15/0x17
> [ 7.880000] [<c03cd4c8>] mutex_trylock+0x56/0x15a
> [ 7.888000] [<c03172c5>] hdaps_mousedev_poll+0x10/0xcb
> [ 7.892000] [<c011f8c5>] run_timer_softirq+0x10e/0x16f
> [ 7.896000] [<c011cabc>] __do_softirq+0x5d/0xc0
> [ 7.900000] [<c0105c88>] do_softirq+0x6e/0xf0
> [ 7.904000] [<c011ca22>] irq_exit+0x3e/0x7b
> [ 7.912000] [<c0105da7>] do_IRQ+0x9d/0xb2
> [ 7.916000] [<c01040fa>] common_interrupt+0x2e/0x34
> [ 7.920000] [<c0118e5f>] printk+0x1b/0x1d
> [ 7.924000] [<c02f2f96>] usb_register_driver+0xa0/0xe5
> [ 7.928000] [<c0525018>] hid_init+0x28/0x51
> [ 7.932000] [<c050e6c4>] kernel_init+0xbc/0x23e
> [ 7.940000] [<c010420f>] kernel_thread_helper+0x7/0x10
> [ 7.944000] =======================
> [ 7.948000] drivers/hid/usbhid/hid-core.c: v2.6:USB HID core driver
>
> Looks like it's triggered by the HDAPS driver.
>
It's complaining about a mutex_trylock() being run from irq context.
And indeed that's buggy - the non-debug version of spin_lock_mutex() is not
irq-safe.
I'd say that's pretty dumb of the mutex interface, really. Doing a
mutex_trylock() should be OK from all contexts.
This is caused by a recent semaphore->mutex conversion and it's in mainline
now.
Ho hum. I suppose a suitable workaround would be to convert hdaps_mtx back
into a semaphore. ug.
On Fri, 11 May 2007 17:53:35 -0700
Andrew Morton <[email protected]> wrote:
> And indeed that's buggy - the non-debug version of spin_lock_mutex() is not
> irq-safe.
>
> I'd say that's pretty dumb of the mutex interface, really. Doing a
> mutex_trylock() should be OK from all contexts.
We can fix this in a low-impact fashion by making mutex_trylock() do a
spin_trylock() on mutex->wait_lock, no?
On Friday 11 May 2007 20:53, Andrew Morton wrote:
> Ho hum. ?I suppose a suitable workaround would be to convert hdaps_mtx back
> into a semaphore. ?ug.
Actually I was looking for victimes^Wvolunteers to test the patch below.
It gets rid of _trylock business.
--
Dmitry
HWMON: hdaps - convert to use input-polldev
Switch to using input-polldev skeleton instead of implementing
polling loop by itself.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/hwmon/Kconfig | 1
drivers/hwmon/hdaps.c | 55 +++++++++++++++++++++-----------------------------
2 files changed, 25 insertions(+), 31 deletions(-)
Index: work/drivers/hwmon/Kconfig
===================================================================
--- work.orig/drivers/hwmon/Kconfig
+++ work/drivers/hwmon/Kconfig
@@ -602,6 +602,7 @@ config SENSORS_W83627EHF
config SENSORS_HDAPS
tristate "IBM Hard Drive Active Protection System (hdaps)"
depends on INPUT && X86
+ select INPUT_POLLDEV
default n
help
This driver provides support for the IBM Hard Drive Active Protection
Index: work/drivers/hwmon/hdaps.c
===================================================================
--- work.orig/drivers/hwmon/hdaps.c
+++ work/drivers/hwmon/hdaps.c
@@ -28,7 +28,7 @@
#include <linux/delay.h>
#include <linux/platform_device.h>
-#include <linux/input.h>
+#include <linux/input-polldev.h>
#include <linux/kernel.h>
#include <linux/mutex.h>
#include <linux/module.h>
@@ -61,13 +61,12 @@
#define INIT_TIMEOUT_MSECS 4000 /* wait up to 4s for device init ... */
#define INIT_WAIT_MSECS 200 /* ... in 200ms increments */
-#define HDAPS_POLL_PERIOD (HZ/20) /* poll for input every 1/20s */
+#define HDAPS_POLL_INTERVAL 50 /* poll for input every 1/20s (50 ms)*/
#define HDAPS_INPUT_FUZZ 4 /* input event threshold */
#define HDAPS_INPUT_FLAT 4
-static struct timer_list hdaps_timer;
static struct platform_device *pdev;
-static struct input_dev *hdaps_idev;
+static struct input_polled_dev *hdaps_idev;
static unsigned int hdaps_invert;
static u8 km_activity;
static int rest_x;
@@ -323,24 +322,19 @@ static void hdaps_calibrate(void)
__hdaps_read_pair(HDAPS_PORT_XPOS, HDAPS_PORT_YPOS, &rest_x, &rest_y);
}
-static void hdaps_mousedev_poll(unsigned long unused)
+static void hdaps_mousedev_poll(struct input_polled_dev *dev)
{
+ struct input_dev *input_dev = dev->input;
int x, y;
- /* Cannot sleep. Try nonblockingly. If we fail, try again later. */
- if (mutex_trylock(&hdaps_mtx)) {
- mod_timer(&hdaps_timer,jiffies + HDAPS_POLL_PERIOD);
- return;
- }
+ mutex_lock(&hdaps_mtx);
if (__hdaps_read_pair(HDAPS_PORT_XPOS, HDAPS_PORT_YPOS, &x, &y))
goto out;
- input_report_abs(hdaps_idev, ABS_X, x - rest_x);
- input_report_abs(hdaps_idev, ABS_Y, y - rest_y);
- input_sync(hdaps_idev);
-
- mod_timer(&hdaps_timer, jiffies + HDAPS_POLL_PERIOD);
+ input_report_abs(input_dev, ABS_X, x - rest_x);
+ input_report_abs(input_dev, ABS_Y, y - rest_y);
+ input_sync(input_dev);
out:
mutex_unlock(&hdaps_mtx);
@@ -536,6 +530,7 @@ static struct dmi_system_id __initdata h
static int __init hdaps_init(void)
{
+ struct input_dev *idev;
int ret;
if (!dmi_check_system(hdaps_whitelist)) {
@@ -563,39 +558,37 @@ static int __init hdaps_init(void)
if (ret)
goto out_device;
- hdaps_idev = input_allocate_device();
+ hdaps_idev = input_allocate_polled_device();
if (!hdaps_idev) {
ret = -ENOMEM;
goto out_group;
}
+ hdaps_idev->poll = hdaps_mousedev_poll;
+ hdaps_idev->poll_interval = HDAPS_POLL_INTERVAL;
+
/* initial calibrate for the input device */
hdaps_calibrate();
/* initialize the input class */
- hdaps_idev->name = "hdaps";
- hdaps_idev->dev.parent = &pdev->dev;
- hdaps_idev->evbit[0] = BIT(EV_ABS);
- input_set_abs_params(hdaps_idev, ABS_X,
+ idev = hdaps_idev->input;
+ idev->name = "hdaps";
+ idev->dev.parent = &pdev->dev;
+ idev->evbit[0] = BIT(EV_ABS);
+ input_set_abs_params(idev, ABS_X,
-256, 256, HDAPS_INPUT_FUZZ, HDAPS_INPUT_FLAT);
- input_set_abs_params(hdaps_idev, ABS_Y,
+ input_set_abs_params(idev, ABS_Y,
-256, 256, HDAPS_INPUT_FUZZ, HDAPS_INPUT_FLAT);
- ret = input_register_device(hdaps_idev);
+ ret = input_register_polled_device(hdaps_idev);
if (ret)
goto out_idev;
- /* start up our timer for the input device */
- init_timer(&hdaps_timer);
- hdaps_timer.function = hdaps_mousedev_poll;
- hdaps_timer.expires = jiffies + HDAPS_POLL_PERIOD;
- add_timer(&hdaps_timer);
-
printk(KERN_INFO "hdaps: driver successfully loaded.\n");
return 0;
out_idev:
- input_free_device(hdaps_idev);
+ input_free_polled_device(hdaps_idev);
out_group:
sysfs_remove_group(&pdev->dev.kobj, &hdaps_attribute_group);
out_device:
@@ -611,8 +604,8 @@ out:
static void __exit hdaps_exit(void)
{
- del_timer_sync(&hdaps_timer);
- input_unregister_device(hdaps_idev);
+ input_unregister_polled_device(hdaps_idev);
+ input_free_polled_device(hdaps_idev);
sysfs_remove_group(&pdev->dev.kobj, &hdaps_attribute_group);
platform_device_unregister(pdev);
platform_driver_unregister(&hdaps_driver);
Hi Dmitry,
On 5/12/07, Dmitry Torokhov <[email protected]> wrote:
> On Friday 11 May 2007 20:53, Andrew Morton wrote:
> > Ho hum. I suppose a suitable workaround would be to convert hdaps_mtx back
> > into a semaphore. ug.
>
> Actually I was looking for victimes^Wvolunteers to test the patch below.
> It gets rid of _trylock business.
Ah! You just beat me here, and your patch is definitely better.
I was wondering why this driver wanted to use a mutex (previously the
semaphore) to synchronize between process and interrupt context in the
first place. Most of the code in here uses synchronous delays so never
sleeps anyway, but then unfortunately it does a weird
repeated-waiting-hardware-status-register-check thingy in its .probe()
which meant a straightforward mutex -> spinlock wasn't possible.
So then made a patch pushing off the poll to keventd workqueue, when
I saw your mail that does exactly the same, but wrapped about in the
generic input-polldev infrastructure! It's barely 12 days old in mainline --
no wonder I didn't know about it. Seems to be good-looking code!
Thanks,
Satyam
Hi Satyam,
On Saturday 12 May 2007 01:45, Satyam Sharma wrote:
> Seems to be good-looking code!
Thanks. Do you have the hardware? Were you able to test the patch?
--
Dmitry
On 5/14/07, Dmitry Torokhov <[email protected]> wrote:
> Hi Satyam,
>
> On Saturday 12 May 2007 01:45, Satyam Sharma wrote:
> > Seems to be good-looking code!
>
> Thanks. Do you have the hardware? Were you able to test the patch?
Oh, sorry, no. I was actually referring to the input-polldev code. Let's
hope Matt can test and ack this.
Thanks,
Satyam
2007/5/14, Satyam Sharma <[email protected]>:
> On 5/14/07, Dmitry Torokhov <[email protected]> wrote:
> > Hi Satyam,
> >
> > On Saturday 12 May 2007 01:45, Satyam Sharma wrote:
> > > Seems to be good-looking code!
> >
> > Thanks. Do you have the hardware? Were you able to test the patch?
>
> Oh, sorry, no. I was actually referring to the input-polldev code. Let's
> hope Matt can test and ack this.
>
> Thanks,
> Satyam
> -
Hi all,
Since i still had that BUG on my T43 running a 2.6.22-rc2-mm1 kernel,
I tried that patch. It sounds like this has solved the bug since I
can't find any trace of that issue anymore once the patch is applied.
Thanks for the patch,
Regards,
C?dric