All of lore.kernel.org
 help / color / mirror / Atom feed
From: Julien Grall <julien.grall@arm.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-rt-users@vger.kernel.org, linux-kernel@vger.kernel.org,
	maz@kernel.org, bigeasy@linutronix.de, rostedt@goodmis.org
Subject: Re: [RT PATCH 3/3] hrtimer: Prevent using uninitialized spin_lock in hrtimer_grab_expiry_lock()
Date: Thu, 22 Aug 2019 11:59:10 +0100	[thread overview]
Message-ID: <6f637e70-9a7b-47fd-08b0-82b6494d3ae1@arm.com> (raw)
In-Reply-To: <alpine.DEB.2.21.1908211557420.2223@nanos.tec.linutronix.de>

Hi Thomas,

Thank you for the review.

On 21/08/2019 15:02, Thomas Gleixner wrote:
> On Wed, 21 Aug 2019, Julien Grall wrote:
> 
>> migration_base is used as a placeholder when an hrtimer is switching
>> between base (see switch_hrtimer_timer_base). It is possible
>> theoritically possible to have timer->base equal to migration_base.
>>
>> Even if it is a placeholder, it would pass all the current check in
>> hrtimer_grab_expiry_lock() leading to use softirq_expiry_lock
>> uninitialized.
>>
>> This is can be prevented by checking whether the base is equal to
>> the placeholder (i.e. migration_base).
> 
> That's a lame argument. The point is that it does not make sense to do that
> on migration base, but not for the reason you are giving (uninitialized
> lock).

Fair point, I will update the commit message.

> 
> If base == migration_base then there is no point to lock soft_expiry_lock
> simply because the timer is not executing the callback in soft irq context
> and the whole lock/unlock dance can be avoided.
> 
> But, yes. Good catch.

Do you want me to resend the series or can I just provide an update to the 
commit message here?

Cheers,

-- 
Julien Grall

  reply	other threads:[~2019-08-22 10:59 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-21  9:24 [RT PATCH 0/3] hrtimer: RT fixes for hrtimer_grab_expiry_lock() Julien Grall
2019-08-21  9:24 ` [RT PATCH 1/3] hrtimer: Use READ_ONCE to access timer->base in hrimer_grab_expiry_lock() Julien Grall
2019-08-21 13:44   ` Sebastian Andrzej Siewior
2019-08-21 13:50     ` Thomas Gleixner
2019-08-21 13:59       ` Sebastian Andrzej Siewior
2019-09-26 21:47       ` Eric Dumazet
2019-08-23  2:12   ` [tip: timers/core] hrtimer: Protect lockless access to timer->base tip-bot2 for Julien Grall
2019-08-21  9:24 ` [RT PATCH 2/3] hrtimer: Don't grab the expiry lock for non-soft hrtimer Julien Grall
2019-08-21 13:49   ` Sebastian Andrzej Siewior
2019-08-21  9:24 ` [RT PATCH 3/3] hrtimer: Prevent using uninitialized spin_lock in hrtimer_grab_expiry_lock() Julien Grall
2019-08-21 14:02   ` Thomas Gleixner
2019-08-22 10:59     ` Julien Grall [this message]
2019-08-23  2:12   ` [tip: timers/core] hrtimer: Don't take expiry_lock when timer is currently migrated tip-bot2 for Julien Grall
2019-09-04 14:15     ` [PATCH] hrtimer: Add a missing bracket and hide `migration_base' on !SMP Sebastian Andrzej Siewior
2019-09-04 14:39       ` Thomas Gleixner
2019-09-04 14:55         ` [PATCH v2] " Sebastian Andrzej Siewior

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6f637e70-9a7b-47fd-08b0-82b6494d3ae1@arm.com \
    --to=julien.grall@arm.com \
    --cc=bigeasy@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.