Re: [PATCH v2 1/1] s390/mm: Fix handling of _PAGE_UNUSED pte bit

[Gerald Schaefer]

On Mon, 15 Jun 2026 13:50:00 +0200
Heiko Carstens <hca@xxxxxxxxxxxxx> wrote:

> On Mon, Jun 15, 2026 at 12:31:03PM +0200, Claudio Imbrenda wrote:
> > On Mon, 15 Jun 2026 11:43:00 +0200
> > Heiko Carstens <hca@xxxxxxxxxxxxx> wrote:
> > 
> > [...]
> > 
> > > > @@ -175,6 +179,8 @@ pte_t ptep_modify_prot_start(struct
> > vm_area_struct *vma, unsigned long addr,
> > > >  void ptep_modify_prot_commit(struct vm_area_struct *vma, unsigned long addr,
> > > >  			     pte_t *ptep, pte_t old_pte, pte_t pte)
> > > >  {
> > > > +	if (pte_present(pte))
> > > > +		pte = clear_pte_bit(pte, __pgprot(_PAGE_UNUSED));
> > > >  	set_pte(ptep, pte);  
> > > 
> > > Can't we move the logic from set_ptes() to set_pte() instead? The above
> > 
> > set_pte() is also used for things that are not ptes, and in those cases
> > we probably don't want to touch that bit, although technically it is
> > currently unused for present large pmds and puds.
> 
> I can only see huge_pte_clear() for this.
> 
> If that's the only user I'd rather add a BUG_ON() there instead of starting to
> sprinkle the logic around. This _will_ break sooner or later.

There is also __set_huge_pte_at(), and there it could also be called for
swap PMDs/PUDs, where bit 56 is used. But they would not be present, and
ATM we use the same present bit for PTEs and PMDs/PUDs, so it should work.

Still feels a bit shaky, but in general I agree that it would be better
to have this logic in a single place, like set_pte().

Also wonder now why we use set_pte() an not set_pmd() / set_pud() for the
hugetlbfs "fake" PTEs. I think at least in s390 code we could switch to
the pmd/pud variants, and then be safe against modifications from set_pte().

BTW, wrt Sashiko report that just dropped in, I also wondered first about
ptep_reset_dat_prot() using set_pte(). Not 100% sure about the exact scenario
where/how the _PAGE_UNUSED bit gets mixed in, where it shouldn't. Maybe
the answer to that question might even show another fix option. But when
changing set_pte(), it should also be fine for ptep_reset_dat_prot(),
which requires that the PROTECT bit is the only HW bit getting changed,
and _PAGE_UNUSED is a SW bit.