From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752675Ab1KHJHq (ORCPT ); Tue, 8 Nov 2011 04:07:46 -0500 Received: from mail-ww0-f44.google.com ([74.125.82.44]:48524 "EHLO mail-ww0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750710Ab1KHJHn (ORCPT ); Tue, 8 Nov 2011 04:07:43 -0500 Date: Tue, 8 Nov 2011 17:07:35 +0800 From: Yong Zhang To: Peter Zijlstra Cc: Vegard Nossum , linux-kernel@vger.kernel.org, sergey.senozhatsky@gmail.com, bp@alien8.de, Ingo Molnar , Tejun Heo , David Rientjes , casteyde.christian@free.fr Subject: Re: [PATCH 1/4] lockdep: lock_set_subclass() fix Message-ID: <20111108090735.GA4038@zhy> Reply-To: Yong Zhang References: <1320398790-21663-1-git-send-email-yong.zhang0@gmail.com> <1320398790-21663-2-git-send-email-yong.zhang0@gmail.com> <1320669279.18053.29.camel@twins> <1320682230.17809.11.camel@twins> <20111108025847.GB11439@zhy> <1320739015.2244.0.camel@twins> <20111108081433.GA3746@zhy> <1320741980.2244.4.camel@twins> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1320741980.2244.4.camel@twins> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Nov 08, 2011 at 09:46:20AM +0100, Peter Zijlstra wrote: > On Tue, 2011-11-08 at 16:14 +0800, Yong Zhang wrote: > > > > But how do we deal with ->class_cache? Always set it in > > loop_up_lock_class()? > > Hrm.. good point, aside from that there's another problem as well, I > think we can deal with the cache being NULL, but is memset() an atomic > write? If not a read could observe an intermediate state and go funny. Yup. > > I'm tempted to go with the pure kmemcheck_mark_initialized() thing for > now. me too :) So something like below? Thanks, Yong --- From: Yong Zhang Subject: [PATCH] lockdep: kmemcheck: annotate ->lock in lockdep_init_map() Since commit f59de89 [lockdep: Clear whole lockdep_map on initialization], lockdep_init_map() will clear all the struct. But it will break lock_set_class()/lock_set_subclass(). A typical race condition is like below: CPU A CPU B lock_set_subclass(lockA); lock_set_class(lockA); lockdep_init_map(lockA); /* lockA->name is cleared */ memset(lockA); __lock_acquire(lockA); /* lockA->class_cache[] is cleared */ register_lock_class(lockA); look_up_lock_class(lockA); WARN_ON_ONCE(class->name != lock->name); lock->name = name; So annotate ->lock with kmemcheck_mark_initialized() to cure this problem and the one reported in commit f59de89. Reported-by: Sergey Senozhatsky Reported-by: Borislav Petkov Suggested-by: Vegard Nossum Signed-off-by: Yong Zhang Cc: Peter Zijlstra Cc: Ingo Molnar Cc: Tejun Heo Cc: David Rientjes --- kernel/lockdep.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/kernel/lockdep.c b/kernel/lockdep.c index e69434b..08a2b1b 100644 --- a/kernel/lockdep.c +++ b/kernel/lockdep.c @@ -2948,7 +2948,7 @@ 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, sizeof(*lock)); #ifdef CONFIG_LOCK_STAT lock->cpu = raw_smp_processor_id(); -- 1.7.5.4