lkml.org 
[lkml]   [2017]   [May]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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.

\
 
 \ /
  Last update: 2017-05-12 23:00    [W:0.048 / U:1.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site