Received-SPF: pass (google.com: domain of linux-kernel+bounces-195004-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Thu, 30 May 2024 15:18:55 +0800
From: Baoquan He <bhe@redhat.com>
To: "zhaoyang.huang" <zhaoyang.huang@unisoc.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Uladzislau Rezki <urezki@gmail.com>,
	Christoph Hellwig <hch@infradead.org>,
	Lorenzo Stoakes <lstoakes@gmail.com>, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org,
	Zhaoyang Huang <huangzhaoyang@gmail.com>, steve.kang@unisoc.com
Subject: Re: [PATCH] mm: fix incorrect vbq reference in purge_fragmented_block
Message-ID: <ZlgoX1E4/juuP7+o@MiWiFi-R3L-srv>
References: <20240530025144.1570865-1-zhaoyang.huang@unisoc.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240530025144.1570865-1-zhaoyang.huang@unisoc.com>

On 05/30/24 at 10:51am, zhaoyang.huang wrote:
> From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> 
> Broken vbq->free reported on a v6.6 based system which is caused
> by invalid vbq->lock protect over vbq->free in purge_fragmented_block.
> This should be introduced by the Fixes below which ignored vbq->lock
> matter.

It will be helpful to provide more details, what's the symptom of the
brekage, and in which case vbq->free is broken.

> 
> Fixes: fc1e0d980037 ("mm/vmalloc: prevent stale TLBs in fully utilized blocks")
> 
> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> ---
>  mm/vmalloc.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 22aa63f4ef63..112b50431725 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2614,9 +2614,10 @@ static void free_vmap_block(struct vmap_block *vb)
>  }
>  
>  static bool purge_fragmented_block(struct vmap_block *vb,
> -		struct vmap_block_queue *vbq, struct list_head *purge_list,
> -		bool force_purge)
> +		struct list_head *purge_list, bool force_purge)
>  {
> +	struct vmap_block_queue *vbq;
> +
>  	if (vb->free + vb->dirty != VMAP_BBMAP_BITS ||
>  	    vb->dirty == VMAP_BBMAP_BITS)
>  		return false;
> @@ -2625,6 +2626,8 @@ static bool purge_fragmented_block(struct vmap_block *vb,
>  	if (!(force_purge || vb->free < VMAP_PURGE_THRESHOLD))
>  		return false;
>  
> +	vbq = container_of(addr_to_vb_xa(vb->va->va_start),
> +		struct vmap_block_queue, vmap_blocks);
>  	/* prevent further allocs after releasing lock */
>  	WRITE_ONCE(vb->free, 0);
>  	/* prevent purging it again */
> @@ -2664,7 +2667,7 @@ static void purge_fragmented_blocks(int cpu)
>  			continue;
>  
>  		spin_lock(&vb->lock);
> -		purge_fragmented_block(vb, vbq, &purge, true);
> +		purge_fragmented_block(vb, &purge, true);
>  		spin_unlock(&vb->lock);
>  	}
>  	rcu_read_unlock();
> @@ -2801,7 +2804,7 @@ static void _vm_unmap_aliases(unsigned long start, unsigned long end, int flush)
>  			 * not purgeable, check whether there is dirty
>  			 * space to be flushed.
>  			 */
> -			if (!purge_fragmented_block(vb, vbq, &purge_list, false) &&
> +			if (!purge_fragmented_block(vb, &purge_list, false) &&
>  			    vb->dirty_max && vb->dirty != VMAP_BBMAP_BITS) {
>  				unsigned long va_start = vb->va->va_start;
>  				unsigned long s, e;
> -- 
> 2.25.1
> 
>