Re: [PATCH 04/14] iommu/pages: Add APIs to preserve/unpreserve/restore iommu pages

[Samiullah Khawaja]

On Fri, Mar 20, 2026 at 11:01:25AM +0000, Pranjal Shrivastava wrote:
> On Tue, Mar 17, 2026 at 01:59:03PM -0700, Vipin Sharma wrote:
> > On Tue, Feb 03, 2026 at 10:09:38PM +0000, Samiullah Khawaja wrote:
> > > +
>
> [------- snip >8 --------]
>
> > > +EXPORT_SYMBOL_GPL(iommu_unpreserve_pages);
> > > +
> > > +void iommu_restore_page(u64 phys)
> > > +{
> > > +	struct ioptdesc *iopt;
> > > +	struct folio *folio;
> > > +	unsigned long pgcnt;
> > > +	unsigned int order;
> > > +
> > > +	folio = kho_restore_folio(phys);
> > > +	BUG_ON(!folio);
> > > +
> > > +	iopt = folio_ioptdesc(folio);
> > > +
> > > +	order = folio_order(folio);
> > > +	pgcnt = 1UL << order;
> > > +	mod_node_page_state(folio_pgdat(folio), NR_IOMMU_PAGES, pgcnt);
> > > +	lruvec_stat_mod_folio(folio, NR_SECONDARY_PAGETABLE, pgcnt);
> >
> > Above two functions are also used in other two places in this file, lets
> > create a common function for these two operations.
>
> I was thinking about this, out of the 2 other places one is
> `__iommu_free_desc` where we are supposed to subtract pgcnt.. but I
> think we can follow the -pgcnt pattern we have currently and add a
> helper `iommu_folio_update_stats(folio, pgcnt)` where we can pass -pgcnt
> to subtract nr_pages? Something like:
>
> +static inline void iommu_folio_update_stats(struct folio *folio, long nr_pages)
> +{
> +	mod_node_page_state(folio_pgdat(folio), NR_IOMMU_PAGES, nr_pages);
> +	lruvec_stat_mod_folio(folio, NR_SECONDARY_PAGETABLE, nr_pages);
> +}
>
> @@ -86,8 +91,7 @@ static void __iommu_free_desc(struct ioptdesc *iopt)
> 	if (IOMMU_PAGES_USE_DMA_API)
> 		WARN_ON_ONCE(iopt->incoherent);
>
> -	mod_node_page_state(folio_pgdat(folio), NR_IOMMU_PAGES, -pgcnt);
> -	lruvec_stat_mod_folio(folio, NR_SECONDARY_PAGETABLE, -pgcnt);
> +	iommu_folio_update_stats(folio, -(long)pgcnt);
> 	folio_put(folio);
> }
>
> But I'm unsure if passing -(long)pgcnt is a good idea?

mod_node_page_state() takes last argument as "long" and I can see other
examples of passing -folio_nr_pages(folio) directly into it. So we can
just keep it as following:

iommu_folio_update_stats(folio, -pgcnt);

I think it is a good cleanup. I will update this in the next revision.
> > > +}
> > > +EXPORT_SYMBOL_GPL(iommu_restore_page);
> > > +
>
> Thanks,
> Praan