linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Liam R. Howlett" <Liam.Howlett@oracle.com>
To: Wei Yang <richard.weiyang@gmail.com>
Cc: akpm@linux-foundation.org, maple-tree@lists.infradead.org,
	linux-mm@kvack.org, Sidhartha Kumar <sidhartha.kumar@oracle.com>,
	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Subject: Re: [PATCH v5 4/5] maple_tree: refine mas_store_root() on storing NULL
Date: Fri, 1 Nov 2024 10:59:24 -0400	[thread overview]
Message-ID: <bgui2jdhv5bp5damujjj4tsepcghhnrzetsk6hozsazwng23rs@jei7badtt36r> (raw)
In-Reply-To: <20241031231627.14316-5-richard.weiyang@gmail.com>

* Wei Yang <richard.weiyang@gmail.com> [241031 19:17]:
> Currently, when storing NULL on mas_store_root(), the behavior could be
> improved.

Storing NULLs over the entire tree may result in a node being used to
store a single range.  Further stores of NULL may cause the node and
tree to be corrupt and cause incorrect behaviour.  Fixing the store to
the root null fixes the issue by ensuring that a range of 0 - ULONG_MAX
results in an empty tree.

Users of the tree may experience incorrect values returned if the tree
was expanded to store values, then overwritten by all NULLS, then
continued to store NULLs over the empty area.

> 
> For example possible cases are:
> 
>   * store NULL at any range result a new node
>   * store NULL at range [m, n] where m > 0 to a single entry tree result
>     a new node with range [m, n] set to NULL
>   * store NULL at range [m, n] where m > 0 to an empty tree result
>     consecutive NULL slot
>   * it allows for multiple NULL entries by expanding root
>     to store NULLs to an empty tree
> 
> This patch tries to improve in:
> 
>   * memory efficient by setting to empty tree instead of using a node
>   * remove the possibility of consecutive NULL slot which will prohibit
>     extended null in later operation
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> CC: Liam R. Howlett <Liam.Howlett@Oracle.com>
> CC: Sidhartha Kumar <sidhartha.kumar@oracle.com>
> CC: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Reviewed-by: Liam R. Howlett <Liam.Howlett@Oracle.com>

Please add stable to Cc list, and fixes tag.  This needs to be
backported, probably to v6.1

> 
> ---
> v3: move change into mas_store_root()
> v4: add a comment and simplify the logic a little
>     adjust the change log a little
> ---
>  lib/maple_tree.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index 4900f182e99d..d0ae808f3a14 100644
> --- a/lib/maple_tree.c
> +++ b/lib/maple_tree.c
> @@ -3447,9 +3447,20 @@ static inline void mas_root_expand(struct ma_state *mas, void *entry)
>  	return;
>  }
>  
> +/*
> + * mas_store_root() - Storing value into root.
> + * @mas: The maple state
> + * @entry: The entry to store.
> + *
> + * There is no root node now and we are storing a value into the root - this
> + * function either assigns the pointer or expands into a node.
> + */
>  static inline void mas_store_root(struct ma_state *mas, void *entry)
>  {
> -	if (likely((mas->last != 0) || (mas->index != 0)))
> +	if (!entry) {
> +		if (!mas->index)
> +			rcu_assign_pointer(mas->tree->ma_root, NULL);
> +	} else if (likely((mas->last != 0) || (mas->index != 0)))
>  		mas_root_expand(mas, entry);
>  	else if (((unsigned long) (entry) & 3) == 2)
>  		mas_root_expand(mas, entry);
> -- 
> 2.34.1
> 


  reply	other threads:[~2024-11-01 14:59 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-31 23:16 [PATCH v5 0/5] refine storing null Wei Yang
2024-10-31 23:16 ` [PATCH v5 1/5] maple_tree: print empty for an empty tree on mt_dump() Wei Yang
2024-10-31 23:16 ` [PATCH v5 2/5] maple_tree: the return value of mas_root_expand() is not used Wei Yang
2024-10-31 23:16 ` [PATCH v5 3/5] maple_tree: not necessary to check index/last again Wei Yang
2024-10-31 23:16 ` [PATCH v5 4/5] maple_tree: refine mas_store_root() on storing NULL Wei Yang
2024-11-01 14:59   ` Liam R. Howlett [this message]
2024-11-01 18:41     ` Andrew Morton
2024-10-31 23:16 ` [PATCH v5 5/5] maple_tree: add a test checking storing null Wei Yang
2024-11-01  3:37   ` Andrew Morton
2024-11-03 22:46     ` Wei Yang
2024-11-01  0:20 ` [PATCH v5 0/5] refine " Andrew Morton
2024-11-01 14:52   ` Liam R. Howlett
2024-11-03 23:09     ` Wei Yang

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=bgui2jdhv5bp5damujjj4tsepcghhnrzetsk6hozsazwng23rs@jei7badtt36r \
    --to=liam.howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=maple-tree@lists.infradead.org \
    --cc=richard.weiyang@gmail.com \
    --cc=sidhartha.kumar@oracle.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox