* Peter Zijlstra <peterz@xxxxxxxxxxxxx> [210514 07:21]:
>
> 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.
>
You are correct, this is a mess.. I will re-examine all comments and
convert to C style and ensure a blank line start.
