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 7B16D611E for ; Wed, 22 Apr 2026 00:12:58 +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=1776816778; cv=none; b=CgBTzpUcvUpJhyi0wc6jFvo36tL/FoYj1Y0WuhJdSm3ap2Rx6uE6tKQ5TAfdDct3V3nvyeg8ZPxVUY+By+5DA5r9vbKGs3uC+bbqZ4ZxzH6sRsLc96gtOSBmwHAH1KQkRD7TadHKVt1ZtfQ+n4dRW3vg9rN24yW7YOxhJ+OErew= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776816778; c=relaxed/simple; bh=G+o7PWS0hDV5gt4XP8uuA8DpSUV7cX3QjnvzK1fc9eA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=c+CvH51v7fecPyhWL6NeGSda1UlenRG6wkbkfwMDVJBSvAbXEhTxuzznvCCKsbA8t2A/IHzSxM/Qh493CVlnezgTTFAVY/xRJDasnDaNySjACANtZot2fAiiiYSan84K6lYE426XLW6s1mV5m79krqDtpJXbhzvYk2iM5FtveTc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=XbMcuJpX; 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="XbMcuJpX" Received: by smtp.kernel.org (Postfix) with ESMTPSA id D0C33C2BCB0; Wed, 22 Apr 2026 00:12:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1776816778; bh=G+o7PWS0hDV5gt4XP8uuA8DpSUV7cX3QjnvzK1fc9eA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=XbMcuJpX46z3bURXgChoSekGUf8tZwFqZu7PlxlXXBk+EBmlO2C91KVu+1RX3a5mo +/goL/S5o1X9GBboeF+FlijdHmirR+id1hs88XzGwfJsTKmXKxFOaAhwUIg3YmjsS3 Eq7+YpR1m/Ea5E+CWFFBb75hsmwxqrj7I3vNJ3rZnroUIgnp/JIJzbPVloLloXQD1F VMN3VngqvPy1anVZ0DRPMJONyKc0dLaU4mfqIFnhKMtep6eoNTgvsrttznZZjcYlkq 5Vgx9nl0dy5Rzrr3yeH1IzOgoz0hod+F6oRafKbjSxa1rysFwbCexQTBI/OgJAG6ic 4cBJc3Q6wGcyw== Date: Tue, 21 Apr 2026 17:12:55 -0700 From: Dennis Zhou To: Joonwon Kang Cc: akpm@linux-foundation.org, cl@gentwo.org, dodam@google.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, tj@kernel.org Subject: Re: [PATCH] percpu: Fix hint invariant breakage Message-ID: References: <20260420123548.2116177-1-joonwonkang@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260420123548.2116177-1-joonwonkang@google.com> On Mon, Apr 20, 2026 at 12:35:48PM +0000, Joonwon Kang wrote: > > Hello, > > > > Sorry for the delay, I've been a bit sick. > > > > On Mon, Mar 23, 2026 at 02:05:14PM +0000, Joonwon Kang wrote: > > > > Hello, > > > > > > > > On Fri, Mar 20, 2026 at 11:52:14AM +0000, Joonwon Kang wrote: > > > > > The invariant "scan_hint_start > contig_hint_start if and only if > > > > > scan_hint == contig_hint" should be kept for hint management. However, > > > > > it could be broken in some cases: > > > > > > > > > > > > > First I'd just like to apologize. I spent an hour yesterday trying to > > > > remember why the invariant exists and the reality is this code is more > > > > clever than it needs to be. > > > > > > Thanks for taking time for this and sharing more context. While you are at > > > it, I have a fundamental question on the invariant. I had deliberation and > > > discussion on what benefits the invariant gets to the percpu allocator by > > > its existence. My understanding is that if we put contig_hint before > > > scan_hint when they are the same, it is more likely that contig_hint is > > > broken by a future allocation, which leads to a linear scan after the > > > scan_hint for hints update, although we could save scanning upto scan_hint > > > when contig_hint is not broken. On the other hand, if we put scan_hint > > > before contig_hint instead, it is more likely that scan_hint is broken > > > while keeping contig_hint, which does not lead to the linear scan for > > > hints update, although we could not save the scanning that could be saved > > > in the other case. > > > > > > In other words, if contig_hint breaking allocations occur a lot in general > > > with the current invariant, the performance may more suffer than without > > > the invariant. I also think that there would be no strict reason of having > > > the invariant. > > > > > > > I think the original premise is that percpu memory is quite expensive, 1 > > allocation costs nr_cpus * sizeof(allocation). So we do our best to bin > > pack at the cost of faster allocations. We could always just break the > > contig_hint but then over time we could cause more fragmentation. > > > > The case that triggered this was netdev needing 8 byte objects with 16 > > byte alignment [1]. > > > > Thank you for sharing the points about the bin packing. Although I did not > fully understand the relationship between breakage of the contig_hint and the > fragmentation trend, it may be helpful to reference the case you referred to. > I guess you may have missed the link for the reference [1]? Could you help to > provide the link, if you intended to leave it? > Ah sorry that's my bad. My intention behind the scan_hint wasn't to use the scan_hint as an earlier contig_hint, but to prevent us from scanning if we need to break the contig_hint. [1] https://lore.kernel.org/netdev/CANn89iKb_vW+LA-91RV=zuAqbNycPFUYW54w_S=KZ3HdcWPw6Q@mail.gmail.com/ > > > So, could you clarify the necessity of the invariant? If there is no must > > > reason, then I could post another spin-off patch to remove the invariant > > > at all so that we could simplify the code and experiment the result. How > > > do you think? > > > > > > > I can't really recall the exact reasoning for the invariant, but it was > > probably along the lines of wanting to not lose information if possible. > > > > Say an earlier area becomes free that is the same size as the > > contig_hint but with better alignment, we ant to use that as the > > contig_hint but then we either have to lose the scan_hint or keep it > > with the invariant. Given the premise above, I believe we want to > > continue bin packing, I think the general idea of scanning next time > > around isn't the worst thing. > > > > Sadly because it's already there, and has worked for quite some time, > > it's kind of on us today to provide data / reasoning to delete it. I'd > > wager that some upcoming work is going to change how percpu gives out > > objects either through some sort of slab caching that we can revisit > > this more in that context. > > > > Understood and thanks for your detailed explanation. I will keep the invariant > as-is unless I have a clear data point to reverse it. I sent the new patch set > v3 recently with this in mind. Please help to review it ;) > Sorry for the delay. Provided a bit of feedback just now. Let me know if you want to discuss more about what we want v4 to look like before spending too much time. Thanks, Dennis