2006-09-26 11:45:28

by Peter Zijlstra

[permalink] [raw]
Subject: [PATCH 2/2] serio: lockdep annotation for ps2dev->cmd_mutex and serio->lock

Based ideas from Jiri Kosina, this patch tracks the nesting depth
and uses the new lockdep_set_class_and_subclass() annotation to store
this information in the lock objects.

Signed-of-by: Peter Zijlstra <[email protected]>
---
drivers/input/serio/libps2.c | 4 ++++
drivers/input/serio/serio.c | 9 ++++++++-
include/linux/serio.h | 1 +
3 files changed, 13 insertions(+), 1 deletion(-)

Index: linux-2.6-mm/drivers/input/serio/libps2.c
===================================================================
--- linux-2.6-mm.orig/drivers/input/serio/libps2.c 2006-09-26 10:25:22.000000000 +0200
+++ linux-2.6-mm/drivers/input/serio/libps2.c 2006-09-26 10:34:49.000000000 +0200
@@ -280,6 +280,8 @@ int ps2_schedule_command(struct ps2dev *
return 0;
}

+static struct lock_class_key ps2_mutex_key;
+
/*
* ps2_init() initializes ps2dev structure
*/
@@ -287,6 +289,8 @@ int ps2_schedule_command(struct ps2dev *
void ps2_init(struct ps2dev *ps2dev, struct serio *serio)
{
mutex_init(&ps2dev->cmd_mutex);
+ lockdep_set_class_and_subclass(&ps2dev->cmd_mutex, &ps2_mutex_key,
+ serio->depth);
init_waitqueue_head(&ps2dev->wait);
ps2dev->serio = serio;
}
Index: linux-2.6-mm/drivers/input/serio/serio.c
===================================================================
--- linux-2.6-mm.orig/drivers/input/serio/serio.c 2006-09-26 10:25:22.000000000 +0200
+++ linux-2.6-mm/drivers/input/serio/serio.c 2006-09-26 10:34:04.000000000 +0200
@@ -521,6 +521,8 @@ static void serio_release_port(struct de
module_put(THIS_MODULE);
}

+static struct lock_class_key serio_lock_key;
+
/*
* Prepare serio port for registration.
*/
@@ -538,8 +540,13 @@ static void serio_init_port(struct serio
"serio%ld", (long)atomic_inc_return(&serio_no) - 1);
serio->dev.bus = &serio_bus;
serio->dev.release = serio_release_port;
- if (serio->parent)
+ if (serio->parent) {
serio->dev.parent = &serio->parent->dev;
+ serio->depth = serio->parent->depth + 1;
+ } else
+ serio->depth = 0;
+ lockdep_set_class_and_subclass(&serio->lock, &serio_lock_key,
+ serio->depth);
}

/*
Index: linux-2.6-mm/include/linux/serio.h
===================================================================
--- linux-2.6-mm.orig/include/linux/serio.h 2006-09-26 10:25:22.000000000 +0200
+++ linux-2.6-mm/include/linux/serio.h 2006-09-26 10:34:04.000000000 +0200
@@ -41,6 +41,7 @@ struct serio {
void (*stop)(struct serio *);

struct serio *parent, *child;
+ unsigned int depth; /* level of nesting in serio hierarchy */

struct serio_driver *drv; /* accessed from interrupt, must be protected by serio->lock and serio->sem */
struct mutex drv_mutex; /* protects serio->drv so attributes can pin driver */

--


2006-09-26 13:22:31

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH 2/2] serio: lockdep annotation for ps2dev->cmd_mutex and serio->lock

On Tue, 26 Sep 2006, Peter Zijlstra wrote:

> Based ideas from Jiri Kosina, this patch tracks the nesting depth
> and uses the new lockdep_set_class_and_subclass() annotation to store
> this information in the lock objects.

Hi,

the lockdep part of the patch (1/2) is OK. The second part, specifically
the libps2.c changes, are not complete - the originally (wrongly)
introduced mutex_lock_nested() has to be changed back to mutex_lock(),
otherwise we will get spurious warning from lockdep about ps2_mutex_key.

