[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230726150252.x56owgz3ikujzicu@revolver>
Date: Wed, 26 Jul 2023 11:02:52 -0400
From: "Liam R. Howlett" <Liam.Howlett@...cle.com>
To: Peng Zhang <zhangpeng.00@...edance.com>
Cc: corbet@....net, akpm@...ux-foundation.org, willy@...radead.org,
brauner@...nel.org, surenb@...gle.com, michael.christie@...cle.com,
peterz@...radead.org, mathieu.desnoyers@...icios.com,
npiggin@...il.com, avagin@...il.com, linux-mm@...ck.org,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 03/11] maple_tree: Add some helper functions
* Peng Zhang <zhangpeng.00@...edance.com> [230726 04:10]:
> Add some helper functions so that their parameters are maple node
> instead of maple enode, these functions will be used later.
>
> Signed-off-by: Peng Zhang <zhangpeng.00@...edance.com>
> ---
> lib/maple_tree.c | 71 +++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 55 insertions(+), 16 deletions(-)
>
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index e0e9a87bdb43..da3a2fb405c0 100644
> --- a/lib/maple_tree.c
> +++ b/lib/maple_tree.c
> @@ -164,6 +164,11 @@ static inline int mt_alloc_bulk(gfp_t gfp, size_t size, void **nodes)
> return kmem_cache_alloc_bulk(maple_node_cache, gfp, size, nodes);
> }
>
> +static inline void mt_free_one(struct maple_node *node)
> +{
> + kmem_cache_free(maple_node_cache, node);
> +}
> +
There is a place in mas_destroy() that could use this if it is added.
> static inline void mt_free_bulk(size_t size, void __rcu **nodes)
> {
> kmem_cache_free_bulk(maple_node_cache, size, (void **)nodes);
> @@ -432,18 +437,18 @@ static inline unsigned long mte_parent_slot_mask(unsigned long parent)
> }
>
> /*
> - * mas_parent_type() - Return the maple_type of the parent from the stored
> - * parent type.
> - * @mas: The maple state
> - * @enode: The maple_enode to extract the parent's enum
> + * ma_parent_type() - Return the maple_type of the parent from the stored parent
> + * type.
> + * @mt: The maple tree
> + * @node: The maple_node to extract the parent's enum
> * Return: The node->parent maple_type
> */
> static inline
> -enum maple_type mas_parent_type(struct ma_state *mas, struct maple_enode *enode)
> +enum maple_type ma_parent_type(struct maple_tree *mt, struct maple_node *node)
I was trying to keep ma_* prefix to mean the first argument is
maple_node and mt_* to mean maple_tree. I wasn't entirely successful
with this and I do see why you want to use ma_, but maybe reverse the
arguments here?
> {
> unsigned long p_type;
>
> - p_type = (unsigned long)mte_to_node(enode)->parent;
> + p_type = (unsigned long)node->parent;
> if (WARN_ON(p_type & MAPLE_PARENT_ROOT))
> return 0;
>
> @@ -451,7 +456,7 @@ enum maple_type mas_parent_type(struct ma_state *mas, struct maple_enode *enode)
> p_type &= ~mte_parent_slot_mask(p_type);
> switch (p_type) {
> case MAPLE_PARENT_RANGE64: /* or MAPLE_PARENT_ARANGE64 */
> - if (mt_is_alloc(mas->tree))
> + if (mt_is_alloc(mt))
> return maple_arange_64;
> return maple_range_64;
> }
> @@ -459,6 +464,19 @@ enum maple_type mas_parent_type(struct ma_state *mas, struct maple_enode *enode)
> return 0;
> }
>
> +/*
> + * mas_parent_type() - Return the maple_type of the parent from the stored
> + * parent type.
> + * @mas: The maple state
> + * @enode: The maple_enode to extract the parent's enum
> + * Return: The node->parent maple_type
> + */
> +static inline
> +enum maple_type mas_parent_type(struct ma_state *mas, struct maple_enode *enode)
> +{
> + return ma_parent_type(mas->tree, mte_to_node(enode));
> +}
> +
> /*
> * mas_set_parent() - Set the parent node and encode the slot
> * @enode: The encoded maple node.
> @@ -499,14 +517,14 @@ void mas_set_parent(struct ma_state *mas, struct maple_enode *enode,
> }
>
> /*
> - * mte_parent_slot() - get the parent slot of @enode.
> - * @enode: The encoded maple node.
> + * ma_parent_slot() - get the parent slot of @node.
> + * @node: The maple node.
> *
> - * Return: The slot in the parent node where @enode resides.
> + * Return: The slot in the parent node where @node resides.
> */
> -static inline unsigned int mte_parent_slot(const struct maple_enode *enode)
> +static inline unsigned int ma_parent_slot(const struct maple_node *node)
> {
> - unsigned long val = (unsigned long)mte_to_node(enode)->parent;
> + unsigned long val = (unsigned long)node->parent;
>
> if (val & MA_ROOT_PARENT)
> return 0;
> @@ -519,15 +537,36 @@ static inline unsigned int mte_parent_slot(const struct maple_enode *enode)
> }
>
> /*
> - * mte_parent() - Get the parent of @node.
> - * @node: The encoded maple node.
> + * mte_parent_slot() - get the parent slot of @enode.
> + * @enode: The encoded maple node.
> + *
> + * Return: The slot in the parent node where @enode resides.
> + */
> +static inline unsigned int mte_parent_slot(const struct maple_enode *enode)
> +{
> + return ma_parent_slot(mte_to_node(enode));
> +}
> +
> +/*
> + * ma_parent() - Get the parent of @node.
> + * @node: The maple node.
> + *
> + * Return: The parent maple node.
> + */
> +static inline struct maple_node *ma_parent(const struct maple_node *node)
I had a lot of these helpers before, but they eventually became used so
little that I dropped them.
> +{
> + return (void *)((unsigned long)(node->parent) & ~MAPLE_NODE_MASK);
> +}
> +
> +/*
> + * mte_parent() - Get the parent of @enode.
> + * @enode: The encoded maple node.
> *
> * Return: The parent maple node.
> */
> static inline struct maple_node *mte_parent(const struct maple_enode *enode)
> {
> - return (void *)((unsigned long)
> - (mte_to_node(enode)->parent) & ~MAPLE_NODE_MASK);
> + return ma_parent(mte_to_node(enode));
> }
>
> /*
> --
> 2.20.1
>
Powered by blists - more mailing lists