Re: [f2fs-dev] [PATCH 1/3] f2fs: factor out victim_entry usage from general rb_tree use

[Eric Biggers]

On Fri, Mar 10, 2023 at 01:04:52PM -0800, Jaegeuk Kim wrote:
> Let's reduce the complexity of mixed use of rb_tree in victim_entry from
> extent_cache and discard_cmd.
> 
> This should fix arm32 memory alignment issue caused by shared rb_entry.
> 
> [struct victim_entry]              [struct rb_entry]
> [0] struct rb_node rb_node;        [0] struct rb_node rb_node;
>                                        union {
>                                          struct {
>                                            unsigned int ofs;
>                                            unsigned int len;
>                                          };
> [16] unsigned long long mtime;     [12] unsigned long long key;
>                                        } __packed;
> 
> Cc: <stable@xxxxxxxxxxxxxxx>
> Fixes: 093749e296e2 ("f2fs: support age threshold based garbage collection")
> Signed-off-by: Jaegeuk Kim <jaegeuk@xxxxxxxxxx>

Thanks for fixing this properly.  It looks much better than the weird type
punning that was being done before...

> +static struct rb_node **f2fs_lookup_rb_tree_ext(struct f2fs_sb_info *sbi,
> +				struct rb_root_cached *root,
> +				struct rb_node **parent,
> +				unsigned long long mtime, bool *leftmost)

Call this f2fs_lookup_victim_entry()?

> +static bool f2fs_check_victim_tree(struct f2fs_sb_info *sbi,
> +				struct rb_root_cached *root)
> +{
> +#ifdef CONFIG_F2FS_CHECK_FS
> +	struct rb_node *cur = rb_first_cached(root), *next;
> +	struct victim_entry *cur_ve, *next_ve;
> +
> +	if (!cur)
> +		return true;
> +
> +	while (cur) {

The !cur check is redundant.

- Eric