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

[Samiullah Khawaja <skhawaja@google.com>]

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