Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 26 Jul 2023 11:02:52 -0400
From:   "Liam R. Howlett" <Liam.Howlett@Oracle.com>
To:     Peng Zhang <zhangpeng.00@bytedance.com>
Cc:     corbet@lwn.net, akpm@linux-foundation.org, willy@infradead.org,
        brauner@kernel.org, surenb@google.com, michael.christie@oracle.com,
        peterz@infradead.org, mathieu.desnoyers@efficios.com,
        npiggin@gmail.com, avagin@gmail.com, linux-mm@kvack.org,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 03/11] maple_tree: Add some helper functions
Message-ID: <20230726150252.x56owgz3ikujzicu@revolver>
Mail-Followup-To: "Liam R. Howlett" <Liam.Howlett@Oracle.com>,
        Peng Zhang <zhangpeng.00@bytedance.com>, corbet@lwn.net,
        akpm@linux-foundation.org, willy@infradead.org, brauner@kernel.org,
        surenb@google.com, michael.christie@oracle.com,
        peterz@infradead.org, mathieu.desnoyers@efficios.com,
        npiggin@gmail.com, avagin@gmail.com, linux-mm@kvack.org,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-fsdevel@vger.kernel.org
References: <20230726080916.17454-1-zhangpeng.00@bytedance.com>
 <20230726080916.17454-4-zhangpeng.00@bytedance.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230726080916.17454-4-zhangpeng.00@bytedance.com>
