linux-kernel - Re: [PATCH v1] timer: Forward timer base before migrating timers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.20.1801181141200.1847@nanos>
Date:   Thu, 18 Jan 2018 11:45:42 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Lingutla Chandrasekhar <clingutla@...eaurora.org>
cc:     linux-kernel@...r.kernel.org, neeraju@...eaurora.org,
        linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v1] timer: Forward timer base before migrating timers

On Wed, 17 Jan 2018, Lingutla Chandrasekhar wrote:

> In case when timers are migrated to a CPU, after it exits
> idle, but before timer base is forwarded, either from
> run_timer_softirq()/mod_timer()/add_timer_on(), it's
> possible that migrated timers are queued, based on older
> clock value. This can cause delays in handling those timers.
> 
> For example, consider below sequence of events:
> 
> - CPU0 timer1 expires = 59969 and base->clk = 59131. So,
>   timer is queued at level 2, with next expiry for this timer
>   = 60032 (due to granularity addition).
> - CPU1 enters idle @60007, with next timer expiry @60020.
> - CPU1 exits idle.
> - CPU0 is hotplugged at 60009, and timers are migrated to
>   CPU1, with new base->clk = 60007. timer1 is queued,
>   based on 60007 at level 0, for immediate handling (in
>   next timer softirq handling).
> - CPU1's base->clk is forwarded to 60009, so, in next sched
>   timer interrupt, timer1 is not handled.
> 
> The issue happens as timer wheel collects expired timers
> starting from the current clk's index onwards, but migrated
> timers, if enqueued, based on older clk value can result
> in their index being less than clk's current index.
> This can only happen if new base->clk is ahead of
> timer->expires, resulting in timer being queued at
> new base->clk's current index.
> 
> Signed-off-by: Lingutla Chandrasekhar <clingutla@...eaurora.org>
> Signed-off-by: Neeraj Upadhyay <neeraju@...eaurora.org>

That's still wrong.

The SOB chain should be:

    SOB:  Author
    SOB:  Handler/Transporter
    SOB:  Committer

So if you wrote te patch then Neeraj's SOB is wrong. If Neeraj wrote the
patch then the patch should contain a

From: Neeraj

line right at the top of the changelog.

If you both developed the patch then please use;

   Co-developed-by: Neeraj
   Signed-off-by:   Neeraj
   Signed-off-by;   You

That would make you the Author in the git commit, but still preserve the
co-authorship.

Please clarify what applies to this particular patch.

Thanks,

	tglx