mirror of
https://github.com/versity/scoutfs.git
synced 2026-09-04 07:07:03 +00:00
Fix item erase augmentation
The item cache was getting inconsistent as items were removed. This would manifest in failing to find dirty items that it had counted as it was writing items into the segment and removing deletion items. For a start it wasn't using the augmented rb_erase(). We make a function that everyone uses. There's no augmented rb_replace() so We just augment erase, restart, and insert. (We could probably augment on descent and replace/propagate but that can come later.) Then the augmentation callbacks got the semantics slightly wrong. The rotation callback is named after a caller that happens to use it, not on any implied relationship between the nodes. It actually just recalculates the augmentation value for the two subtrees. Mischief managed. (We'll probably rework the augmentation so the value is for the node and its children and we can get rid of the extra code we have today to support our augmentation value that is sensitive to the difference between the left and write subtrees.) Signed-off-by: Zach Brown <zab@versity.com>
This commit is contained in:
+25
-14
@@ -239,10 +239,9 @@ static void scoutfs_item_rb_propagate(struct rb_node *node,
|
||||
|
||||
static void scoutfs_item_rb_copy(struct rb_node *old, struct rb_node *new)
|
||||
{
|
||||
struct cached_item *o = container_of(old, struct cached_item, node);
|
||||
struct cached_item *n = container_of(new, struct cached_item, node);
|
||||
|
||||
n->dirty = o->dirty;
|
||||
n->dirty = compute_item_dirty(n);
|
||||
}
|
||||
|
||||
/* calculate the new parent last as it depends on the old parent */
|
||||
@@ -251,8 +250,6 @@ static void scoutfs_item_rb_rotate(struct rb_node *old, struct rb_node *new)
|
||||
struct cached_item *o = container_of(old, struct cached_item, node);
|
||||
struct cached_item *n = container_of(new, struct cached_item, node);
|
||||
|
||||
BUG_ON(rb_parent(old) != new);
|
||||
|
||||
o->dirty = compute_item_dirty(o);
|
||||
n->dirty = compute_item_dirty(n);
|
||||
}
|
||||
@@ -325,6 +322,20 @@ static void clear_item_dirty(struct item_cache *cac,
|
||||
update_dirty_parents(item);
|
||||
}
|
||||
|
||||
/*
|
||||
* Safely erase an item from the tree. Make sure to remove its dirty
|
||||
* accounting, use the augmented erase, and free it.
|
||||
*/
|
||||
static void erase_item(struct super_block *sb, struct item_cache *cac,
|
||||
struct cached_item *item)
|
||||
{
|
||||
trace_printk("erasing item %p\n", item);
|
||||
|
||||
clear_item_dirty(cac, item);
|
||||
rb_erase_augmented(&item->node, &cac->items, &scoutfs_item_rb_cb);
|
||||
free_item(sb, item);
|
||||
}
|
||||
|
||||
/*
|
||||
* Try to insert the given item. If there's already a non-deletion item
|
||||
* with the insertion key then return -EEXIST. An existing deletion
|
||||
@@ -336,11 +347,14 @@ static int insert_item(struct super_block *sb, struct item_cache *cac,
|
||||
struct cached_item *ins)
|
||||
{
|
||||
struct rb_root *root = &cac->items;
|
||||
struct rb_node **node = &root->rb_node;
|
||||
struct rb_node *parent = NULL;
|
||||
struct cached_item *item;
|
||||
struct rb_node *parent;
|
||||
struct rb_node **node;
|
||||
int cmp;
|
||||
|
||||
restart:
|
||||
node = &root->rb_node;
|
||||
parent = NULL;
|
||||
while (*node) {
|
||||
parent = *node;
|
||||
item = container_of(*node, struct cached_item, node);
|
||||
@@ -358,10 +372,9 @@ static int insert_item(struct super_block *sb, struct item_cache *cac,
|
||||
if (!item->deletion)
|
||||
return -EEXIST;
|
||||
|
||||
clear_item_dirty(cac, item);
|
||||
rb_replace_node(&item->node, &ins->node, root);
|
||||
free_item(sb, item);
|
||||
return 0;
|
||||
/* sadly there's no augmented replace */
|
||||
erase_item(sb, cac, item);
|
||||
goto restart;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1275,10 +1288,8 @@ int scoutfs_item_dirty_seg(struct super_block *sb, struct scoutfs_segment *seg)
|
||||
del = item;
|
||||
item = next_dirty(item);
|
||||
|
||||
if (del->deletion) {
|
||||
rb_erase(&del->node, &cac->items);
|
||||
free_item(sb, del);
|
||||
}
|
||||
if (del->deletion)
|
||||
erase_item(sb, cac, del);
|
||||
|
||||
nr_items--;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user