Re: [PATCH 1/4] mm/damon: defensively skip zone device folios in damon_get_folio()

On Tue, Jul 28, 2026 at 07:57:04PM +0000, [email protected] wrote:
> --- Patch [1]: [PATCH 1/4] mm/damon: defensively skip zone device folios in damon_get_folio() ---
> commit 8181b2e77c1b49bcb808528f343f72fdbf9d168c
> Author: Gregory Price <[email protected]>
> 
> mm/damon: defensively skip zone device folios in damon_get_folio()
> 
> All DAMON physical- and virtual-address operations obtain their folios
> through damon_get_folio(). That helper already excludes ZONE_DEVICE
> memory implicitly via pfn_to_online_page() and folio_test_lru(), but
> this is inconsistent with other callers in mm/ which test explicitly.
> 
> > diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> > index 0981a5e3af172..0db1383de5d01 100644
> > --- a/mm/damon/ops-common.c
> > +++ b/mm/damon/ops-common.c
> > @@ -32,7 +32,8 @@ struct folio *damon_get_folio(unsigned long pfn)
> >  	folio = page_folio(page);
> >  	if (!folio_try_get(folio))
> >  		return NULL;
> > -	if (unlikely(page_folio(page) != folio) || !folio_test_lru(folio)) {
> > +	if (unlikely(page_folio(page) != folio) || !folio_test_lru(folio) ||
> > +	    folio_is_zone_device(folio)) {
> 
> [Severity: Medium]
> Since ZONE_DEVICE folios do not have the LRU flag set, won't the
> !folio_test_lru(folio) check always evaluate to true for them?
> 
> Additionally, earlier in damon_get_folio(), pfn_to_online_page(pfn) is
> called, which strictly returns NULL for ZONE_DEVICE memory. Doesn't this
> mean ZONE_DEVICE memory will never reach this new folio_is_zone_device()
> check at all, making it dead code?
> 

pfn_to_online_page() says this:

    /* The presence of a pgmap indicates ZONE_DEVICE offline pfn */
    if (pgmap)
        return NULL;

Which is the kind of implementation detail that could easily come around
to bite us. I'm ok dropping this patch if folks think it's too paranoid.

~Gregory