On 2/19/25 04:19, Jason Gunthorpe wrote:

On Sat, Feb 15, 2025 at 04:48:04PM +0800, Baolu Lu wrote:

quoted

quoted

+#include <linux/gfp.h>
+#include <linux/mm.h>
+
+/**
+ * iommu_alloc_pages_node - Allocate a zeroed page of a given order from
+ *                          specific NUMA node
+ * @nid: memory NUMA node id
+ * @gfp: buddy allocator flags
+ * @order: page order
+ *
+ * Returns the virtual address of the allocated page. The page must be
+ * freed either by calling iommu_free_page() or via iommu_put_pages_list().

nit: ... by calling iommu_free_pages() ...

Got it

quoted

and

  s/page/pages/g in above comments?

There is alot of historical confusion here because it was all designed
around alloc_pages() which allocated a list of contiguous pages that
could be subdivided. When this moved to GFP_COMP and later to
folio_alloc() the subdivision is no longer possible. So it is not
"pages" at all anymore, but a single "[compound] page".

So the module name is called "iommu-pages" but aside from the free
list functions everything else acts on a single [compound] page only.

If you think about it too much it makes no sense but I didn't want to
rename every function. I tried to keep it so that "iommu pages" was
part of othe module name, and function designators, but the comments
talk about a singular [compound] page

quoted

quoted

+static void __iommu_free_page(struct page *page)

It's more readable if renaming it to __iommu_free_pages()?

Ah.. Well, it captures the module name but nothing it does acts on
multiple things, since it is internal I used the other name

How about I rename it later on to:

static void __iommu_free_desc(struct ioptdesc *iopt)

?

No problem. It actually depends on you. Since this is only used locally,
the iommu drivers won't use it. So, anything that helps understand what
it does is okay. :-)

Thanks,
baolu