linux-kernel - Re: [RFC PATCH 05/12] fs/proc/task_mmu: refactor pagemap_pmd

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <76348b1f-2626-4010-8269-edd74a936982@lucifer.local>
Date: Fri, 24 Oct 2025 19:19:11 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Gregory Price <gourry@...rry.net>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
        Christian Borntraeger <borntraeger@...ux.ibm.com>,
        Janosch Frank <frankja@...ux.ibm.com>,
        Claudio Imbrenda <imbrenda@...ux.ibm.com>,
        David Hildenbrand <david@...hat.com>,
        Alexander Gordeev <agordeev@...ux.ibm.com>,
        Gerald Schaefer <gerald.schaefer@...ux.ibm.com>,
        Heiko Carstens <hca@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>,
        Sven Schnelle <svens@...ux.ibm.com>, Zi Yan <ziy@...dia.com>,
        Baolin Wang <baolin.wang@...ux.alibaba.com>,
        "Liam R . Howlett" <Liam.Howlett@...cle.com>,
        Nico Pache <npache@...hat.com>, Ryan Roberts <ryan.roberts@....com>,
        Dev Jain <dev.jain@....com>, Barry Song <baohua@...nel.org>,
        Lance Yang <lance.yang@...ux.dev>,
        Kemeng Shi <shikemeng@...weicloud.com>,
        Kairui Song <kasong@...cent.com>, Nhat Pham <nphamcs@...il.com>,
        Baoquan He <bhe@...hat.com>, Chris Li <chrisl@...nel.org>,
        Peter Xu <peterx@...hat.com>, Matthew Wilcox <willy@...radead.org>,
        Jason Gunthorpe <jgg@...pe.ca>, Leon Romanovsky <leon@...nel.org>,
        Muchun Song <muchun.song@...ux.dev>,
        Oscar Salvador <osalvador@...e.de>, Vlastimil Babka <vbabka@...e.cz>,
        Mike Rapoport <rppt@...nel.org>,
        Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>,
        Jann Horn <jannh@...gle.com>, Matthew Brost <matthew.brost@...el.com>,
        Joshua Hahn <joshua.hahnjy@...il.com>, Rakie Kim <rakie.kim@...com>,
        Byungchul Park <byungchul@...com>,
        Ying Huang <ying.huang@...ux.alibaba.com>,
        Alistair Popple <apopple@...dia.com>, Pedro Falcato <pfalcato@...e.de>,
        Pasha Tatashin <pasha.tatashin@...een.com>,
        Rik van Riel <riel@...riel.com>, Harry Yoo <harry.yoo@...cle.com>,
        kvm@...r.kernel.org, linux-s390@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-mm@...ck.org
Subject: Re: [RFC PATCH 05/12] fs/proc/task_mmu: refactor pagemap_pmd_range()

On Fri, Oct 24, 2025 at 01:32:29PM -0400, Gregory Price wrote:
> On Fri, Oct 24, 2025 at 08:41:21AM +0100, Lorenzo Stoakes wrote:
> > Separate out THP logic so we can drop an indentation level and reduce the
> > amount of noise in this function.
> >
> > We add pagemap_pmd_range_thp() for this purpose.
> >
> > While we're here, convert the VM_BUG_ON() to a VM_WARN_ON_ONCE() at the
> > same time.
> >
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
> ... >8
> > +static int pagemap_pmd_range(pmd_t *pmdp, unsigned long addr, unsigned long end,
> > +			     struct mm_walk *walk)
> > +{
> > +	struct vm_area_struct *vma = walk->vma;
> > +	struct pagemapread *pm = walk->private;
> > +	spinlock_t *ptl;
> > +	pte_t *pte, *orig_pte;
> > +	int err = 0;
> > +
> > +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> > +	ptl = pmd_trans_huge_lock(pmdp, vma);
> > +	if (ptl)
> > +		return pagemap_pmd_range_thp(pmdp, addr, end, vma, pm, ptl);
> > +#endif
>
> Maybe something like...
>
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> ptl = pmd_trans_huge_lock(pmdp, vma);
> if (ptl) {
> 	err = pagemap_pmd_range_thp(pmdp, addr, end, vma, pm, ptl);
> 	spin_unlock(ptl);
> 	return err;
> }
> #endif
>
> and drop the spin_unlock(ptl) calls from pagemap_pmd_range_thp?

Ah yeah that's a good idea, will do that on respin!

>
> Makes it easier to understand the locking semantics.

Absolutely.

>
> Might be worth adding a comment to pagemap_pmd_range_thp that callers
> must hold the ptl lock.

Ack will do!

>
> ~Gregory
>
> P.S. This patch set made my day, the whole non-swap-swap thing has
> always broken my brain.  <3

Thanks :) yeah this came out of my advocating _for_ is_swap_pte() on a series,
because hey - if you're going to operate on PTEs based on them being 'xxx', and
you have a predicate 'is_xxx()' it follows that you should only do those
operations if you have applied the predicate right?

But of course we largely don't do that, so we end up with horrible confusion
between a convention that not-none, not-present = swap entry + this predicate.

PLUS on top of that, we have the 'non-swap swap entry' so we have not-none,
not-present = swap, or swap that is not swap but also swap but hey definitely
isn't and and... :)

A next step will be to at least rename swp_entry_t to something else, because
every last remnant of this 'swap entries but not really' needs to be dealt
with...

Anyway this goes a long way to getting there :)

Cheers, Lorenzo