| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <e106ee62-8b4b-59bc-d7ec-510c0c75c30a@gmail.com>
Date: Mon, 31 Jul 2023 19:40:16 +0800
From: Peng Zhang <perlyzhang@...il.com>
To: "Liam R. Howlett" <Liam.Howlett@...cle.com>
Cc: peterz@...radead.org, mathieu.desnoyers@...icios.com,
brauner@...nel.org, Peng Zhang <zhangpeng.00@...edance.com>,
surenb@...gle.com, linux-kernel@...r.kernel.org, npiggin@...il.com,
linux-fsdevel@...r.kernel.org, linux-doc@...r.kernel.org,
akpm@...ux-foundation.org, corbet@....net, willy@...radead.org,
linux-mm@...ck.org, avagin@...il.com, michael.christie@...cle.com
Subject: Re: [PATCH 03/11] maple_tree: Add some helper functions
在 2023/7/26 23:02, Liam R. Howlett 写道:
> * 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.
I will make changes accordingly. It's not done here because it doesn't
seem to be relevant to the theme of this patchset.
>
>> 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?
I just think it is redundant to construct maple enode through
node->parent in order to adapt the parameters of mte_*. So ma_* are
introduced to avoid meaningless construction.
>
>> {
>> 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.
Just for not wanting to construct maple enode. It's not really a
problem.
>
>> +{
>> + 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