From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752223AbaCXGum (ORCPT ); Mon, 24 Mar 2014 02:50:42 -0400 Received: from mail2-relais-roc.national.inria.fr ([192.134.164.83]:53767 "EHLO mail2-relais-roc.national.inria.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750810AbaCXGuk (ORCPT ); Mon, 24 Mar 2014 02:50:40 -0400 X-IronPort-AV: E=Sophos;i="4.97,719,1389740400"; d="scan'208";a="64371805" Date: Mon, 24 Mar 2014 07:50:38 +0100 (CET) From: Julia Lawall X-X-Sender: jll@localhost6.localdomain6 To: Thomas Gleixner cc: LKML , Andrew Morton Subject: Re: [patch 00/16] timers: Plug debugobject leaks and use del_timer_sync() in exit/teardown In-Reply-To: Message-ID: References: <20140323150557.288925975@linutronix.de> User-Agent: Alpine 2.02 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 24 Mar 2014, Thomas Gleixner wrote: > On Sun, 23 Mar 2014, Julia Lawall wrote: > > As far as I could tell, (part of) the issue is that any kind of exit or > > close function should use del_timer_sync, because they could be called > > from a different CPU than the one that started up the timer? > > > > Here is a semantic patch that takes care of the case of simple module_exit > > functions: > > > > @r@ > > declarer name module_exit; > > identifier ex; > > @@ > > > > module_exit(ex); > > > > @@ > > identifier r.ex; > > @@ > > > > ex(...) { > > <... > > - del_timer > > + del_timer_sync > > (...) > > ...> > > } > > > > The transformations it makes are below. I haven't had a chance to check > > which results overlap with what Thomas has already sent, but I could look > > Minimal overlap, but as I said I did just a few conversions. > > > into it if this is the right idea. I guess other kinds of close/exit > > functions would have to be identified manually, to make more rules. > > If you look through the examples I sent, you'll find the close() > callbacks of various devices. So everything which is a function > pointer to a ops->close(), ops->remove(), ops_>teardown() is dangerous > if using del_timer(). There are a few exceptions, but.... > > Another thing I saw is > > del_timer(&bla->timer); > .... > kfree(&bla); > > That's also a potential source of trouble. You can't tell without > analyzing the code, whether it's a problem or not. But making the > responsible people to look at it is definitely a good thing. > > > In what circumstance can one be sure that two instructions are executed on > > the same CPU? > > If interrupts or preemption are disabled. But that's not the issue at > hand. > > The del_timer vs. del_timer_sync problem is: > > CPU0 CPU1 > > add_timer(&bla->timer); > > close(bla) > timer expires del_timer(&bla->timer); > callback is invoked > kfree(bla); > derefernces bla > > I'll look at your findings on Tuesday, but feel free to send them to > the relevant maintainers for review. Thanks for all of the suggestions! julia