[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3f86d58e-7f36-c6b4-c43a-2a7bcffd3bd@linux-m68k.org>
Date: Tue, 29 Aug 2023 18:42:48 +0200 (CEST)
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: "Liam R. Howlett" <Liam.Howlett@...cle.com>
cc: Andrew Morton <akpm@...ux-foundation.org>,
maple-tree@...ts.infradead.org, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, stable@...r.kernel.org,
linux-renesas-soc@...r.kernel.org
Subject: Re: [PATCH v2 1/2] maple_tree: Disable mas_wr_append() when other
readers are possible
Hi Liam,
On Fri, 18 Aug 2023, Liam R. Howlett wrote:
> The current implementation of append may cause duplicate data and/or
> incorrect ranges to be returned to a reader during an update. Although
> this has not been reported or seen, disable the append write operation
> while the tree is in rcu mode out of an abundance of caution.
>
> During the analysis of the mas_next_slot() the following was
> artificially created by separating the writer and reader code:
>
> Writer: reader:
> mas_wr_append
> set end pivot
> updates end metata
> Detects write to last slot
> last slot write is to start of slot
> store current contents in slot
> overwrite old end pivot
> mas_next_slot():
> read end metadata
> read old end pivot
> return with incorrect range
> store new value
>
> Alternatively:
>
> Writer: reader:
> mas_wr_append
> set end pivot
> updates end metata
> Detects write to last slot
> last lost write to end of slot
> store value
> mas_next_slot():
> read end metadata
> read old end pivot
> read new end pivot
> return with incorrect range
> set old end pivot
>
> There may be other accesses that are not safe since we are now updating
> both metadata and pointers, so disabling append if there could be rcu
> readers is the safest action.
>
> Fixes: 54a611b60590 ("Maple Tree: add new data structure")
> Cc: stable@...r.kernel.org
> Signed-off-by: Liam R. Howlett <Liam.Howlett@...cle.com>
Thanks for your patch, which is now commit cfeb6ae8bcb96ccf
("maple_tree: disable mas_wr_append() when other readers are
possible") in v6.5, and is being backported to stable.
On Renesas RZ/A1 and RZ/A2 (single-core Cortex-A9), this causes the
following warning:
clocksource: timer@...3b000: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 28958491609 ns
sched_clock: 32 bits at 66MHz, resolution 15ns, wraps every 32537631224ns
/soc/timer@...3b000: used for clocksource
/soc/timer@...3c000: used for clock events
+------------[ cut here ]------------
+WARNING: CPU: 0 PID: 0 at init/main.c:992 start_kernel+0x2f0/0x480
+Interrupts were enabled early
+CPU: 0 PID: 0 Comm: swapper Not tainted 6.5.0-rza2mevb-10197-g99b80d6b92b5 #237
+Hardware name: Generic R7S9210 (Flattened Device Tree)
+ unwind_backtrace from show_stack+0x10/0x14
+ show_stack from dump_stack_lvl+0x24/0x3c
+ dump_stack_lvl from __warn+0x74/0xb8
+ __warn from warn_slowpath_fmt+0x78/0xb0
+ warn_slowpath_fmt from start_kernel+0x2f0/0x480
+ start_kernel from 0x0
+---[ end trace 0000000000000000 ]---
Console: colour dummy device 80x30
printk: console [tty0] enabled
Calibrating delay loop (skipped) preset value.. 1056.00 BogoMIPS (lpj=5280000)
Reverting this commit fixes the issue.
RCU-related configs:
$ grep RCU .config
# RCU Subsystem
CONFIG_TINY_RCU=y
# CONFIG_RCU_EXPERT is not set
CONFIG_TINY_SRCU=y
# end of RCU Subsystem
# RCU Debugging
# CONFIG_RCU_SCALE_TEST is not set
# CONFIG_RCU_TORTURE_TEST is not set
# CONFIG_RCU_REF_SCALE_TEST is not set
# CONFIG_RCU_TRACE is not set
# CONFIG_RCU_EQS_DEBUG is not set
# end of RCU Debugging
CONFIG_MAPLE_RCU_DISABLED is not defined (and should BTW be renamed,
as CONFIG_* is reserved for kernel configuration options).
I do not see this issue on any other platform
(arm/arm64/risc-v/mips/sh/m68k), several of them use the same
RCU configuration.
Do you have a clue?
Thanks!
> --- a/lib/maple_tree.c
> +++ b/lib/maple_tree.c
> @@ -4107,6 +4107,10 @@ static inline unsigned char mas_wr_new_end(struct ma_wr_state *wr_mas)
> * mas_wr_append: Attempt to append
> * @wr_mas: the maple write state
> *
> + * This is currently unsafe in rcu mode since the end of the node may be cached
> + * by readers while the node contents may be updated which could result in
> + * inaccurate information.
> + *
> * Return: True if appended, false otherwise
> */
> static inline bool mas_wr_append(struct ma_wr_state *wr_mas,
> @@ -4116,6 +4120,9 @@ static inline bool mas_wr_append(struct ma_wr_state *wr_mas,
> struct ma_state *mas = wr_mas->mas;
> unsigned char node_pivots = mt_pivots[wr_mas->type];
>
> + if (mt_in_rcu(mas->tree))
> + return false;
> +
> if (mas->offset != wr_mas->node_end)
> return false;
>
> --
> 2.39.2
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Powered by blists - more mailing lists