On Wed, Apr 28, 2021 at 03:36:02PM +0000, Liam Howlett wrote:
> +/* ma_free_rcu() - Use rcu callback to free a maple node
> + * @node: The node to free
> + *
> + * The maple tree uses the parent pointer to indicate this node is no longer in
> + * use and will be freed.
> + */
If this was supposed to be a kernel doc, then it would need to start
with /**, if it was not then it's an inconsistent comment style; by far
the majority of comments in this file have the regular:
/*
* multiline-
* comment
*/
style.
Like
> +/*
> + * We also reserve values with the bottom two bits set to '10' which are
> + * below 4096
> + */
> +/*
> + * mte_to_mat() - Convert a maple encoded node to a maple topiary node.
> + * @entry: The maple encoded node
> + *
> + * Return: a maple topiary pointer
> + */
And:
> +/*
> + * mas_mn() - Get the maple state node.
> + * @mas: The maple state
> + *
> + * Return: the maple node (not encoded - bare pointer).
> + */
But then you also have:
> + // Removing the pivot overflow optimizes the loop below.
> + // Check the first implied pivot.
> + // Check end implied pivot which can only be a gap on the right most
> + // node.
And:
> + /* If the split is less than the max slot && the right side will
> + * still be sufficient, then increment the split on NULL.
> + */
> + /* Avoid having a range less than the slot count unless it
> + * causes one node to be deficient.
> + * NOTE: mt_min_slots is 1 based, b_end and split are zero.
> + */
Single line comments are also an inconsistent mess:
> + /* Avoid ending a node on a NULL entry */
> + // Possible underflow of piv will wrap back to 0 before use.
> + // Copy start data up to insert.
Even in a single function, you can't be consistent:
> +static inline void mast_topiary(struct maple_subtree_state *mast)
> +{
> + unsigned char l_off, r_off, offset;
> + unsigned long l_index, range_min, range_max;
> + struct maple_enode *child;
> + void __rcu **slots;
> + enum maple_type mt;
> +
> + // The left node is consumed, so add to the free list.
> + l_index = mast->orig_l->index;
> + mast->orig_l->index = mast->orig_l->last;
> + mt = mte_node_type(mast->orig_l->node);
> + mas_node_walk(mast->orig_l, mt, &range_min, &range_max);
> + mast->orig_l->index = l_index;
> + l_off = mast->orig_l->offset;
> + r_off = mast->orig_r->offset;
> + if (mast->orig_l->node == mast->orig_r->node) {
> + slots = ma_slots(mte_to_node(mast->orig_l->node), mt);
> + for (offset = l_off + 1; offset < r_off; offset++)
> + mat_add(mast->destroy, mas_slot_locked(mast->orig_l,
> + slots, offset));
> +
> + return;
> + }
> + /* mast->orig_r is different and consumed. */
> + if (mte_is_leaf(mast->orig_r->node))
> + return;
> +
> + /* Now destroy l_off + 1 -> end and 0 -> r_off - 1 */
> + offset = l_off + 1;
> + slots = ma_slots(mte_to_node(mast->orig_l->node), mt);
> + while (offset < mt_slots[mt]) {
> + child = mas_slot_locked(mast->orig_l, slots, offset++);
> + if (!child)
> + break;
> +
> + mat_add(mast->destroy, child);
> + }
> +
> + slots = ma_slots(mte_to_node(mast->orig_r->node),
> + mte_node_type(mast->orig_r->node));
> + for (offset = 0; offset < r_off; offset++)
> + mat_add(mast->destroy,
> + mas_slot_locked(mast->orig_l, slots, offset));
> +}
This mixing of C and C++ style comments is a mess.
