Messages in this thread | | | Date | Mon, 24 Mar 2014 07:50:38 +0100 (CET) | From | Julia Lawall <> | Subject | Re: [patch 00/16] timers: Plug debugobject leaks and use del_timer_sync() in exit/teardown |
| |
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
| |