From: Anshuman Khandual <email@example.com> To: Will Deacon <firstname.lastname@example.org> Cc: "Steven Price" <email@example.com>, firstname.lastname@example.org, "Mark Rutland" <Mark.Rutland@arm.com>, email@example.com, "Arnd Bergmann" <firstname.lastname@example.org>, "Ard Biesheuvel" <email@example.com>, "Peter Zijlstra" <firstname.lastname@example.org>, "Catalin Marinas" <email@example.com>, "Dave Hansen" <firstname.lastname@example.org>, email@example.com, "Jérôme Glisse" <firstname.lastname@example.org>, "Ingo Molnar" <email@example.com>, "Borislav Petkov" <firstname.lastname@example.org>, "Andy Lutomirski" <email@example.com>, "H. Peter Anvin" <firstname.lastname@example.org>, "James Morse" <email@example.com>, "Thomas Gleixner" <firstname.lastname@example.org>, "Andrew Morton" <email@example.com>, firstname.lastname@example.org, "Liang, Kan" <email@example.com> Subject: Re: [PATCH v9 00/21] Generic page walk and ptdump Date: Fri, 26 Jul 2019 11:33:14 +0530 Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <20190725093036.dzn6uulcihhkohm2@willie-the-truck> On 07/25/2019 03:00 PM, Will Deacon wrote: > On Thu, Jul 25, 2019 at 02:39:22PM +0530, Anshuman Khandual wrote: >> On 07/24/2019 07:05 PM, Steven Price wrote: >>> There isn't any problem as such with using p?d_large macros. However the >>> name "large" has caused confusion in the past. In particular there are >>> two types of "large" page: >>> >>> 1. leaf entries at high levels than normal ('sections' on Arm, for 4K >>> pages this gives you 2MB and 1GB pages). >>> >>> 2. sets of contiguous entries that can share a TLB entry (the >>> 'Contiguous bit' on Arm - which for 4K pages gives you 16 entries = 64 >>> KB 'pages'). >> >> This is arm64 specific and AFAIK there are no other architectures where there >> will be any confusion wrt p?d_large() not meaning a single entry. >> >> As you have noted before if we are printing individual entries with PTE_CONT >> then they need not be identified as p??d_large(). In which case p?d_large() >> can just safely point to p?d_sect() identifying regular huge leaf entries. > > Steven's stuck in the middle of things here, but I do object to p?d_large() > because I find it bonkers to have p?d_large() and p?d_huge() mean completely > different things when they are synonyms in the English language. Agreed that both p?d_large() and p?d_huge() should not exist at the same time when they imply the same thing. I believe all these name proliferation happened because THP, HugeTLB and kernel large mappings like linear, vmemmap, ioremap etc which the platform code had to deal with in various forms. > > Yes, p?d_leaf() matches the terminology used by the Arm architecture, but > given that most page table structures are arranged as a 'tree', then it's > not completely unreasonable, in my opinion. If you have a more descriptive > name, we could use that instead. We could also paint it blue. The alternate name chosen p?d_leaf() is absolutely fine and it describes the entry as intended. There is no disagreement over that. My original concern was introduction of yet another page table helper. > >>> In many cases both give the same effect (reduce pressure on TLBs and >>> requires contiguous and aligned physical addresses). But for this case >>> we only care about the 'leaf' case (because the contiguous bit makes no >>> difference to walking the page tables). >> >> Right and we can just safely identify section entries with it. What will be >> the problem with that ? Again this is only arm64 specific. >> >>> >>> As far as I'm aware p?d_large() currently implements the first and >>> p?d_(trans_)huge() implements either 1 or 2 depending on the architecture. >> >> AFAIK option 2 exists only on arm6 platform. IIUC generic MM requires two >> different huge page dentition from platform. HugeTLB identifies large entries >> at PGD|PUD|PMD after converting it's content into PTE first. So there is no >> need for direct large page definitions for other levels. >> >> 1. THP - pmd_trans_huge() >> 2. HugeTLB - pte_huge() CONFIG_ARCH_WANT_GENERAL_HUGETLB is set >> >> A simple check for p?d_large() on mm/ and include/linux shows that there are >> no existing usage for these in generic MM. Hence it is available. > > Alternatively, it means we have a good opportunity to give it a better name > before it spreads into the core code. Fair enough, that is another way. So you expect existing platform definitions for p?d_large()/p?d_huge() getting cleaned up and to start using new p?d_leaf() instead ? > >> IMHO the new addition of p?d_leaf() can be avoided and p?d_large() should be >> cleaned up (if required) in platforms and used in generic functions. > > Again, I disagree and think p?d_large() should be confined to arch code > if it sticks around at all. All of those instances should migrate to using p?d_leaf() eventually else there will be three different helpers which probably mean the same thing.
next prev parent reply index Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-22 15:41 Steven Price 2019-07-22 15:41 ` [PATCH v9 01/21] arc: mm: Add p?d_leaf() definitions Steven Price 2019-07-22 15:41 ` [PATCH v9 02/21] arm: " Steven Price 2019-07-22 15:41 ` [PATCH v9 03/21] arm64: " Steven Price 2019-07-22 15:41 ` [PATCH v9 04/21] mips: " Steven Price 2019-07-22 21:47 ` Paul Burton 2019-07-24 13:03 ` Steven Price 2019-07-22 15:41 ` [PATCH v9 05/21] powerpc: " Steven Price 2019-07-22 15:41 ` [PATCH v9 06/21] riscv: " Steven Price 2019-07-22 15:41 ` [PATCH v9 07/21] s390: " Steven Price 2019-07-22 15:41 ` [PATCH v9 08/21] sparc: " Steven Price 2019-07-22 15:41 ` [PATCH v9 09/21] x86: " Steven Price 2019-07-22 15:41 ` [PATCH v9 10/21] mm: Add generic p?d_leaf() macros Steven Price 2019-07-23 9:41 ` Mark Rutland 2019-07-24 13:48 ` Steven Price 2019-07-28 11:44 ` Anshuman Khandual 2019-07-29 11:38 ` Steven Price 2019-08-01 6:09 ` Anshuman Khandual 2019-08-01 12:22 ` Steven Price 2019-07-29 12:50 ` Mark Rutland 2019-08-01 6:13 ` Anshuman Khandual 2019-07-22 15:42 ` [PATCH v9 11/21] mm: pagewalk: Add p4d_entry() and pgd_entry() Steven Price 2019-07-23 10:14 ` Mark Rutland 2019-07-24 13:53 ` Steven Price 2019-07-24 14:09 ` Mark Rutland 2019-07-28 12:33 ` Anshuman Khandual 2019-07-29 12:17 ` Steven Price 2019-07-22 15:42 ` [PATCH v9 12/21] mm: pagewalk: Allow walking without vma Steven Price 2019-07-28 14:20 ` Anshuman Khandual 2019-07-29 12:29 ` Steven Price 2019-08-01 6:41 ` Anshuman Khandual 2019-07-22 15:42 ` [PATCH v9 13/21] mm: pagewalk: Add test_p?d callbacks Steven Price 2019-07-28 13:41 ` Anshuman Khandual 2019-07-29 12:34 ` Steven Price 2019-07-22 15:42 ` [PATCH v9 14/21] x86: mm: Don't display pages which aren't present in debugfs Steven Price 2019-07-22 15:42 ` [PATCH v9 15/21] x86: mm: Point to struct seq_file from struct pg_state Steven Price 2019-07-22 15:42 ` [PATCH v9 16/21] x86: mm+efi: Convert ptdump_walk_pgd_level() to take a mm_struct Steven Price 2019-07-22 15:42 ` [PATCH v9 17/21] x86: mm: Convert ptdump_walk_pgd_level_debugfs() to take an mm_struct Steven Price 2019-07-22 15:42 ` [PATCH v9 18/21] x86: mm: Convert ptdump_walk_pgd_level_core() " Steven Price 2019-07-22 15:42 ` [PATCH v9 19/21] mm: Add generic ptdump Steven Price 2019-07-23 9:57 ` Mark Rutland 2019-07-24 16:36 ` Steven Price 2019-07-29 2:59 ` Anshuman Khandual 2019-07-29 13:56 ` Steven Price 2019-07-22 15:42 ` [PATCH v9 20/21] x86: mm: Convert dump_pagetables to use walk_page_range Steven Price 2019-07-22 15:42 ` [PATCH v9 21/21] arm64: mm: Convert mm/dump.c to use walk_page_range() Steven Price 2019-07-23 6:39 ` [PATCH v9 00/21] Generic page walk and ptdump Anshuman Khandual 2019-07-24 13:35 ` Steven Price 2019-07-25 9:09 ` Anshuman Khandual 2019-07-25 9:30 ` Will Deacon 2019-07-26 6:03 ` Anshuman Khandual [this message] 2019-07-25 10:15 ` Steven Price 2019-07-23 10:16 ` Mark Rutland 2019-07-24 13:35 ` Steven Price 2019-07-24 13:57 ` Thomas Gleixner 2019-07-24 14:07 ` Mark Rutland 2019-07-24 14:18 ` Steven Price 2019-07-24 14:37 ` Thomas Gleixner 2019-07-28 11:20 ` Anshuman Khandual 2019-07-29 11:32 ` Steven Price 2019-07-31 9:27 ` Sven Schnelle 2019-07-31 11:18 ` Steven Price
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --email@example.com \ --firstname.lastname@example.org \ --cc=Mark.Rutland@arm.com \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
LKML Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git git clone --mirror https://lore.kernel.org/lkml/10 lkml/git/10.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \ email@example.com public-inbox-index lkml Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel AGPL code for this site: git clone https://public-inbox.org/public-inbox.git