On Tue, Apr 18, 2023 at 05:43:15PM +0200, Sebastian Andrzej Siewior wrote:
> From: Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx>
>
> commit 1c0908d8e441631f5b8ba433523cf39339ee2ba0 upstream.
>
> Jan Kara reported the following bug triggering on 6.0.5-rt14 running dbench
> on XFS on arm64.
>
> kernel BUG at fs/inode.c:625!
> Internal error: Oops - BUG: 0 [#1] PREEMPT_RT SMP
> CPU: 11 PID: 6611 Comm: dbench Tainted: G E 6.0.0-rt14-rt+ #1
> pc : clear_inode+0xa0/0xc0
> lr : clear_inode+0x38/0xc0
> Call trace:
> clear_inode+0xa0/0xc0
> evict+0x160/0x180
> iput+0x154/0x240
> do_unlinkat+0x184/0x300
> __arm64_sys_unlinkat+0x48/0xc0
> el0_svc_common.constprop.4+0xe4/0x2c0
> do_el0_svc+0xac/0x100
> el0_svc+0x78/0x200
> el0t_64_sync_handler+0x9c/0xc0
> el0t_64_sync+0x19c/0x1a0
>
> It also affects 6.1-rc7-rt5 and affects a preempt-rt fork of 5.14 so this
> is likely a bug that existed forever and only became visible when ARM
> support was added to preempt-rt. The same problem does not occur on x86-64
> and he also reported that converting sb->s_inode_wblist_lock to
> raw_spinlock_t makes the problem disappear indicating that the RT spinlock
> variant is the problem.
>
> Which in turn means that RT mutexes on ARM64 and any other weakly ordered
> architecture are affected by this independent of RT.
>
> Will Deacon observed:
>
> "I'd be more inclined to be suspicious of the slowpath tbh, as we need to
> make sure that we have acquire semantics on all paths where the lock can
> be taken. Looking at the rtmutex code, this really isn't obvious to me
> -- for example, try_to_take_rt_mutex() appears to be able to return via
> the 'takeit' label without acquire semantics and it looks like we might
> be relying on the caller's subsequent _unlock_ of the wait_lock for
> ordering, but that will give us release semantics which aren't correct."
>
> Sebastian Andrzej Siewior prototyped a fix that does work based on that
> comment but it was a little bit overkill and added some fences that should
> not be necessary.
>
> The lock owner is updated with an IRQ-safe raw spinlock held, but the
> spin_unlock does not provide acquire semantics which are needed when
> acquiring a mutex.
>
> Adds the necessary acquire semantics for lock owner updates in the slow path
> acquisition and the waiter bit logic.
>
> It successfully completed 10 iterations of the dbench workload while the
> vanilla kernel fails on the first iteration.
>
> [ bigeasy@xxxxxxxxxxxxx: Initial prototype fix ]
>
> Fixes: 700318d1d7b38 ("locking/rtmutex: Use acquire/release semantics")
> Fixes: 23f78d4a03c5 ("[PATCH] pi-futex: rt mutex core")
> Reported-by: Jan Kara <jack@xxxxxxx>
> Signed-off-by: Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Link: https://lore.kernel.org/r/20221202100223.6mevpbl7i6x5udfd@xxxxxxxxxxxxxxxxxxx
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---
>
> Could this be please backported to 5.15 and earlier? It is already part
> of the 6.X kernels. I asked about this by the end of January and I'm
> kindly asking again ;)
I thought this was only an issues when using the out-of-tree RT patches
with these kernels, right? Or is it relevant for 5.15.y from kernel.org
without anything else?
> This patch applies against v5.15. Should it not apply to earlier
> versions, please let me know an I kindly provide a backport.
How far back should it go?
> I received reports that this fixes "mysterious" crashes and that is how
> I noticed that it is not part of the earlier kernels.
Again, isn't this only needed for -rt kernels?
thanks,
greg k-h
