On 1/27/25 16:50, Lorenzo Stoakes wrote:
> --- a/mm/vma.h
> +++ b/mm/vma.h
> @@ -67,6 +67,16 @@ enum vma_merge_flags {
> * at the gap.
> */
> VMG_FLAG_JUST_EXPAND = 1 << 0,
> + /*
> + * Internal flag used during the merge operation to indicate we will
> + * remove vmg->middle.
> + */
> + __VMG_FLAG_REMOVE_MIDDLE = 1 << 1,
> + /*
> + * Internal flag used during the merge operationr to indicate we will
> + * remove vmg->next.
> + */
> + __VMG_FLAG_REMOVE_NEXT = 1 << 2,
> };
Hm this is actually kinda weird? It's an enum, but the values of it are
defined as different bits. And then struct vma_merge_struct has a "enum
vma_merge_flags merge_flags;" but we don't store to it a single "enum
vma_merge_flags" value defined above, but a combination of those. Is that
even legal to do in C?
AFAIK the more common pattern is enum that has normal incremental values
that are used for the shifts.
But I don't think we need all of this at all here? Just have bitfields in
struct vma_merge_struct?
bool just_expand : 1;
bool remove_middle : 1;
...
> /*
> diff --git a/tools/testing/vma/vma.c b/tools/testing/vma/vma.c
> index 3c0572120e94..8cce67237d86 100644
> --- a/tools/testing/vma/vma.c
> +++ b/tools/testing/vma/vma.c
> @@ -154,6 +154,9 @@ static void vmg_set_range(struct vma_merge_struct *vmg, unsigned long start,
> vmg->end = end;
> vmg->pgoff = pgoff;
> vmg->flags = flags;
> +
> + vmg->merge_flags = VMG_FLAG_DEFAULT;
> + vmg->target = NULL;
> }
>
> /*
