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, 18 Jan 2023 17:16:46 +0000
From:   Catalin Marinas <catalin.marinas@arm.com>
To:     "Isaac J. Manjarres" <isaacmanjarres@google.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Saravana Kannan <saravanak@google.com>,
        Suren Baghdasaryan <surenb@google.com>,
        kernel-team@android.com, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 0/2] Fixes for kmemleak tracking with CMA regions
Message-ID: <Y8gpfgEXtialPVLk@arm.com>
References: <20230109221624.592315-1-isaacmanjarres@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230109221624.592315-1-isaacmanjarres@google.com>
Precedence: bulk

Hi Isaac,

Please cc me on kmemleak patches. I only noticed when Andrew picket them
up.

On Mon, Jan 09, 2023 at 02:16:21PM -0800, Isaac J. Manjarres wrote:
> When trying to boot a device with an ARM64 kernel with the following
> config options enabled:
> 
> CONFIG_DEBUG_PAGEALLOC=y
> CONFIG_DEBUG_PAGEALLOC_ENABLE_DEFAULT=y
> CONFIG_DEBUG_KMEMLEAK=y
> 
> a page-fault is encountered when kmemleak starts to scan the list of gray
> or allocated objects that it maintains. Upon closer inspection, it was
> observed that these page-faults always occurred when kmemleak attempted
> to scan a CMA region.

What I don't understand is why kmemleak scans such CMA regions. The only
reason for a kmemleak_ignore_phys() call in cma_declare_contiguous_nid()
is because the kmemleak_alloc_phys() hook was called on the
memblock_alloc_range_nid() path, so we don't want this scanned.

Do you have a backtrace?

> At the moment, kmemleak is made aware of CMA regions that are specified
> through the devicetree to be created at specific memory addresses or
> dynamically allocated within a range of addresses. However, if the
> CMA region is constrained to a certain range of addresses through the
> command line, the region is reserved through the memblock_reserve()
> function, but kmemleak_alloc_phys() is not invoked.

The combination of kmemleak_alloc_phys() + kmemleak_free_part_phys() in
your series is equivalent to not adding it at all in the first place.

> Furthermore,
> kmemleak is never informed about CMA regions being freed to buddy at
> boot, which is problematic when CONFIG_DEBUG_PAGEALLOC is enabled, as
> all CMA regions are unmapped from the kernel's address space, and
> subsequently causes a page-fault when kmemleak attempts to scan any
> of them.

kmemleak would only scan such objects if it knows about them. So I think
it's only the case where CMA does a memblock allocation. The
kmemleak_ignore_phys() should tell kmemleak not to touch this region but
it's probably better to just free it altogether (i.e. replace the ignore
with the free kmemleak callback). Would this be sufficient for your
scenario?

> This series makes it so that kmemleak is aware of every CMA region before
> they are freed to the buddy allocator, so that at that time, kmemleak
> can be informed that each region is about to be freed, and thus it
> should not attempt to scan those regions.

I may be missing something but I don't get why kmemleak needs to be
informed only to tell kmemleak shortly after to remove them from its
list of objects.

-- 
Catalin