Re: [PATCH] maple_tree: do not preallocate nodes for slot stores

[Matthew Wilcox <willy@xxxxxxxxxxxxx>]

On Tue, Dec 12, 2023 at 11:46:40AM -0800, Sidhartha Kumar wrote:
> +	/* Slot store, does not require additional nodes */
> +	if ((node_size == mas->end) && ((!mt_in_rcu(mas->tree))
> +		|| (wr_mas.offset_end - mas->offset == 1)))
> +		return 0;

Should we refactor this into a mas_is_slot_store() predicate?

A few coding-style problems with it as it's currently written:

1. The indentation on the second line is wrong.  It makes the
continuation of the condition look like part of the statement.  Use
extra whitespace to indent.  eg:

	if ((node_size == mas->end) && ((!mt_in_rcu(mas->tree))
			|| (wr_mas.offset_end - mas->offset == 1)))
		return 0;

2. The operator goes last on the line, not at the beginning of the
continuation line.  ie:

	if ((node_size == mas->end) && ((!mt_in_rcu(mas->tree)) ||
			(wr_mas.offset_end - mas->offset == 1)))
		return 0;

3. You don't need parens around the !mt_in_rcu(mas->tree).  There's
no ambiguity to solve here:

	if ((node_size == mas->end) && (!mt_in_rcu(mas->tree) ||
			(wr_mas.offset_end - mas->offset == 1)))
		return 0;

But I'd write it as:

	if ((node_size == mas->end) &&
	    (!mt_in_rcu(mas->tree) || (wr_mas.offset_end - mas->offset == 1)))
		return 0;

because then the whitespace matches how you're supposed to parse the
condition, and so the next person to read this code will have an easier
time of it.