On Mon, Mar 02, 2020 at 05:45:49PM +0800, Qu Wenruo wrote:
> Since backref_edge is used to connect upper and lower backref nodes, and
> need to access both nodes, some code can look pretty nasty:
>
> list_add_tail(&edge->list[LOWER], &cur->upper);
>
> The above code will link @cur to the LOWER side of the edge, while both
> "LOWER" and "upper" words show up.
> This can sometimes be very confusing for reader to grasp.
>
> This patch introduce a new wrapper, link_backref_edge(), to handle the
> linking behavior.
> Which also has extra ASSERT() to ensure caller won't pass wrong nodes
> in.
>
> Also, this updates the comment of related lists of backref_node and
> backref_edge, to make it more clear that each list points to what.
>
> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
> ---
> fs/btrfs/relocation.c | 53 ++++++++++++++++++++++++++++++-------------
> 1 file changed, 37 insertions(+), 16 deletions(-)
>
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 04416489d87a..c76849409c81 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -91,10 +91,12 @@ struct backref_node {
> u64 owner;
> /* link to pending, changed or detached list */
> struct list_head list;
> - /* list of upper level blocks reference this block */
> +
> + /* List of upper level edges, which links this node to its parent(s) */
> struct list_head upper;
> - /* list of child blocks in the cache */
> + /* List of lower level edges, which links this node to its child(ren) */
> struct list_head lower;
> +
> /* NULL if this node is not tree root */
> struct btrfs_root *root;
> /* extent buffer got by COW the block */
> @@ -123,17 +125,26 @@ struct backref_node {
> unsigned int detached:1;
> };
>
> +#define LOWER 0
> +#define UPPER 1
> +#define RELOCATION_RESERVED_NODES 256
> /*
> - * present a block pointer in the backref cache
> + * present an edge connecting upper and lower backref nodes.
> */
> struct backref_edge {
> + /*
> + * list[LOWER] is linked to backref_node::upper of lower level node,
> + * and list[UPPER] is linked to backref_node::lower of upper level node.
> + *
> + * Also, build_backref_tree() uses list[UPPER] for pending edges, before
> + * linking list[UPPER] to its upper level nodes.
> + */
> struct list_head list[2];
> +
> + /* Two related nodes */
> struct backref_node *node[2];
> };
>
> -#define LOWER 0
> -#define UPPER 1
> -#define RELOCATION_RESERVED_NODES 256
>
> struct backref_cache {
> /* red black tree of all backref nodes in the cache */
> @@ -332,6 +343,22 @@ static struct backref_edge *alloc_backref_edge(struct backref_cache *cache)
> return edge;
> }
>
> +#define LINK_LOWER (1 << 0)
> +#define LINK_UPPER (1 << 1)
> +static inline void link_backref_edge(struct backref_edge *edge,
> + struct backref_node *lower,
> + struct backref_node *upper,
> + int link_which)
Again not a static inline, but plain static function.
> +{
> + ASSERT(upper && lower && upper->level == lower->level + 1);
> + edge->node[LOWER] = lower;
> + edge->node[UPPER] = upper;
> + if (link_which & LINK_LOWER)
> + list_add_tail(&edge->list[LOWER], &lower->upper);
> + if (link_which & LINK_UPPER)
> + list_add_tail(&edge->list[UPPER], &upper->lower);
> +}
