Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 8 Jun 2022 11:45:57 +0800
From:   Baoquan He <bhe@redhat.com>
To:     "Uladzislau Rezki (Sony)" <urezki@gmail.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org,
        LKML <linux-kernel@vger.kernel.org>,
        Christoph Hellwig <hch@infradead.org>,
        Matthew Wilcox <willy@infradead.org>,
        Nicholas Piggin <npiggin@gmail.com>,
        Oleksiy Avramchenko <oleksiy.avramchenko@sony.com>
Subject: Re: [PATCH 1/5] mm/vmalloc: Make link_va()/unlink_va() common to
 different rb_root
Message-ID: <YqAbdQ1voSCCaZtz@MiWiFi-R3L-srv>
References: <20220607093449.3100-1-urezki@gmail.com>
 <20220607093449.3100-2-urezki@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220607093449.3100-2-urezki@gmail.com>
Precedence: bulk

On 06/07/22 at 11:34am, Uladzislau Rezki (Sony) wrote:
> Currently link_va() and unlik_va(), in order to figure out a tree
> type, compares a passed root value with a global free_vmap_area_root
> variable to distinguish the augmented rb-tree from a regular one. It
> is hard coded since such functions can manipulate only with specific
> "free_vmap_area_root" tree that represents a global free vmap space.
> 
> Make it common by introducing "_augment" versions of both internal
> functions, so it is possible to deal with different trees.
> 
> There is no functional change as a result of this patch.

Patch looks good to me. Looking forward to seeing the later per-cpu
vmalloc allocation patches.

Reviewed-by: Baoquan He <bhe@redhat.com>

> 
> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> ---
>  mm/vmalloc.c | 60 +++++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 48 insertions(+), 12 deletions(-)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index be8ed06804a5..0102d6d5fcdf 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -911,8 +911,9 @@ get_va_next_sibling(struct rb_node *parent, struct rb_node **link)
>  }
>  
>  static __always_inline void
> -link_va(struct vmap_area *va, struct rb_root *root,
> -	struct rb_node *parent, struct rb_node **link, struct list_head *head)
> +__link_va(struct vmap_area *va, struct rb_root *root,
> +	struct rb_node *parent, struct rb_node **link,
> +	struct list_head *head, bool augment)
>  {
>  	/*
>  	 * VA is still not in the list, but we can
> @@ -926,7 +927,7 @@ link_va(struct vmap_area *va, struct rb_root *root,
>  
>  	/* Insert to the rb-tree */
>  	rb_link_node(&va->rb_node, parent, link);
> -	if (root == &free_vmap_area_root) {
> +	if (augment) {
>  		/*
>  		 * Some explanation here. Just perform simple insertion
>  		 * to the tree. We do not set va->subtree_max_size to
> @@ -950,12 +951,28 @@ link_va(struct vmap_area *va, struct rb_root *root,
>  }
>  
>  static __always_inline void
> -unlink_va(struct vmap_area *va, struct rb_root *root)
> +link_va(struct vmap_area *va, struct rb_root *root,
> +	struct rb_node *parent, struct rb_node **link,
> +	struct list_head *head)
> +{
> +	__link_va(va, root, parent, link, head, false);
> +}
> +
> +static __always_inline void
> +link_va_augment(struct vmap_area *va, struct rb_root *root,
> +	struct rb_node *parent, struct rb_node **link,
> +	struct list_head *head)
> +{
> +	__link_va(va, root, parent, link, head, true);
> +}
> +
> +static __always_inline void
> +__unlink_va(struct vmap_area *va, struct rb_root *root, bool augment)
>  {
>  	if (WARN_ON(RB_EMPTY_NODE(&va->rb_node)))
>  		return;
>  
> -	if (root == &free_vmap_area_root)
> +	if (augment)
>  		rb_erase_augmented(&va->rb_node,
>  			root, &free_vmap_area_rb_augment_cb);
>  	else
> @@ -965,6 +982,18 @@ unlink_va(struct vmap_area *va, struct rb_root *root)
>  	RB_CLEAR_NODE(&va->rb_node);
>  }
>  
> +static __always_inline void
> +unlink_va(struct vmap_area *va, struct rb_root *root)
> +{
> +	__unlink_va(va, root, false);
> +}
> +
> +static __always_inline void
> +unlink_va_augment(struct vmap_area *va, struct rb_root *root)
> +{
> +	__unlink_va(va, root, true);
> +}
> +
>  #if DEBUG_AUGMENT_PROPAGATE_CHECK
>  /*
>   * Gets called when remove the node and rotate.
> @@ -1060,7 +1089,7 @@ insert_vmap_area_augment(struct vmap_area *va,
>  		link = find_va_links(va, root, NULL, &parent);
>  
>  	if (link) {
> -		link_va(va, root, parent, link, head);
> +		link_va_augment(va, root, parent, link, head);
>  		augment_tree_propagate_from(va);
>  	}
>  }
> @@ -1077,8 +1106,8 @@ insert_vmap_area_augment(struct vmap_area *va,
>   * ongoing.
>   */
>  static __always_inline struct vmap_area *
> -merge_or_add_vmap_area(struct vmap_area *va,
> -	struct rb_root *root, struct list_head *head)
> +__merge_or_add_vmap_area(struct vmap_area *va,
> +	struct rb_root *root, struct list_head *head, bool augment)
>  {
>  	struct vmap_area *sibling;
>  	struct list_head *next;
> @@ -1140,7 +1169,7 @@ merge_or_add_vmap_area(struct vmap_area *va,
>  			 * "normalized" because of rotation operations.
>  			 */
>  			if (merged)
> -				unlink_va(va, root);
> +				__unlink_va(va, root, augment);
>  
>  			sibling->va_end = va->va_end;
>  
> @@ -1155,16 +1184,23 @@ merge_or_add_vmap_area(struct vmap_area *va,
>  
>  insert:
>  	if (!merged)
> -		link_va(va, root, parent, link, head);
> +		__link_va(va, root, parent, link, head, augment);
>  
>  	return va;
>  }
>  
> +static __always_inline struct vmap_area *
> +merge_or_add_vmap_area(struct vmap_area *va,
> +	struct rb_root *root, struct list_head *head)
> +{
> +	return __merge_or_add_vmap_area(va, root, head, false);
> +}
> +
>  static __always_inline struct vmap_area *
>  merge_or_add_vmap_area_augment(struct vmap_area *va,
>  	struct rb_root *root, struct list_head *head)
>  {
> -	va = merge_or_add_vmap_area(va, root, head);
> +	va = __merge_or_add_vmap_area(va, root, head, true);
>  	if (va)
>  		augment_tree_propagate_from(va);
>  
> @@ -1348,7 +1384,7 @@ adjust_va_to_fit_type(struct vmap_area *va,
>  		 * V      NVA      V
>  		 * |---------------|
>  		 */
> -		unlink_va(va, &free_vmap_area_root);
> +		unlink_va_augment(va, &free_vmap_area_root);
>  		kmem_cache_free(vmap_area_cachep, va);
>  	} else if (type == LE_FIT_TYPE) {
>  		/*
> -- 
> 2.30.2
> 
>