Received-SPF: pass (google.com: domain of linux-kernel+bounces-59955-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <28c47a9f-5ee4-425e-bd8d-bd251634534c@suse.cz>
Date: Fri, 9 Feb 2024 21:46:09 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3 2/3] mm/compaction: add support for >0 order folio
 memory compaction.
Content-Language: en-US
To: Zi Yan <ziy@nvidia.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
 "Huang, Ying" <ying.huang@intel.com>, Ryan Roberts <ryan.roberts@arm.com>,
 Andrew Morton <akpm@linux-foundation.org>,
 "Matthew Wilcox (Oracle)" <willy@infradead.org>,
 David Hildenbrand <david@redhat.com>, "Yin, Fengwei"
 <fengwei.yin@intel.com>, Yu Zhao <yuzhao@google.com>,
 "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
 Johannes Weiner <hannes@cmpxchg.org>,
 Baolin Wang <baolin.wang@linux.alibaba.com>,
 Kemeng Shi <shikemeng@huaweicloud.com>,
 Mel Gorman <mgorman@techsingularity.net>, Rohan Puri
 <rohan.puri15@gmail.com>, Mcgrof Chamberlain <mcgrof@kernel.org>,
 Adam Manzanares <a.manzanares@samsung.com>,
 "Vishal Moola (Oracle)" <vishal.moola@gmail.com>
References: <20240202161554.565023-1-zi.yan@sent.com>
 <20240202161554.565023-3-zi.yan@sent.com>
 <025b7e7c-b17f-47c7-8677-ee36fc6dbc52@suse.cz>
 <5F394663-0AFE-4C12-827C-3A0723863395@nvidia.com>
 <193E0D43-0084-48AA-91F5-8316A130ADC3@nvidia.com>
From: Vlastimil Babka <vbabka@suse.cz>
In-Reply-To: <193E0D43-0084-48AA-91F5-8316A130ADC3@nvidia.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

On 2/9/24 20:40, Zi Yan wrote:
> On 9 Feb 2024, at 14:36, Zi Yan wrote:
> 
>> On 9 Feb 2024, at 11:37, Vlastimil Babka wrote:
>>
>>> On 2/2/24 17:15, Zi Yan wrote:
>>>
>>> ...
>>>
>>>>  /*
>>>> @@ -1835,9 +1857,17 @@ static struct folio *compaction_alloc(struct folio *src, unsigned long data)
>>>>  static void compaction_free(struct folio *dst, unsigned long data)
>>>>  {
>>>>  	struct compact_control *cc = (struct compact_control *)data;
>>>> +	int order = folio_order(dst);
>>>> +	struct page *page = &dst->page;
>>>> +
>>>> +	folio_set_count(dst, 0);
>>>
>>> We can't change refcount to 0 like this, after it was already set to 1 and
>>> somebody else might have done get_page_unless_zero(). You need to either
>>> put_page_testzero() and if it's false, consider the page lost, or leave it
>>> refcounted and adjust the code to handle both refcounted and non-refcounted
>>> pages on the lists (the first option is simpler and shouldn't be too bad).
>> Got it. Will fix it with the first option. Thanks.
> 
> Do you think we should have a WARN or WARN_ONCE if we lose a page here?

No, no WARN, it all happens legitimately. It's only our compaction losing
the page - whoever would do the get_page_unless_zero() to inspect that page
would then have to put_page() which will free it back to page allocator.

> --
> Best Regards,
> Yan, Zi