From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0179D232395 for ; Mon, 4 May 2026 16:14:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.176.79.56 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777911286; cv=none; b=hqg4FURtC1SSyaYdsf7CTjJMIsDkfEkiX/nMiOoGyjy1nuKjAOOdUR45e8PWY/2i+bpLn0jtuiVpoTtqZTHcKs9wJ92CLbeZD+fqOCsUlU6Msu4Xafi0OP7EUOBnOTqrBffAcRkKtoy5hCJLN/7+nSQGFT8/y8uFNDE2Mi/eST8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777911286; c=relaxed/simple; bh=rEQQ+YdkEbnai34QkvFu2sZWIYdtNYB5QteEB8ntyh4=; h=Message-ID:Date:MIME-Version:Subject:To:CC:References:From: In-Reply-To:Content-Type; b=KZBVQ5OuGhaLwmFSNKshhLgNegfdjoXfvVg6RYtnd/l72yC02FsGYyKgxL2bV9IWfRMGMTk4LcA45A5jlOCdIOCw2+yvTaoN2JSsQC7+Qej1ZDejyauRfhLaAgrNG3MD2pbB/i1uLMGrZmjY/mouHopilcFuSc17gBWdH52RGh8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=h-partners.com; spf=pass smtp.mailfrom=h-partners.com; arc=none smtp.client-ip=185.176.79.56 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=h-partners.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=h-partners.com Received: from mail.maildlp.com (unknown [172.18.224.150]) by frasgout.his.huawei.com (SkyGuard) with ESMTPS id 4g8RYF5kG5zJ46XM; Tue, 5 May 2026 00:14:25 +0800 (CST) Received: from mscpeml500003.china.huawei.com (unknown [7.188.49.51]) by mail.maildlp.com (Postfix) with ESMTPS id CAA0E40572; Tue, 5 May 2026 00:14:39 +0800 (CST) Received: from [10.123.122.186] (10.123.122.186) by mscpeml500003.china.huawei.com (7.188.49.51) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Mon, 4 May 2026 19:14:39 +0300 Message-ID: <616a62b0-680f-4a86-8ca9-75bba10684dd@h-partners.com> Date: Mon, 4 May 2026 19:14:38 +0300 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 1/2] mm: drop page refcount zero state semantics Content-Language: en-GB To: Zi Yan CC: , , , , , , , , , , , , , , , , , , , References: <9fd8ebbc0f4f45be611bae0d03dd25dd994233c0.1776350895.git.gorbunov.ivan@h-partners.com> <53FA8505-E7C0-404B-B7F5-3EBE89B64E0C@nvidia.com> From: Gorbunov Ivan In-Reply-To: <53FA8505-E7C0-404B-B7F5-3EBE89B64E0C@nvidia.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: mscpeml100003.china.huawei.com (10.199.174.67) To mscpeml500003.china.huawei.com (7.188.49.51) On 4/23/2026 10:32 PM, Zi Yan wrote: > On 20 Apr 2026, at 4:01, Gorbunov Ivan wrote: > >> This patch proposes 2 changes >> 1) Deprecate invariant that the value stored in reference count of frozen page is 0 >> (Getter functions folio_ref_count/page_ref_count must still return 0 for frozen pages) >> 2) Allow modification operations like page_ref_add to be used only with >> pages with owners > > Should we also ban calling set_page_count() except init_page_count() and > set_page_count_as_frozen()? So that no one can manipulate page refcount > randomly. The same applies to folio_set_count(). > > All set_page_count(..., 0) are converted to set_page_count_as_frozen(). > All set_page_count(..., 1) are converted to init_page_count(). > > We might need an init_folio_count() for folio_set_count(..., 1) users > and folio_set_count() has no other use. > > I see set_page_count(page, PAGE_FRAG_CACHE_MAX_SIZE + 1); from > mm/page_frag_cache.c and it could be converted to page_ref_unfreeze(), > since above it there is free_frozen_page(), which makes me think the > page is frozen. > Overall I agree, but this is out of scope for this patch main change. This could be handled as an independent patch later. >> static inline int page_ref_dec_return(struct page *page) >> { >> int ret = atomic_dec_return(&page->_refcount); >> + VM_BUG_ON(__page_count_is_frozen(ret + 1)); >> >> if (page_ref_tracepoint_active(page_ref_mod_and_return)) >> __page_ref_mod_and_return(page, -1, ret); > > VM_WARN_ON_ONCE() might be better? > With "locking via dedicated bit" patch modification, any function which drop reference counter to zero changes the behavior of refcount(page_add_unless would succeed and etc.). This could lead to hazardous results. Therefore, we decided to prohibit that completely and make zero value only possible as middle state during dec_and_test. > >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >> index 65e702fade61..27734cf795da 100644 >> --- a/mm/page_alloc.c >> +++ b/mm/page_alloc.c >> @@ -1639,14 +1639,14 @@ void __meminit __free_pages_core(struct page *page, unsigned int order, >> for (loop = 0; loop < nr_pages; loop++, p++) { >> VM_WARN_ON_ONCE(PageReserved(p)); >> __ClearPageOffline(p); >> - set_page_count(p, 0); >> + set_page_count_as_frozen(p); >> } >> >> adjust_managed_page_count(page, nr_pages); >> } else { >> for (loop = 0; loop < nr_pages; loop++, p++) { >> __ClearPageReserved(p); >> - set_page_count(p, 0); >> + set_page_count_as_frozen(p); >> } >> >> /* memblock adjusts totalram_pages() manually. */ >> -- > > Not sure about these two, they freeze p and p goes into buddy without > unfreeze to refcount==0. But at the beginning, you said there are two > states: > 1) Unfrozen page which right now has no explicit owner > 2) Frozen page > > 1) means free pages in buddy. But the above change brings frozen pages > into buddy, mixing the two states into one. Is that intended? Since state 1 only appears during dec_and_test, pages in the buddy allocator are considered frozen. So there is no mixing of states.