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 E5B2D1D63E4 for ; Fri, 19 Dec 2025 19:09:02 +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=1766171346; cv=none; b=fFcFiXn8Q/TYSeIpQFKKYKhTiFQZWNe3hxqN+o55Dn56JGOU6NT+Zf7kLVlXfm9dyVK5UaDEqszPYmJxzJaffoJay04ERxxbQ5FePMuDkCKSdDJvCpBINvZMtBN+33oYbV0KTiTYzbM2Q2Oz8zg6e8S+sLhm8eNo8LHM7U0qcv8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766171346; c=relaxed/simple; bh=IX/O7hlhdhDUmzJHKEqQqCpvDjnz4b6Vp+x1m0oFyz8=; h=Message-ID:Date:MIME-Version:Subject:To:CC:References:From: In-Reply-To:Content-Type; b=lKY9cS0UwjW7YkXgr7cwtW+4zNg46HrdIbeG9Z/sm4O5YRktguKSD0+DKGGzqH1jXkI6CL5t1mni9NIPTU5XzG6QsNcxG3JjVjfK+OjGLga2rvZxPq1WGccZBPmT4e4W9/yLYFt9YOB27khzjhttjOFJ0x1O0jyaquKHGr5NhXo= 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.83]) by frasgout.his.huawei.com (SkyGuard) with ESMTPS id 4dXxrt3QF2zHnGf4; Sat, 20 Dec 2025 03:08:30 +0800 (CST) Received: from mscpeml500003.china.huawei.com (unknown [7.188.49.51]) by mail.maildlp.com (Postfix) with ESMTPS id 1F0EA40569; Sat, 20 Dec 2025 03:09:00 +0800 (CST) Received: from [10.123.123.67] (10.123.123.67) 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; Fri, 19 Dec 2025 22:08:56 +0300 Message-ID: Date: Fri, 19 Dec 2025 22:08:54 +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: [RFC PATCH 2/2] mm: implement page refcount locking via dedicated bit To: Kiryl Shutsemau CC: , , , , , , , , , , , , , , , , , , , , , , , , References: <81e3c45f49bdac231e831ec7ba09ef42fbb77930.1766145604.git.gladyshev.ilya1@h-partners.com> <9822c658-c2f0-4b1c-9eef-9ffa865e44f7@h-partners.com> Content-Language: en-US From: Gladyshev Ilya In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: lhrpeml500012.china.huawei.com (7.191.174.4) To mscpeml500003.china.huawei.com (7.188.49.51) On 12/19/2025 8:46 PM, Kiryl Shutsemau wrote: > On Fri, Dec 19, 2025 at 07:18:53PM +0300, Gladyshev Ilya wrote: >> On 12/19/2025 5:50 PM, Kiryl Shutsemau wrote: >>> On Fri, Dec 19, 2025 at 12:46:39PM +0000, Gladyshev Ilya wrote: >>>> The current atomic-based page refcount implementation treats zero >>>> counter as dead and requires a compare-and-swap loop in folio_try_get() >>>> to prevent incrementing a dead refcount. This CAS loop acts as a >>>> serialization point and can become a significant bottleneck during >>>> high-frequency file read operations. >>>> >>>> This patch introduces FOLIO_LOCKED_BIT to distinguish between a >>> >>> s/FOLIO_LOCKED_BIT/PAGEREF_LOCKED_BIT/ >> Ack, thanks >> >>>> (temporary) zero refcount and a locked (dead/frozen) state. Because now >>>> incrementing counter doesn't affect it's locked/unlocked state, it is >>>> possible to use an optimistic atomic_fetch_add() in >>>> page_ref_add_unless_zero() that operates independently of the locked bit. >>>> The locked state is handled after the increment attempt, eliminating the >>>> need for the CAS loop. >>> >>> I don't think I follow. >>> >>> Your trick with the PAGEREF_LOCKED_BIT helps with serialization against >>> page_ref_freeze(), but I don't think it does anything to serialize >>> against freeing the page under you. >>> >>> Like, if the page in the process of freeing, page allocator sets its >>> refcount to zero and your version of page_ref_add_unless_zero() >>> successfully acquirees reference for the freed page. >>> >>> How is it safe? >> >> Page is freed only after a successful page_ref_dec_and_test() call, which >> will set LOCKED_BIT. This bit will persist until set_page_count(1) is called >> somewhere in the allocation path [alloc_pages()], and effectively block any >> "use after free" users. > > Okay, fair enough. > > But what prevent the following scenario? > > CPU0 CPU1 > page_ref_dec_and_test() > atomic_dec_and_test() // refcount=0 > page_ref_add_unless_zero() > atomic_add_return() // refcount=1, no LOCKED_BIT > page_ref_dec_and_test() > atomic_dec_and_test() // refcount=0 > atomic_cmpxchg(0, LOCKED_BIT) // succeeds > atomic_cmpxchg(0, LOCKED_BIT) // fails > // return false to caller > // Use-after-free: BOOM! > But you can't trust that the page is safe to use after page_ref_dec_and_test() returns false, if I understood your example correctly. For example, current implementation can also lead to this 'bug' if you slightly change the order of atomic ops in your example: Initial refcount value: 1 from CPU 0 CPU 0 CPU 1 page_ref_and_dec() page_ref_add_unless_zero() atomic_add_return() [1 -> 2] atomic_dec_and_test() [2 -> 1] page_ref_dec_and_test() atomic_dec_and_test() [1 -> 0] /* page is logically freed here */ return false [cause 1!=0] // Caller with use after free?