Re: [PATCH 2/2] serio: lockdep annotation for ps2dev->cmd_mutex andserio->lock

[Jiri Kosina]

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 <jikos@xxxxxxxx>

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/