Below is the fixed version of the patch. I confirm that this (together
with Peter's original changes in lockdep, already acked by Ingo) fixes the
synpatics passthrough port lockdep warnings.

So, as long as you, Dmitry, seem to be convenient with this approach,
please apply. Thanks.

Signed-off-by: Jiri Kosina <[email protected]>

Index: linux-2.6-mm/drivers/input/serio/libps2.c
===================================================================
--- linux-2.6.18-rc6-mm2.orig/drivers/input/serio/libps2.c 2006-09-04 04:19:48.000000000 +0200
+++ linux-2.6.18-rc6-mm2/drivers/input/serio/libps2.c 2006-09-26 14:45:18.000000000 +0200
@@ -182,7 +182,7 @@ int ps2_command(struct ps2dev *ps2dev, u
return -1;
}

- mutex_lock_nested(&ps2dev->cmd_mutex, SINGLE_DEPTH_NESTING);
+ mutex_lock(&ps2dev->cmd_mutex);

serio_pause_rx(ps2dev->serio);
ps2dev->flags = command == PS2_CMD_GETID ? PS2_FLAG_WAITID : 0;
@@ -280,6 +280,8 @@ int ps2_schedule_command(struct ps2dev *
return 0;
}

+static struct lock_class_key ps2_mutex_key;
+
/*
* ps2_init() initializes ps2dev structure
*/
@@ -287,6 +289,8 @@ int ps2_schedule_command(struct ps2dev *
void ps2_init(struct ps2dev *ps2dev, struct serio *serio)
{
mutex_init(&ps2dev->cmd_mutex);
+ lockdep_set_class_and_subclass(&ps2dev->cmd_mutex, &ps2_mutex_key,
+ serio->depth);
init_waitqueue_head(&ps2dev->wait);
ps2dev->serio = serio;
}
Index: linux-2.6-mm/drivers/input/serio/serio.c
===================================================================
--- linux-2.6-mm.orig/drivers/input/serio/serio.c 2006-09-26 10:25:22.000000000 +0200
+++ linux-2.6-mm/drivers/input/serio/serio.c 2006-09-26 10:34:04.000000000 +0200
@@ -521,6 +521,8 @@ static void serio_release_port(struct de
module_put(THIS_MODULE);
}

+static struct lock_class_key serio_lock_key;
+
/*
* Prepare serio port for registration.
*/
@@ -538,8 +540,13 @@ static void serio_init_port(struct serio
"serio%ld", (long)atomic_inc_return(&serio_no) - 1);
serio->dev.bus = &serio_bus;
serio->dev.release = serio_release_port;
- if (serio->parent)
+ if (serio->parent) {
serio->dev.parent = &serio->parent->dev;
+ serio->depth = serio->parent->depth + 1;
+ } else
+ serio->depth = 0;
+ lockdep_set_class_and_subclass(&serio->lock, &serio_lock_key,
+ serio->depth);
}

/*
Index: linux-2.6-mm/include/linux/serio.h
===================================================================
--- linux-2.6-mm.orig/include/linux/serio.h 2006-09-26 10:25:22.000000000 +0200
+++ linux-2.6-mm/include/linux/serio.h 2006-09-26 10:34:04.000000000 +0200
@@ -41,6 +41,7 @@ struct serio {
void (*stop)(struct serio *);

struct serio *parent, *child;
+ unsigned int depth; /* level of nesting in serio hierarchy */

struct serio_driver *drv; /* accessed from interrupt, must be protected by serio->lock and serio->sem */
struct mutex drv_mutex; /* protects serio->drv so attributes can pin driver */


--
Jiri Kosina

2006-09-26 13:36:45

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 2/2] serio: lockdep annotation for ps2dev->cmd_mutex and serio->lock

On Tue, 2006-09-26 at 15:21 +0200, Jiri Kosina wrote:

> the lockdep part of the patch (1/2) is OK. The second part, specifically
> the libps2.c changes, are not complete - the originally (wrongly)
> introduced mutex_lock_nested() has to be changed back to mutex_lock(),
> otherwise we will get spurious warning from lockdep about ps2_mutex_key.

Ah, missed that that part was actually in the tree ;-)

> Below is the fixed version of the patch. I confirm that this (together
> with Peter's original changes in lockdep, already acked by Ingo) fixes the
> synpatics passthrough port lockdep warnings.
>
> So, as long as you, Dmitry, seem to be convenient with this approach,
> please apply. Thanks.
>
> Signed-off-by: Jiri Kosina <[email protected]>
Acked-by: Peter Zijlstra <[email protected]>

Thanks for testing



2006-09-26 13:38:36

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/2] serio: lockdep annotation for ps2dev->cmd_mutex and serio->lock


* Peter Zijlstra <[email protected]> wrote:

> > Below is the fixed version of the patch. I confirm that this
> > (together with Peter's original changes in lockdep, already acked by
> > Ingo) fixes the synpatics passthrough port lockdep warnings.
> >
> > So, as long as you, Dmitry, seem to be convenient with this
> > approach, please apply. Thanks.
> >
> > Signed-off-by: Jiri Kosina <[email protected]>
> Acked-by: Peter Zijlstra <[email protected]>

great. Re-ack:

Acked-by: Ingo Molnar <[email protected]>

Ingo