lkml.org 
[lkml]   [2010]   [Dec]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] fix unsafe operation in high resolution timer
    On Fri, Dec 24, 2010 at 10:28:52PM +0800, Hillf Danton wrote:
    > On Fri, Dec 24, 2010 at 3:17 PM, Yong Zhang <yong.zhang0@gmail.com> wrote:
    > > On Thu, Dec 23, 2010 at 9:29 PM, Hillf Danton <dhillf@gmail.com> wrote:
    > >> After calling the callback function of hrtimer, the timer could become
    > >> unreliable in corner cases where the timer will no longer be queued
    > >> and the mm segment, in which the timer is embedded, could be reclaimed
    > >> in the callback.
    > >>
    > >> The unreliability is fixed by checking the result of callback before
    > >> operating the timer again.
    > >
    > > Though the patch is buggy. But it actually explores a real problem.
    > >
    >
    > Would you please finger out why the patch is buggy?

    No problem.

    Actually the patch change the behavior of current hrtimer.
    See comments below :)

    > >> ---
    > >>
    > >> --- a/kernel/hrtimer.c  2010-11-01 19:54:12.000000000 +0800
    > >> +++ b/kernel/hrtimer.c  2010-12-23 21:17:02.000000000 +0800
    > >> @@ -1225,6 +1225,7 @@ static void __run_hrtimer(struct hrtimer
    > >>        raw_spin_unlock(&cpu_base->lock);
    > >>        trace_hrtimer_expire_entry(timer, now);
    > >>        restart = fn(timer);
    > >> +       if (restart != HRTIMER_NORESTART)
    > >>        trace_hrtimer_expire_exit(timer);
    > >>        raw_spin_lock(&cpu_base->lock);
    > >>
    > >> @@ -1236,11 +1237,8 @@ static void __run_hrtimer(struct hrtimer
    > >>        if (restart != HRTIMER_NORESTART) {
    > >>                BUG_ON(timer->state != HRTIMER_STATE_CALLBACK);
    > >>                enqueue_hrtimer(timer, base);
    > >> +               timer->state &= ~HRTIMER_STATE_CALLBACK;

    HRTIMER_STATE_CALLBACK is only cleared for RESTART hrtimer with
    your modification.

    > >>        }
    > >> -
    > >> -       WARN_ON_ONCE(!(timer->state & HRTIMER_STATE_CALLBACK));
    > >> -
    > >> -       timer->state &= ~HRTIMER_STATE_CALLBACK;

    But for a hrtimer which is not free in its callback, like a
    static defined one. the hrtimer could be referenced at the same
    time. So here you cann't just delete the two lines.

    Thanks,
    Yong
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2010-12-25 03:15    [W:0.024 / U:241.844 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site