From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 14C032472A8 for ; Fri, 21 Nov 2025 17:09:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763744963; cv=none; b=geTK47iFZfu9UjVvAZcBKwfb50PMH9SQ1DEmTyFyKoqk5yW1hi/wWTvVGBvGFEVoaXVnD5GXd1yplZ9vfH3796bxy3RKHKZ2W7pTpYIt4ZcE3pfpMDUSRXEqivdRdBMmtMUXUQPQ8T6wmGfnB2cM/u5SLarg2iB94t19tUP6VGs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763744963; c=relaxed/simple; bh=wbtbUmI+JYBD2T+C4kTcqxXZdRf5waIyL5iLdAIi17I=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=Udsn4vK0xMCZB8Q2TVEstZsGoRFQI/1v/xah4DnFyKYVv9KWatEOAsweoHWeynZvYtUjj5Dd0+8fQRXdUY064JKEagR5OgVMdUJ8dG0s1h4N8qu/FMeLsWLp6e21gVSlNHUxEdnt8wz2GWftTqlgHut8c6oSgrZdU+9Pka0vh6o= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=ODepD5DC; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="ODepD5DC" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8246BC4CEF1; Fri, 21 Nov 2025 17:09:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1763744962; bh=wbtbUmI+JYBD2T+C4kTcqxXZdRf5waIyL5iLdAIi17I=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=ODepD5DCIv5oRt2HVCh5GwfvCEkSdBsa9KK7pBqILQdKuNbSqMeSJtIi/PaTKYmVe KJ45zZjgb/1Dkmu/Xu3M+lQ5kjQpOGKMX7opaOYWO4HABkS11L2YMUS5sxtE4Hx2R1 q1MVVEqCmnHhyOCHzhkTpB9OSdPvu9jEb62+5sLEu5lO7yy4K1nuRg5KfWqL+Mpa2p WO8uifpeuNXi5CJMvIB2upXpp1NPov2CwTCmyvBjnu1qxLyGtmHwBud8qDK4snUE9t dZreWNmiDJzdonAjkfsywN9wmVbBsav6MlxrmLqAvVbDm17sglU0PNsu0bGy6NdR8C ZSHfquPmOps3g== Message-ID: Date: Fri, 21 Nov 2025 18:09:16 +0100 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 1/3] mm/huge_memory: prevent NULL pointer dereference in try_folio_split_to_order() To: Zi Yan Cc: Lorenzo Stoakes , Andrew Morton , Baolin Wang , "Liam R. Howlett" , Nico Pache , Ryan Roberts , Dev Jain , Barry Song , Lance Yang , Miaohe Lin , Naoya Horiguchi , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20251120035953.1115736-1-ziy@nvidia.com> <20251120035953.1115736-2-ziy@nvidia.com> <875584d7-5a68-4f7a-8549-2a9cd6c7f9d8@kernel.org> <73075A18-3C07-46F5-B8C8-9018D2CD22DE@nvidia.com> From: "David Hildenbrand (Red Hat)" Content-Language: en-US In-Reply-To: <73075A18-3C07-46F5-B8C8-9018D2CD22DE@nvidia.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit >> >> BTW, I wonder if the is_huge_zero_folio() check should go into folio_split_supported() and just return in -EINVAL. (we shouldn't really trigger that). Similarly we could add a hugetlb sanity check. > > Yeah, is_huge_zero_folio() should return -EINVAL not -EBUSY, except > the case the split happens before a process writes 0 to a zero large folio > and gets a new writable large folio, in which we can kinda say it looks like > -EBUSY. But it is still a stretch. I see what you mean, but I think this has less to do with actual races. SO yeah, -EINVAL is likely the tight thing. > > Ack on adding hugetlb sanity check. > > OK, just to reiterate my above idea on renaming folio_split_supported(). > Are you OK with renaming it to folio_split_check(), so that returning -EBUSY > and -EINVAL looks more reasonable? The benefit is that we no longer need > to worry about we need to always do folio->mapping check before > folio_split_supported(). (In addition, I would rename can_split_folio() > to folio_split_refcount_check() for clarification) I guess having some function that tells you "I performed all checks I could without taking locks/references (like anon_vma) and starting with the real magic" is what you have in mind. For these we don't have to prefix with "folio_split" if it sounds weird. folio_check_splittable() ? Regarding can_split_folio(), I was wondering whether we can just get rid of it and use folio_expect_ref_count() instead? For the two callers that need extra_pins, we could just have something simple helper in huge_memory.c like /* Number of folio references from the pagecache or the swapcache. */ unsigned int folio_cache_references(const struct folio *folio) { if (folio_test_anon(folio) && !folio_test_swapcache(folio)) return 0; return folio_nr_pages(folio); } -- Cheers David