On Thu, 14 Dec 2023, Pasha Tatashin wrote: > On Thu, Dec 14, 2023 at 12:58 PM David Rientjes <rientjes@xxxxxxxxxx> wrote: > > > > On Thu, 30 Nov 2023, Pasha Tatashin wrote: > > > > > diff --git a/drivers/iommu/iommu-pages.h b/drivers/iommu/iommu-pages.h > > > new file mode 100644 > > > index 000000000000..2332f807d514 > > > --- /dev/null > > > +++ b/drivers/iommu/iommu-pages.h > > > @@ -0,0 +1,199 @@ > > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > > +/* > > > + * Copyright (c) 2023, Google LLC. > > > + * Pasha Tatashin <pasha.tatashin@xxxxxxxxxx> > > > + */ > > > + > > > +#ifndef __IOMMU_PAGES_H > > > +#define __IOMMU_PAGES_H > > > + > > > +#include <linux/vmstat.h> > > > +#include <linux/gfp.h> > > > +#include <linux/mm.h> > > > + > > > +/* > > > + * All page allocation that are performed in the IOMMU subsystem must use one of > > > + * the functions below. This is necessary for the proper accounting as IOMMU > > > + * state can be rather large, i.e. multiple gigabytes in size. > > > + */ > > > + > > > +/** > > > + * __iommu_alloc_pages_node - allocate a zeroed page of a given order from > > > + * specific NUMA node. > > > + * @nid: memory NUMA node id > > > > NUMA_NO_NODE if no locality requirements? > > If no locality is required, there is a better interface: > __iommu_alloc_pages(). That one will also take a look at the calling > process policies to determine the proper NUMA node when nothing is > specified. However, when policies should be ignored, and no locality > required, NUMA_NO_NODE can be passed. > Gotcha, thanks! > > > > > + * @gfp: buddy allocator flags > > > + * @order: page order > > > + * > > > + * returns the head struct page of the allocated page. > > > + */ > > > +static inline struct page *__iommu_alloc_pages_node(int nid, gfp_t gfp, > > > + int order) > > > +{ > > > + struct page *pages; > > > > s/pages/page/ here and later in this file. > > In this file, where there a page with an "order", I reference it with > "pages", when no order (i.e. order = 0), I reference it with "page" > > I.e.: __iommu_alloc_page vs. __iommu_alloc_pages > Eh, the struct page points to a (potentially compound) page, not a set or list of pages. I won't bikeshed on it, but "struct page *pages" never makes sense unless it's **pages or *pages[] :)