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:   Thu, 9 Jun 2022 19:06:36 +0100
From:   Catalin Marinas <catalin.marinas@arm.com>
To:     Patrick Wang <patrick.wang.shcn@gmail.com>
Cc:     akpm@linux-foundation.org, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, yee.lee@mediatek.com
Subject: Re: [PATCH v3 1/3] mm: kmemleak: add OBJECT_PHYS flag for objects
 allocated with physical address
Message-ID: <YqI2rE+YB/+06t3w@arm.com>
References: <20220609124950.1694394-1-patrick.wang.shcn@gmail.com>
 <20220609124950.1694394-2-patrick.wang.shcn@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220609124950.1694394-2-patrick.wang.shcn@gmail.com>
Precedence: bulk

On Thu, Jun 09, 2022 at 08:49:48PM +0800, Patrick Wang wrote:
> @@ -1125,15 +1142,18 @@ EXPORT_SYMBOL(kmemleak_no_scan);
>   *			 address argument
>   * @phys:	physical address of the object
>   * @size:	size of the object
> - * @min_count:	minimum number of references to this object.
> - *              See kmemleak_alloc()
>   * @gfp:	kmalloc() flags used for kmemleak internal memory allocations
>   */
> -void __ref kmemleak_alloc_phys(phys_addr_t phys, size_t size, int min_count,
> -			       gfp_t gfp)
> +void __ref kmemleak_alloc_phys(phys_addr_t phys, size_t size, gfp_t gfp)
>  {
> +	pr_debug("%s(0x%pa, %zu)\n", __func__, &phys, size);
> +
>  	if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
> -		kmemleak_alloc(__va(phys), size, min_count, gfp);
> +		/*
> +		 * Create object with OBJECT_PHYS flag and
> +		 * assume min_count 0.
> +		 */
> +		create_object_phys((unsigned long)__va(phys), size, 0, gfp);
>  }
>  EXPORT_SYMBOL(kmemleak_alloc_phys);
>  
> diff --git a/mm/memblock.c b/mm/memblock.c
> index e4f03a6e8e56..749abd2685c4 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -1345,8 +1345,8 @@ __next_mem_pfn_range_in_zone(u64 *idx, struct zone *zone,
>   * from the regions with mirroring enabled and then retried from any
>   * memory region.
>   *
> - * In addition, function sets the min_count to 0 using kmemleak_alloc_phys for
> - * allocated boot memory block, so that it is never reported as leaks.
> + * In addition, function using kmemleak_alloc_phys for allocated boot
> + * memory block, it is never reported as leaks.
>   *
>   * Return:
>   * Physical address of allocated memory block on success, %0 on failure.
> @@ -1398,12 +1398,12 @@ phys_addr_t __init memblock_alloc_range_nid(phys_addr_t size,
>  	 */
>  	if (end != MEMBLOCK_ALLOC_NOLEAKTRACE)
>  		/*
> -		 * The min_count is set to 0 so that memblock allocated
> -		 * blocks are never reported as leaks. This is because many
> -		 * of these blocks are only referred via the physical
> -		 * address which is not looked up by kmemleak.
> +		 * Memblock allocated blocks are never reported as
> +		 * leaks. This is because many of these blocks are
> +		 * only referred via the physical address which is
> +		 * not looked up by kmemleak.
>  		 */
> -		kmemleak_alloc_phys(found, size, 0, 0);
> +		kmemleak_alloc_phys(found, size, 0);
>  
>  	return found;
>  }
> diff --git a/tools/testing/memblock/linux/kmemleak.h b/tools/testing/memblock/linux/kmemleak.h
> index 462f8c5e8aa0..5fed13bb9ec4 100644
> --- a/tools/testing/memblock/linux/kmemleak.h
> +++ b/tools/testing/memblock/linux/kmemleak.h
> @@ -7,7 +7,7 @@ static inline void kmemleak_free_part_phys(phys_addr_t phys, size_t size)
>  }
>  
>  static inline void kmemleak_alloc_phys(phys_addr_t phys, size_t size,
> -				       int min_count, gfp_t gfp)
> +				       gfp_t gfp)
>  {
>  }

If you respin, I'd move the prototype change to a separate patch (and
make it first in the series). Otherwise it looks fine:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>