Re: [PATCH v3 13/13] mm: Remove device private pages from the physical address space

[Jordan Niethe]

Hi,

On 28/1/26 08:12, Zi Yan wrote:
> > diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
> > index 039a2d71e92f..e61a0e49a7c9 100644
> > --- a/mm/page_vma_mapped.c
> > +++ b/mm/page_vma_mapped.c
> > @@ -107,6 +107,7 @@ static bool map_pte(struct page_vma_mapped_walk *pvmw, pmd_t *pmdvalp,
> >   static bool check_pte(struct page_vma_mapped_walk *pvmw, unsigned long pte_nr)
> >   {
> >   	unsigned long pfn;
> > +	bool device_private = false;
> >   	pte_t ptent = ptep_get(pvmw->pte);
> >
> >   	if (pvmw->flags & PVMW_MIGRATION) {
> > @@ -115,6 +116,9 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw, unsigned long pte_nr)
> >   		if (!softleaf_is_migration(entry))
> >   			return false;
> >
> > +		if (softleaf_is_migration_device_private(entry))
> > +			device_private = true;
> > +
> >   		pfn = softleaf_to_pfn(entry);
> >   	} else if (pte_present(ptent)) {
> >   		pfn = pte_pfn(ptent);
> > @@ -127,8 +131,14 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw, unsigned long pte_nr)
> >   			return false;
> >
> >   		pfn = softleaf_to_pfn(entry);
> > +
> > +		if (softleaf_is_device_private(entry))
> > +			device_private = true;
> >   	}
> >
> > +	if ((device_private) ^ !!(pvmw->flags & PVMW_DEVICE_PRIVATE))
>
> Would “device_private != !!(pvmw->flags & PVMW_DEVICE_PRIVATE)” be more
> readable? Also I wonder if “!!” is needed here, since I remember modern
> C can convert “pvmw->flags & PVMW_DEVICE_PRIVATE” to bool.

Yeah, "!=" should work. I did need the "!!" - maybe because xor is bitwise?
If it's not needed with "!=" I"ll remove it.

> > +		return false;
> > +
> >   	if ((pfn + pte_nr - 1) < pvmw->pfn)
> >   		return false;
> >   	if (pfn > (pvmw->pfn + pvmw->nr_pages - 1))
> > @@ -137,8 +147,11 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw, unsigned long pte_nr)
> >   }
> >
> >   /* Returns true if the two ranges overlap.  Careful to not overflow. */
> > -static bool check_pmd(unsigned long pfn, struct page_vma_mapped_walk *pvmw)
> > +static bool check_pmd(unsigned long pfn, bool device_private, struct page_vma_mapped_walk *pvmw)
> >   {
> > +	if ((device_private) ^ !!(pvmw->flags & PVMW_DEVICE_PRIVATE))
>
> Ditto.

ack.

> Feel free to add:
>
> Reviewed-by: Zi Yan <ziy@xxxxxxxxxx> # for MM changes

Thanks for reviewing.

> Best Regards,
> Yan, Zi