Messages in this thread | | | Date | Fri, 12 May 2017 13:58:58 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH v2] lib/btree.c: optimise the code by previously getpos function |
| |
On Thu, 11 May 2017 17:42:21 +0800 Leno Hou <lenohou@gmail.com> wrote:
> This patch optimized the code by previously getpos function call. > Therefore, It's takes the convenience to understand logic of code.
I would rewrite this changelog to read
: Rework the getpos() helper function and use it to remove various : open-coded implemetnations of its funtionality.
> ... > > @@ -466,7 +458,7 @@ static int btree_insert_level(struct btree_head *head, struct btree_geo *geo, > /* two identical keys are not allowed */ > BUG_ON(pos < fill && keycmp(geo, node, pos, key) == 0); > > - if (fill == geo->no_pairs) { > + if (fill < 0) { > /* need to split node */ > unsigned long *new;
The code here is a bit awkward.
: retry: : node = find_level(head, geo, key, level); : pos = getpos(geo, node, key); : fill = getfill(geo, node, pos); : /* two identical keys are not allowed */ : BUG_ON(pos < fill && keycmp(geo, node, pos, key) == 0); : : if (fill < 0) {
If getpos() returns -ENOENT then we're passing -ENOENT into getfill(). That might happen to work OK (or it might go BUG) but it's ugly and unobvious.
There's a similar issue in rebalance() and in btree_remove_level(): failed to update existing getpos() callers for the new getpos() return value semantics.
| |