Re: hrtimer possible issue

Re: hrtimer possible issue

[Thomas Gleixner]

On Sun, 3 Feb 2013, Izik Eidus wrote:

> Hi,
> 
> it seems like hrtimer_enqueue_reprogram contain a race which could result in
> timer.base switch during unlock/lock sequence.
> 
> See the code at __hrtimer_start_range_ns where it calls
> hrtimer_enqueue_reprogram. The later is releasing lock protecting the timer
> base for a short time and timer base switch can occur from a different CPU
> thread. Later when __hrtimer_start_range_ns calls unlock_hrtimer_base, a base
> switch could have happened and this causes the bug
> 
> Try to start the same hrtimer from two different threads in kernel running
> each one on a different CPU. Eventually one of the calls will cause timer base
> switch while another thread is not expecting it.
> 
> This can happen in virtualized environment where one thread can be delayed by
> lower hypervisor, and due to time delay a different CPU is taking care of
> missed timer start and runs the timer start logic on its own.

Nice analysis.
 
> This simple patch (just to give example of a fix) refactor this function to
> get rid of unneeded lock which immediately was followed by the unlock (with
> possible undesired base switch).
> 
> (Both the bug and the fixed were found/patched by Leonid Shatz)

The patch got mangled by your mail client and it is missing the proper
Signed-off-by annotation in the patch description. See
Documentation/SubmittingPatches.

Can you please resend ?

Thanks,

	tglx

RE: hrtimer possible issue

[Leonid Shatz]

I assume the race can also happen between hrtimer cancel and start. In both
cases timer base switch can happen.

Izik, please check if you can arrange the patch in the standard format (do
we need to do it against latest kernel version?)

Leonid Shatz

> -----Original Message-----
> From: Thomas Gleixner [mailto:tglx@linutronix.de]
> Sent: Monday, February 04, 2013 12:13 PM
> To: Izik Eidus
> Cc: linux-kernel@vger.kernel.org; Andrea Arcangeli; Leonid Shatz
> Subject: Re: hrtimer possible issue
> 
> On Sun, 3 Feb 2013, Izik Eidus wrote:
> 
> > Hi,
> >
> > it seems like hrtimer_enqueue_reprogram contain a race which could
> > result in timer.base switch during unlock/lock sequence.
> >
> > See the code at __hrtimer_start_range_ns where it calls
> > hrtimer_enqueue_reprogram. The later is releasing lock protecting the
> > timer base for a short time and timer base switch can occur from a
> > different CPU thread. Later when __hrtimer_start_range_ns calls
> > unlock_hrtimer_base, a base switch could have happened and this causes
> > the bug
> >
> > Try to start the same hrtimer from two different threads in kernel
> > running each one on a different CPU. Eventually one of the calls will
> > cause timer base switch while another thread is not expecting it.
> >
> > This can happen in virtualized environment where one thread can be
> > delayed by lower hypervisor, and due to time delay a different CPU is
> > taking care of missed timer start and runs the timer start logic on its
own.
> 
> Nice analysis.
> 
> > This simple patch (just to give example of a fix) refactor this
> > function to get rid of unneeded lock which immediately was followed by
> > the unlock (with possible undesired base switch).
> >
> > (Both the bug and the fixed were found/patched by Leonid Shatz)
> 
> The patch got mangled by your mail client and it is missing the proper
Signed-
> off-by annotation in the patch description. See
> Documentation/SubmittingPatches.
> 
> Can you please resend ?
> 
> Thanks,
> 
> 	tglx
> 

RE: hrtimer possible issue

[Thomas Gleixner]

On Mon, 4 Feb 2013, Leonid Shatz wrote:

> I assume the race can also happen between hrtimer cancel and start. In both
> cases timer base switch can happen.
> 
> Izik, please check if you can arrange the patch in the standard format (do
> we need to do it against latest kernel version?)

Yes please.

Thanks,

	tglx

Re: hrtimer possible issue

[Izik Eidus]

On 02/05/2013 12:20 PM, Thomas Gleixner wrote:
> On Mon, 4 Feb 2013, Leonid Shatz wrote:
>
>> I assume the race can also happen between hrtimer cancel and start. In both
>> cases timer base switch can happen.
>>
>> Izik, please check if you can arrange the patch in the standard format (do
>> we need to do it against latest kernel version?)
> Yes please.

But I sent it already yasterday with git-send-email and checkpatch.pl :-)

> Thanks,
>
> 	tglx

Re: hrtimer possible issue

[Mike Rapoport]

Thomas,

On Tue, Feb 5, 2013 at 12:20 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Mon, 4 Feb 2013, Leonid Shatz wrote:
>
>> I assume the race can also happen between hrtimer cancel and start. In both
>> cases timer base switch can happen.
>>
>> Izik, please check if you can arrange the patch in the standard format (do
>> we need to do it against latest kernel version?)
>
> Yes please.

The patch is here: https://patchwork.kernel.org/patch/2091501/

> Thanks,
>
>         tglx

-- 
Sincerely yours,
Mike.