From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id F2203C4332F for ; Thu, 10 Nov 2022 16:32:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231863AbiKJQcS (ORCPT ); Thu, 10 Nov 2022 11:32:18 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60088 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231670AbiKJQcJ (ORCPT ); Thu, 10 Nov 2022 11:32:09 -0500 Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:8b0:10b:1236::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D83163FBA3 for ; Thu, 10 Nov 2022 08:32:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=NPEF063H+Y7JeJNfnAB1kp+fd1pLsT9ML+6VjdUPh7o=; b=mx+EY9COzoDzeuy2+0heRU2yNe yzX/vmiC19SUiPXxXDe3Mld28kvj/feSddm0YDw9iYaoR6cFptf4oO6yG82v/riN6Wpid4/0SRwQl zoOCHLqUuayVhzSFL6UbBwFSp6aZgG6Tb1dHoy0X72Ced80huuiy8/kMhkeaeH5LT47Jc4CoDZB1T mA+DweHo/jN0Rlhgnyde/Z0ueQJn3iMdcvqFEG2W66SIEdiQxy2oinLi/WkRa3UmvD8cFjFfFsRPH tp+x9oFo59yFei677vlB7qS8S/c2iCw3nDiN9eDPQftxhpdbchYaCJjS8hwCKiFm3t6iqIpCkjLS1 W2XpdHBQ==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1otASt-00CD9f-Ff; Thu, 10 Nov 2022 16:31:43 +0000 Date: Thu, 10 Nov 2022 16:31:43 +0000 From: Matthew Wilcox To: Linus Torvalds Cc: Hugh Dickins , Andrew Morton , Johannes Weiner , "Kirill A. Shutemov" , David Hildenbrand , Vlastimil Babka , Peter Xu , Yang Shi , John Hubbard , Mike Kravetz , Sidhartha Kumar , Muchun Song , Miaohe Lin , Naoya Horiguchi , Mina Almasry , James Houghton , Zach O'Keefe , linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH 4/3] mm,thp,rmap: handle the normal !PageCompound case first Message-ID: References: <5f52de70-975-e94f-f141-543765736181@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Nov 09, 2022 at 07:23:08PM -0800, Linus Torvalds wrote: > On Wed, Nov 9, 2022 at 6:18 PM Hugh Dickins wrote: > > > > Commit ("mm,thp,rmap: lock_compound_mapcounts() on THP mapcounts") > > propagated the "if (compound) {lock} else if (PageCompound) {lock} else > > {atomic}" pattern throughout; but Linus hated the way that gives primacy > > to the uncommon case: switch to "if (!PageCompound) {atomic} else if > > (compound) {lock} else {lock}" throughout. > > Side note, that 'compound' naming is also on my list of "I'm _really_ > not a fan". > > We actually have a completely different meaning for PageCompound() > than the meaning of 'compound' in the rmap functions, and those > functions literally mix those meanings if not on the same line, then > at least right next to each other. > > What 'rmap' actually means with 'compound' in the add/remove functions > is basically 'not PAGE_SIZE' as far as I can tell. Ah. I've been trying to understand what that 'compound' really means, and what the difference is to 'PageCompound()' and why we need both. Thanks! > One reason I find the "compound" name so horrifying is that it is used > very much for HUGETLB pages, which I don't think end up ever being > marked as PageCompund(), and which are - for various historical > reasons - doubly confusing because they use a "pte_t" to describe > themselves, even when they are actually using a "pmd_t" or a "pud_t" > to actually map the page. HugeTLB pages _are_ marked as Compound. There's some fairly horrific code to manually make them compound when they have to be allocated piecemeal (because they're 1GB and too large for the page allocator). > To make things more confusing, some places use PageHeadHuge() > instead (but the folio version of said test is called > "folio_test_hugetlb()", just so that nobody could possibly ever accuse > the HUGETLB code to have consistency). That one's my fault, but it's a reaction to all the times that I and others have got confused between PageHuge and PageTransHuge. I suppose we could do a big sed s/PageHuge/PageHugeTLB/, but I'm hopeful the entire hugetlb codebase is either converted to folios or unified with THP handling. > I do wish the HUGETLB case didn't use 'pte' for its notion of how > HUGETLB entries are mapped, but that's literally how HUGETLB is > designed: it started life as a larger last-level pte. > > It just means that it ends up being very confusing when from a page > table walk perspective, you're walking a pud or a pmd entry, and then > you see a 'pte_t' instead. Yes, one of the long-term things I want to try is making the hugetlb code use the pmd/pud types like the THP code does.