public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Vegard Nossum <vegard.nossum@gmail.com>,
	Yong Zhang <yong.zhang0@gmail.com>,
	linux-kernel@vger.kernel.org, sergey.senozhatsky@gmail.com,
	bp@alien8.de, Ingo Molnar <mingo@elte.hu>,
	David Rientjes <rientjes@google.com>,
	casteyde.christian@free.fr
Subject: Re: [PATCH 1/4] lockdep: lock_set_subclass() fix
Date: Mon, 7 Nov 2011 08:21:59 -0800	[thread overview]
Message-ID: <20111107162159.GC13699@google.com> (raw)
In-Reply-To: <1320682230.17809.11.camel@twins>

Hello,

On Mon, Nov 07, 2011 at 05:10:29PM +0100, Peter Zijlstra wrote:
> We could move the key and name pointer to the start of the structure and
> memset everything after that, however wouldn't that leave kmemcheck with
> the same problem? It wouldn't know those two pointers would be
> initialized properly.

At that point, lockdep_map is guaranteed to have passed through
lockdep_init_map(), so I think it should be fine.

> @@ -148,9 +148,9 @@ void clear_lock_stats(struct lock_class *class);
>   * This is embedded into specific lock instances:
>   */
>  struct lockdep_map {
> +	const char			*name;
>  	struct lock_class_key		*key;
>  	struct lock_class		*class_cache[NR_LOCKDEP_CACHING_CLASSES];
> -	const char			*name;
>  #ifdef CONFIG_LOCK_STAT
>  	int				cpu;
>  	unsigned long			ip;

Probably fat comment explaining the ordering requirement here w/

#define LOCKDEP_MAP_INIT_OFFSET		offsetof(struct lockdep_map, class_cache)

> diff --git a/kernel/lockdep.c b/kernel/lockdep.c
> index e69434b..81855cf 100644
> --- a/kernel/lockdep.c
> +++ b/kernel/lockdep.c
> @@ -2948,7 +2948,8 @@ static int mark_lock(struct task_struct *curr, struct held_lock *this,
>  void lockdep_init_map(struct lockdep_map *lock, const char *name,
>  		      struct lock_class_key *key, int subclass)
>  {
> -	memset(lock, 0, sizeof(*lock));
> +	kmemcheck_mark_initialized(lock, 2*sizeof(void *));
> +	memset(&lock->class_cache[0], 0, sizeof(*lock)-2*sizeof(void *));

And something like the following?

	memset((void *)lock + LOCKDEP_MAP_INIT_OFFSET, 0,
		sizeof(*lock) - LOCKDEP_MAP_INIT_OFFSET);

Thanks.

-- 
tejun

  reply	other threads:[~2011-11-07 16:22 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-04  9:26 [PATCH 0/4] patches to cure race in lock_set_class() Yong Zhang
2011-11-04  9:26 ` [PATCH 1/4] lockdep: lock_set_subclass() fix Yong Zhang
2011-11-07 12:34   ` Peter Zijlstra
2011-11-07 13:31     ` Yong Zhang
2011-11-07 14:03       ` Tejun Heo
2011-11-07 13:54     ` Borislav Petkov
2011-11-07 15:28     ` Vegard Nossum
2011-11-07 16:10       ` Peter Zijlstra
2011-11-07 16:21         ` Tejun Heo [this message]
2011-11-07 16:26           ` Peter Zijlstra
2011-11-08  2:58         ` Yong Zhang
2011-11-08  3:02           ` Yong Zhang
2011-11-08  7:56           ` Peter Zijlstra
2011-11-08  8:14             ` Yong Zhang
2011-11-08  8:46               ` Peter Zijlstra
2011-11-08  9:07                 ` Yong Zhang
2011-11-08  9:37                   ` Yong Zhang
2011-11-08  9:40                   ` Peter Zijlstra
2011-11-09  8:04                     ` [PATCH 1/2] lockdep: kmemcheck: annotate ->lock in lockdep_init_map() Yong Zhang
2011-11-09  8:07                       ` [PATCH 2/2] lockdep: always try to set ->class_cache in register_lock_class() lockdep_init_map() Yong Zhang
2011-11-18 23:39                         ` [tip:core/locking] lockdep: Always " tip-bot for Yong Zhang
2011-12-06  9:39                       ` [tip:core/locking] lockdep, kmemcheck: Annotate ->lock in lockdep_init_map() tip-bot for Yong Zhang
2011-12-06 19:56                         ` David Rientjes
2011-12-06 20:14                       ` [tip:perf/urgent] " tip-bot for Yong Zhang
2011-11-08  2:22       ` [PATCH 1/4] lockdep: lock_set_subclass() fix Yong Zhang
2011-11-04  9:26 ` [RFC PATCH 2/4] lockdep: Let register_lock_class() can be called with/without graph_lock Yong Zhang
2011-11-04  9:26 ` [RFC PATCH 3/4] lockdep: split lockdep_init_map() Yong Zhang
2011-11-04  9:26 ` [RFC PATCH 4/4] lockdep: fix race condition in __lock_set_class() Yong Zhang
2011-11-07 12:30   ` Peter Zijlstra
2011-11-07 13:26     ` Yong Zhang
2011-11-06 11:52 ` [PATCH 0/4] patches to cure race in lock_set_class() Borislav Petkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111107162159.GC13699@google.com \
    --to=tj@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=bp@alien8.de \
    --cc=casteyde.christian@free.fr \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=rientjes@google.com \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=vegard.nossum@gmail.com \
    --cc=yong.zhang0@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox