lkml.org 
[lkml]   [2023]   [Dec]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] maple_tree: do not preallocate nodes for slot stores
From
On 12/12/23 12:57 PM, Matthew Wilcox wrote:
> 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?
yes, I think we should add helper functions to identify the different type of
stores. Thanks for the pointers to code style this is what I think the slot
store identifying helper function would look like:

static inline bool mas_wr_is_slot_store(struct ma_wr_state *wr_mas)
{
struct ma_state *mas = wr_mas->mas;
unsigned char node_size = mas_wr_new_end(wr_mas);

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

return false;
}

thanks,
Sid
> 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.
>

\
 
 \ /
  Last update: 2023-12-12 22:50    [W:1.836 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site