Re: [PATCH] mm: Always initialise folio->_deferred_list

[David Hildenbrand <david@redhat.com>]

On 12.03.24 15:01, Matthew Wilcox wrote:
> On Tue, Mar 12, 2024 at 02:17:01PM +0100, David Hildenbrand wrote:
>> On 12.03.24 04:50, Matthew Wilcox (Oracle) wrote:
>>> +++ b/mm/page_alloc.c
>>> @@ -1006,10 +1006,11 @@ static int free_tail_page_prepare(struct page *head_page, struct page *page)
>>>    		}
>>>    		break;
>>>    	case 2:
>>> -		/*
>>> -		 * the second tail page: ->mapping is
>>> -		 * deferred_list.next -- ignore value.
>>> -		 */
>>> +		/* the second tail page: deferred_list overlaps ->mapping */
>>> +		if (unlikely(!list_empty(&folio->_deferred_list))) {
>>> +			bad_page(page, "on deferred list");
>>> +			goto out;
>>> +		}
>>
>> IIRC, hugetlb might overwrite this with _hugetlb_subpool? Isn't that a
>> problem we have to handle?
> 
> Ah yes, you're right.  I think this should do the trick?
> 
> +++ b/mm/hugetlb.c
> @@ -1796,6 +1796,7 @@ static void __update_and_free_hugetlb_folio(struct hstate *h,
>                  destroy_compound_gigantic_folio(folio, huge_page_order(h));
>                  free_gigantic_folio(folio, huge_page_order(h));
>          } else {
> +               INIT_LIST_HEAD(&folio->_deferred_list);
>                  __free_pages(&folio->page, huge_page_order(h));

Heh, __free_pages() says:

"This function can free multi-page allocations that are not compound 
pages". I suspect that means "can also free compound pages", but it's 
confusing.

Especially, I thought we recently learned that free hugetlb folios do 
have a refcount of 0? Confusing.

Anyhow, I suspect your change does the trick and agree that 
__free_pages() might need a replacement.

... but it's confusing.

-- 
Cheers,

David / dhildenb