lkml.org 
[lkml]   [2014]   [Mar]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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


\
 
 \ /
  Last update: 2014-03-24 08:41    [W:0.090 / U:0.000 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site