User-Agent: NeoMutt/20220429
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?+WXdehoGwJw3shuUQdRU9J4VjpOKhU2AzQXZH8OPZ0xld8PgIh5/gEIoF/3J?=
 =?us-ascii?Q?4h6EihSf1G4YuqT/OoFlxfD6OuI0GIjQ7OB84T4JUAmaW89sHkWXZdp6Cteh?=
 =?us-ascii?Q?RhiYkQA5QmFfPmOsSytW+NjzXiqyA5ldY2JnQw6aQbRfFWt4sQFjZwuMt6he?=
 =?us-ascii?Q?Pz3lnt/HSXa3PZxoMxrT0IRDG+qY3ENSPU11pXS4Lf/lXmmLqZsp0NVaEVZP?=
 =?us-ascii?Q?yZRdXR0AJJAkE2/f6HUFR17V95IrnsCj5UngmY1HYfjcwJ+N/D3nEcQJHBua?=
 =?us-ascii?Q?PNctbbmYWEeu5yTT7su9aBTIZUtNd90Ru/5S+jCzl9tyk1LQojMWdig9vywx?=
 =?us-ascii?Q?j1PW0BXBjbwRsdZe7ehebfqZ81CkeJId0SyvCloDPlttTW1MpcHtllK5zbPe?=
 =?us-ascii?Q?m2JrKfBIK85zSMmrTljPFCYmfSD27zal/8jOk/67IeFEQXisZ59cJVcXVGbZ?=
 =?us-ascii?Q?PLOHn5Uz+u660EHPudv50c6Uziei9+oHHY3sDw8ApBkXoddb0bQgBVBt93QG?=
 =?us-ascii?Q?/zFXZC7MnZH4lXj/Gj7KWv8aN3kBCdEHll7ViMn4V06MzbEnz/TlrgWY92gX?=
 =?us-ascii?Q?VO7PzTWv/hNYHNnFgJNA/xIE1EFm+vrPn/PiOVfYQdAm/fA/9+y0cKaKrBMQ?=
 =?us-ascii?Q?FPpfjSm/oYde7ZVmpplsqO7SsVpFvieTzn1BwQMl7H1M3IB7qmCpD8i1JUeF?=
 =?us-ascii?Q?3ztcsK6vrLgExwWZfahQ0LreM7343SYivlZnh/kES/ufHWCbuqcy/UmGb1zG?=
 =?us-ascii?Q?QpWjKfSTASgIgOrdKuqOioLoHaOLQldFBDyTkfXzq2uZwgJLj9j7+jWAodPy?=
 =?us-ascii?Q?cultKDZFpcQnhkIR0cQeQEMtIYZY1oL+S2XzDfWAHy3YMQAjY47dm9PuWc4V?=
 =?us-ascii?Q?FRb+xeVvxbJWPwqd7gRpKL6FbyCz8sjfO/BR1dKnDv0akveicG8fAhXRvYVa?=
 =?us-ascii?Q?0JkaBpocEhIEKGQoY6Nh+of/XuIeqwPdYaWqWApS4hAM22objHV19GaCzFYO?=
 =?us-ascii?Q?uEjuh4Lr75vZIUsUs1ObPivPbS/nRAEMv9eAYCu9RLhuFZmzpLHkC/TSZe/F?=
 =?us-ascii?Q?xa8b4WB9KbDHt35qQLLu5F4BPLv5+rkF9o7FZoWjDBk3/Pwizhm21Ov4YWqK?=
 =?us-ascii?Q?Fazkw7dIYDf2Gy3cO94RJOq3XRx2D70JKRsLaMpiuKBcKNXITjLprTIFGj4T?=
 =?us-ascii?Q?2CnE331ovrzZL23DRjkNHw5ihAJRr3vEcJZ/z49cIatEQfQLst3uWWGIbg0r?=
 =?us-ascii?Q?V8N50+Q6SjPxOPf0VJgWpaUihFbLtPZg63z8ZktGLimYeWUFbNpspVlPMr4t?=
 =?us-ascii?Q?K0ys9+ik11AfIuF9zPKRsdwJNuvjDDL0eqS1pX0nQqrPmbucmWiCF6SOepHj?=
 =?us-ascii?Q?UYgZUmB9Z7pJ8nLzCOL3+vIsEBhbH0ITIw4sh0uY+yevEHtOmE0U0jHjWrt1?=
 =?us-ascii?Q?leMbbJUjsXluAZppp3gfX8IYXMwLIF5vzIIHqrToLvTGapyPg2fTUTG3fAhO?=
 =?us-ascii?Q?Jv5YkTfna6J8m53z2EjMP6dbHuw6/jXTORj8m24BOoup7pt8KDLJHzrSP6V/?=
 =?us-ascii?Q?B3W7mD9GpW97ZuHKd/BA1iU0of6mv1OkG3n0vRbTWtNzOzTWb1Xt6gfiTr5z?=
 =?us-ascii?Q?DQ=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?us-ascii?Q?DXQmDZJkfi8QAEEEGf7qI1k/MXjfFH4RJKx6z6fdfsbohw5oaEMh91BDZc1E?=
 =?us-ascii?Q?2F0y/EC1TIJBySTX4vq+1TBWq45UiJblVsEzH2CZszGOOf6I1GO1qvdFGLW+?=
 =?us-ascii?Q?l55Dp95j0rTM/kS7iSsI1wHKagA1+CVG9KCa9e08yiUmRra3kKVivIlb3DzF?=
 =?us-ascii?Q?b3pME6Pg0rpEIf5R+GghAG7Ubax9IQatflkQeTZ+hsNniMtdrOxUU2pFKL8Z?=
 =?us-ascii?Q?52CGAIrcXM86B/0fK7ju5vdgb7mK6YR0VWv/ojGvod4RtBsUn5SPJmPgzwey?=
 =?us-ascii?Q?AaNshFjThF1gnBfmliThv+yP6uHEW/t+eOrLAy9avXYqfC6HtwHK7Q9k7dFm?=
 =?us-ascii?Q?0bFm/wGOTqYGImbdMP/8xTHo/7Isxt708Kb135KKiqppzyq6i1NE7MlEC8py?=
 =?us-ascii?Q?nxsbhOcNFpyvhCMYNXMiWxOXEEtmAj+quDfsUY5gty/+Ilc5287SaSIiNtYq?=
 =?us-ascii?Q?OEoca2yr/GlanhyPLXwAuRXs/qooRnx7LnQ7avskwrfs9KbiWYYIvneOp4FO?=
 =?us-ascii?Q?TRTa4f2ApzrjdCMzHut/ieNJOQaBpinHHtyxjioCRaHL0VaDuFHb0DI9r9i9?=
 =?us-ascii?Q?VjZqRlI91OljAY9+KDcI29GmFBgKFFsrYPKhSXoiqHd+xJJ/TQUAs7YVMZ/9?=
 =?us-ascii?Q?uttTlsC2FRMesovdzLm8Xe0Z5CQ50ACy7FwhCp9EHmUOMbOVsCBflmlrAtOM?=
 =?us-ascii?Q?SqELvrSJm9R5Kd5C9S7tlF1qyVhCNiaal1z0txiq2PYHP1PjzNTW7zincxRW?=
 =?us-ascii?Q?q13DdDqB2203MLPBR4VhG8xuaR0N0a0KHiZpwU20RtK4Dsfsi9GrChmiyGh5?=
 =?us-ascii?Q?BZTlzmn5nNHexO4PuZVty6TF1yiC2yYDGzIO8hDhyYNfqAtQYR2Xpe6ZWB6Y?=
 =?us-ascii?Q?WD8Mkm+fC8qpJ5dMq09mNa6u7oab1X8ClH+HhNj/9WT4bvXEUP2DsgarLviH?=
 =?us-ascii?Q?YqGOxQevkjaO0IIUraOqe7bU7elcwS9NfPp9CJcYe0EqL3A5pB8nuY9oDTAL?=
 =?us-ascii?Q?yUAsHySbf+P/77TbooldrTCK2fMLa4lHzQpsdWOgvqcdLSY9DScZcKBSLlw1?=
 =?us-ascii?Q?a4DYvMq6?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: bfff3f94-bb3a-4f85-c180-08db8de9647d
X-MS-Exchange-CrossTenant-AuthSource: SN6PR10MB3022.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Jul 2023 15:02:55.7110
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: n8FlfIsaVFHh6s3yYHoLqLjDrFL5iymMkGEmC7dvfXFf0MP6J5TTEhYMQwi7x9wct/fKZHvoWmUSS7aOs9DpKQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB5552
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26
 definitions=2023-07-26_06,2023-07-26_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 phishscore=0 bulkscore=0
 malwarescore=0 mlxscore=0 spamscore=0 mlxlogscore=999 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2306200000
 definitions=main-2307260133
X-Proofpoint-ORIG-GUID: LcyRf-GKWjDF1Cm4BjkilJ6mzSwGPgZd
X-Proofpoint-GUID: LcyRf-GKWjDF1Cm4BjkilJ6mzSwGPgZd
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

* Peng Zhang <zhangpeng.00@bytedance.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@bytedance.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
>