diff for duplicates of <20240820175417.2782532-1-Liam.Howlett@oracle.com> diff --git a/a/1.txt b/N1/1.txt index 955d37b..c78fd26 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,47 +1,52 @@ -From: "Liam R. Howlett" <Liam.Howlett@Oracle.com> +On Tue, 20 Aug 2024 13:54:17 -0400 "Liam R. Howlett" <Liam.Howlett@Oracle.com> +> +> The write lock should be held when validating the tree to avoid updates +> racing with checks. Holding the rcu read lock during a large tree +> validation may also cause a prolonged rcu read window. +> +From the rcu stall's view, holding spin lock plays the same role of rcu +read lock, so what are you fixing by simply dropping rcu read lock? -The write lock should be held when validating the tree to avoid updates -racing with checks. Holding the rcu read lock during a large tree -validation may also cause a prolonged rcu read window. +> Link: https://lore.kernel.org/all/0000000000001d12d4062005aea1@google.com/ +> Fixes: 54a611b60590 ("Maple Tree: add new data structure") +> Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> +> Reported-by: syzbot+036af2f0c7338a33b0cd@syzkaller.appspotmail.com +> --- +> lib/maple_tree.c | 7 ++----- +> 1 file changed, 2 insertions(+), 5 deletions(-) +> +> diff --git a/lib/maple_tree.c b/lib/maple_tree.c +> index 755ba8b18e14..fe1b01b29201 100644 +> --- a/lib/maple_tree.c +> +++ b/lib/maple_tree.c +> @@ -7588,14 +7588,14 @@ static void mt_validate_nulls(struct maple_tree *mt) +> * 2. The gap is correctly set in the parents +> */ +> void mt_validate(struct maple_tree *mt) +> + __must_hold(mas->tree->ma_lock) +> { +> unsigned char end; +> + lockdep_assert_held(ma_lock); instead -Link: https://lore.kernel.org/all/0000000000001d12d4062005aea1@google.com/ -Fixes: 54a611b60590 ("Maple Tree: add new data structure") -Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> -Reported-by: syzbot+036af2f0c7338a33b0cd@syzkaller.appspotmail.com ---- - lib/maple_tree.c | 7 ++----- - 1 file changed, 2 insertions(+), 5 deletions(-) - -diff --git a/lib/maple_tree.c b/lib/maple_tree.c -index 755ba8b18e14..fe1b01b29201 100644 ---- a/lib/maple_tree.c -+++ b/lib/maple_tree.c -@@ -7588,14 +7588,14 @@ static void mt_validate_nulls(struct maple_tree *mt) - * 2. The gap is correctly set in the parents - */ - void mt_validate(struct maple_tree *mt) -+ __must_hold(mas->tree->ma_lock) - { - unsigned char end; - - MA_STATE(mas, mt, 0, 0); -- rcu_read_lock(); - mas_start(&mas); - if (!mas_is_active(&mas)) -- goto done; -+ return; - - while (!mte_is_leaf(mas.node)) - mas_descend(&mas); -@@ -7616,9 +7616,6 @@ void mt_validate(struct maple_tree *mt) - mas_dfs_postorder(&mas, ULONG_MAX); - } - mt_validate_nulls(mt); --done: -- rcu_read_unlock(); -- - } - EXPORT_SYMBOL_GPL(mt_validate); - --- -2.43.0 +> MA_STATE(mas, mt, 0, 0); +> - rcu_read_lock(); +> mas_start(&mas); +> if (!mas_is_active(&mas)) +> - goto done; +> + return; +> +> while (!mte_is_leaf(mas.node)) +> mas_descend(&mas); +> @@ -7616,9 +7616,6 @@ void mt_validate(struct maple_tree *mt) +> mas_dfs_postorder(&mas, ULONG_MAX); +> } +> mt_validate_nulls(mt); +> -done: +> - rcu_read_unlock(); +> - +> } +> EXPORT_SYMBOL_GPL(mt_validate); +> +> -- +> 2.43.0 diff --git a/a/content_digest b/N1/content_digest index f7727e7..2c88689 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -1,60 +1,67 @@ - "From\0Liam R. Howlett <Liam.Howlett@oracle.com>\0" - "Subject\0[PATCH] maple_tree: Remove rcu_read_lock() from mt_validate()\0" - "Date\0Tue, 20 Aug 2024 13:54:17 -0400\0" - "To\0Andrew Morton <akpm@linux-foundation.org>\0" - "Cc\0maple-tree@lists.infradead.org" + "From\0Hillf Danton <hdanton@sina.com>\0" + "Subject\0Re: [PATCH] maple_tree: Remove rcu_read_lock() from mt_validate()\0" + "Date\0Wed, 21 Aug 2024 06:38:45 +0800\0" + "To\0Liam R. Howlett <Liam.Howlett@oracle.com>\0" + "Cc\0Andrew Morton <akpm@linux-foundation.org>" + maple-tree@lists.infradead.org linux-mm@kvack.org linux-kernel@vger.kernel.org + Paul E. McKenney <paulmck@kernel.org> Liam R. Howlett <Liam.Howlett@oracle.com> " syzbot+036af2f0c7338a33b0cd@syzkaller.appspotmail.com\0" "\00:1\0" "b\0" - "From: \"Liam R. Howlett\" <Liam.Howlett@Oracle.com>\n" + "On Tue, 20 Aug 2024 13:54:17 -0400 \"Liam R. Howlett\" <Liam.Howlett@Oracle.com>\n" + "> \n" + "> The write lock should be held when validating the tree to avoid updates\n" + "> racing with checks. Holding the rcu read lock during a large tree\n" + "> validation may also cause a prolonged rcu read window.\n" + "> \n" + "From the rcu stall's view, holding spin lock plays the same role of rcu\n" + "read lock, so what are you fixing by simply dropping rcu read lock?\n" "\n" - "The write lock should be held when validating the tree to avoid updates\n" - "racing with checks. Holding the rcu read lock during a large tree\n" - "validation may also cause a prolonged rcu read window.\n" + "> Link: https://lore.kernel.org/all/0000000000001d12d4062005aea1@google.com/\n" + "> Fixes: 54a611b60590 (\"Maple Tree: add new data structure\")\n" + "> Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com>\n" + "> Reported-by: syzbot+036af2f0c7338a33b0cd@syzkaller.appspotmail.com\n" + "> ---\n" + "> lib/maple_tree.c | 7 ++-----\n" + "> 1 file changed, 2 insertions(+), 5 deletions(-)\n" + "> \n" + "> diff --git a/lib/maple_tree.c b/lib/maple_tree.c\n" + "> index 755ba8b18e14..fe1b01b29201 100644\n" + "> --- a/lib/maple_tree.c\n" + "> +++ b/lib/maple_tree.c\n" + "> @@ -7588,14 +7588,14 @@ static void mt_validate_nulls(struct maple_tree *mt)\n" + "> * 2. The gap is correctly set in the parents\n" + "> */\n" + "> void mt_validate(struct maple_tree *mt)\n" + "> +\t__must_hold(mas->tree->ma_lock)\n" + "> {\n" + "> \tunsigned char end;\n" + "> \n" + "\tlockdep_assert_held(ma_lock); instead\n" "\n" - "Link: https://lore.kernel.org/all/0000000000001d12d4062005aea1@google.com/\n" - "Fixes: 54a611b60590 (\"Maple Tree: add new data structure\")\n" - "Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com>\n" - "Reported-by: syzbot+036af2f0c7338a33b0cd@syzkaller.appspotmail.com\n" - "---\n" - " lib/maple_tree.c | 7 ++-----\n" - " 1 file changed, 2 insertions(+), 5 deletions(-)\n" - "\n" - "diff --git a/lib/maple_tree.c b/lib/maple_tree.c\n" - "index 755ba8b18e14..fe1b01b29201 100644\n" - "--- a/lib/maple_tree.c\n" - "+++ b/lib/maple_tree.c\n" - "@@ -7588,14 +7588,14 @@ static void mt_validate_nulls(struct maple_tree *mt)\n" - " * 2. The gap is correctly set in the parents\n" - " */\n" - " void mt_validate(struct maple_tree *mt)\n" - "+\t__must_hold(mas->tree->ma_lock)\n" - " {\n" - " \tunsigned char end;\n" - " \n" - " \tMA_STATE(mas, mt, 0, 0);\n" - "-\trcu_read_lock();\n" - " \tmas_start(&mas);\n" - " \tif (!mas_is_active(&mas))\n" - "-\t\tgoto done;\n" - "+\t\treturn;\n" - " \n" - " \twhile (!mte_is_leaf(mas.node))\n" - " \t\tmas_descend(&mas);\n" - "@@ -7616,9 +7616,6 @@ void mt_validate(struct maple_tree *mt)\n" - " \t\tmas_dfs_postorder(&mas, ULONG_MAX);\n" - " \t}\n" - " \tmt_validate_nulls(mt);\n" - "-done:\n" - "-\trcu_read_unlock();\n" - "-\n" - " }\n" - " EXPORT_SYMBOL_GPL(mt_validate);\n" - " \n" - "-- \n" - 2.43.0 + "> \tMA_STATE(mas, mt, 0, 0);\n" + "> -\trcu_read_lock();\n" + "> \tmas_start(&mas);\n" + "> \tif (!mas_is_active(&mas))\n" + "> -\t\tgoto done;\n" + "> +\t\treturn;\n" + "> \n" + "> \twhile (!mte_is_leaf(mas.node))\n" + "> \t\tmas_descend(&mas);\n" + "> @@ -7616,9 +7616,6 @@ void mt_validate(struct maple_tree *mt)\n" + "> \t\tmas_dfs_postorder(&mas, ULONG_MAX);\n" + "> \t}\n" + "> \tmt_validate_nulls(mt);\n" + "> -done:\n" + "> -\trcu_read_unlock();\n" + "> -\n" + "> }\n" + "> EXPORT_SYMBOL_GPL(mt_validate);\n" + "> \n" + "> -- \n" + > 2.43.0 -f2056c94a3de0378bbd2fc862c88c8bbfb0e9604f2d541ad7bcb2fe315f57480 +a5f359f244e4dea6a0bbb90c572e2c27cfe75023f6e9fb5ddf0e014924038077
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox