linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Zi Yan <ziy@nvidia.com>
To: SeongJae Park <sj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	David Hildenbrand <david@kernel.org>,
	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>,
	"Liam R. Howlett" <Liam.Howlett@oracle.com>,
	Vlastimil Babka <vbabka@kernel.org>,
	Mike Rapoport <rppt@kernel.org>,
	Suren Baghdasaryan <surenb@google.com>,
	Michal Hocko <mhocko@suse.com>,
	Kefeng Wang <wangkefeng.wang@huawei.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Ron Economos <re@w6rz.net>
Subject: Re: [PATCH v2] mm/cma: move put_page_testzero() out of VM_WARN_ON in cma_release()
Date: Tue, 24 Feb 2026 22:04:15 -0500	[thread overview]
Message-ID: <7AAE3B00-627F-4F40-A383-7F5FDCBAC8F5@nvidia.com> (raw)
In-Reply-To: <20260225025440.63897-1-sj@kernel.org>

On 24 Feb 2026, at 21:54, SeongJae Park wrote:

> On Tue, 24 Feb 2026 21:28:52 -0500 Zi Yan <ziy@nvidia.com> wrote:
>
>> When CONFIG_DEBUG_VM is not set, VM_WARN_ON is a NOP. Putting any statement
>> with side effect inside it is incorrect. Collect all put_page_testzero()
>> results and check the sum using WARN instead after the loop.
>>
>> Fixes: 9bda131c6093 ("mm: cma: add cma_alloc_frozen{_compound}()")
>> Reported-by: Ron Economos <re@w6rz.net>
>> Closes: https://lore.kernel.org/all/1b17c38f-30d3-4bb4-a7e1-e74b19ada885@w6rz.net/
>> Suggested-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>> ---
>>> From V1:
>> - Collect all put_page_testzero() results and do a single WARN after the
>>   loop.
>>
>>  mm/cma.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/cma.c b/mm/cma.c
>> index 94b5da468a7d..ea540d3f473c 100644
>> --- a/mm/cma.c
>> +++ b/mm/cma.c
>> @@ -1013,6 +1013,7 @@ bool cma_release(struct cma *cma, const struct page *pages,
>>  		 unsigned long count)
>>  {
>>  	struct cma_memrange *cmr;
>> +	unsigned long ret = 0;
>>  	unsigned long i, pfn;
>>
>>  	cmr = find_cma_memrange(cma, pages, count);
>> @@ -1021,7 +1022,9 @@ bool cma_release(struct cma *cma, const struct page *pages,
>>
>>  	pfn = page_to_pfn(pages);
>>  	for (i = 0; i < count; i++, pfn++)
>> -		VM_WARN_ON(!put_page_testzero(pfn_to_page(pfn)));
>> +		ret += put_page_testzero(pfn_to_page(pfn));
>
> So, 'ret' will be incremented when put_page_testzero() returns 'true', right?
> Shouldn't 'ret' be inceremented only when put_page_testzero() returns 'false'?

Good catch. Let me fix it. Thanks.

>
>> +
>> +	WARN(ret, "%lu pages are still in use!\n", ret);
>
> This will trigger the warning even if CONFIG_DEBUG_VM is unset.  I'm fine with
> the changed behavior, but I'm curious if that is the real intention of this
> revision.

It was in free_contig_range()[1], which was called by cma_release() before.

[1] https://elixir.bootlin.com/linux/v6.19/source/mm/page_alloc.c#L7238


--
Best Regards,
Yan, Zi


      reply	other threads:[~2026-02-25  3:04 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-25  2:28 Zi Yan
2026-02-25  2:54 ` SeongJae Park
2026-02-25  3:04   ` Zi Yan [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7AAE3B00-627F-4F40-A383-7F5FDCBAC8F5@nvidia.com \
    --to=ziy@nvidia.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=mhocko@suse.com \
    --cc=re@w6rz.net \
    --cc=rppt@kernel.org \
    --cc=sj@kernel.org \
    --cc=surenb@google.com \
    --cc=vbabka@kernel.org \
    --cc=wangkefeng.wang@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox