Re: [PATCH 2/3] md: Use folio APIs in free_page()

[Zi Yan <ziy@nvidia.com>]

On 14 Sep 2026, at 12:21, Matthew Wilcox wrote:

> On Mon, Sep 14, 2026 at 04:31:29AM +0000, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>>
>> Pre-existing issues:
>> - [High] Memory leak of internal bitmap pages and uninitialized file bitmap pages during teardown or error handling due to an early return in free_buffers().
>> --
>
> Yes, I thought the early return looked weird.  I just believed that I
> hadn't looked into the code enough to understand why it looked weird but
> was right.  Since the AI says it's wrong, I'm happy to believe it's wrong.
> It's probably not really a high seveirty problem because most RAIDs are
> set up once and then never removed, so the leak doesn't matter.
>
> The other thing that looks weird is that this code believes the b_this_page
> list is NULL terminated.  Most loops over "all BHs attached to a folio"
> have a circular list.  However, that is established by link_dev_buffers()
> which is not called by md-bitmap.  So it's correct, even though weird.
>
> The good news is this is an easy fix:
>
> @@ -536,9 +536,6 @@ static void free_buffers(struct page *page)
>         struct folio *folio = page_folio(page);
>         struct buffer_head *bh = folio_detach_private(folio);
>
> -       if (!bh)
> -               return;
> -
>         while (bh) {
>                 struct buffer_head *next = bh->b_this_page;
>                 free_buffer_head(bh);
>
> I can submit a new vrsion, or Zi can just pick this up.

I can fold this into your patch. Or should this be sent separately
as a fix and maybe a backport?

>
>> commit d68f524234e61e6567e52a27e09b9d2546ab54af
>> Author: Matthew Wilcox (Oracle) <willy@infradead.org>
>>
>> md: Use folio APIs in free_page()
>>
>> This commit converts the page to a folio, removing some of the last uses of
>> a few page APIs and replacing two calls to compound_head() with one.
>>
>>> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
>>> index 5f1637f974c15..7b42db87d536d 100644
>>> --- a/drivers/md/md-bitmap.c
>>> +++ b/drivers/md/md-bitmap.c
>>> @@ -533,19 +533,18 @@ static void write_file_page(struct bitmap *bitmap, struct page *page, int wait)
>>>
>>>  static void free_buffers(struct page *page)
>>>  {
>>> -	struct buffer_head *bh;
>>> +	struct folio *folio = page_folio(page);
>>> +	struct buffer_head *bh = folio_detach_private(folio);
>>>
>>> -	if (!PagePrivate(page))
>>> +	if (!bh)
>>>  		return;
>>
>> [Severity: High]
>> This is a pre-existing issue, but this early return skips the folio_put()
>> required to drop the base allocation reference for pages that do not have
>> buffers attached.
>>
>> For internal bitmaps, pages are allocated via alloc_page() with a refcount
>> of 1 in md_bitmap_storage_alloc():
>>
>> drivers/md/md-bitmap.c:md_bitmap_storage_alloc() {
>> 	...
>> 	store->filemap[pnum] = alloc_page(GFP_KERNEL|__GFP_ZERO);
>> 	...
>> }
>>
>> During MD array teardown or failure, md_bitmap_file_unmap() calls
>> free_buffers() on these pages:
>>
>> drivers/md/md-bitmap.c:md_bitmap_file_unmap() {
>> 	...
>> 	free_buffers(map[pages]);
>> 	...
>> }
>>
>> Because internal bitmap pages don't have buffers attached, (!bh) evaluates
>> to true, triggering the early return and completely bypassing the folio_put()
>> at the end of the function.
>>
>> This permanently leaks the pages allocated for the internal bitmap's filemap
>> and superblock.
>>
>> Does this early return need to be updated so that folio_put() is still
>> called for folios without buffers?
>>
>>> -	bh = page_buffers(page);
>>>  	while (bh) {
>>>  		struct buffer_head *next = bh->b_this_page;
>>>  		free_buffer_head(bh);
>>>  		bh = next;
>>>  	}
>>> -	detach_page_private(page);
>>> -	put_page(page);
>>> +	folio_put(folio);
>>>  }
>>
>> -- 
>> Sashiko AI review · https://sashiko.dev/#/patchset/20260913-remove-pg_private-v4-0-848550f7574e@nvidia.com?part=2


Best Regards,
Yan, Zi