| Message ID | 9-v3-e797f4dc6918+93057-iommu_pages_jgg@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | iommu: Further abstract iommu-pages | expand |
On 2/26/25 03:39, Jason Gunthorpe wrote: > diff --git a/include/linux/iommu.h b/include/linux/iommu.h > index 38c65e92ecd091..e414951c0af83f 100644 > --- a/include/linux/iommu.h > +++ b/include/linux/iommu.h > @@ -326,6 +326,18 @@ typedef unsigned int ioasid_t; > /* Read but do not clear any dirty bits */ > #define IOMMU_DIRTY_NO_CLEAR (1 << 0) > > +/* > + * Pages allocated through iommu_alloc_pages_node() can be placed on this list > + * using iommu_pages_list_add(). Note: ONLY pages from iommu_alloc_pages_node() > + * can be used this way! > + */ > +struct iommu_pages_list { > + struct list_head pages; > +}; > + > +#define IOMMU_PAGES_LIST_INIT(name) \ > + ((struct iommu_pages_list){ .pages = LIST_HEAD_INIT(name.pages) }) > + > #ifdef CONFIG_IOMMU_API Any reason why the above cannot be placed in the iommu-pages.h header file? My understanding is that iommu-pages is only for the iommu drivers and should not be accessible for external subsystems. Thanks, baolu
On Wed, Feb 26, 2025 at 02:56:29PM +0800, Baolu Lu wrote: > On 2/26/25 03:39, Jason Gunthorpe wrote: > > diff --git a/include/linux/iommu.h b/include/linux/iommu.h > > index 38c65e92ecd091..e414951c0af83f 100644 > > --- a/include/linux/iommu.h > > +++ b/include/linux/iommu.h > > @@ -326,6 +326,18 @@ typedef unsigned int ioasid_t; > > /* Read but do not clear any dirty bits */ > > #define IOMMU_DIRTY_NO_CLEAR (1 << 0) > > +/* > > + * Pages allocated through iommu_alloc_pages_node() can be placed on this list > > + * using iommu_pages_list_add(). Note: ONLY pages from iommu_alloc_pages_node() > > + * can be used this way! > > + */ > > +struct iommu_pages_list { > > + struct list_head pages; > > +}; > > + > > +#define IOMMU_PAGES_LIST_INIT(name) \ > > + ((struct iommu_pages_list){ .pages = LIST_HEAD_INIT(name.pages) }) > > + > > #ifdef CONFIG_IOMMU_API > > Any reason why the above cannot be placed in the iommu-pages.h header > file? My understanding is that iommu-pages is only for the iommu drivers > and should not be accessible for external subsystems. I wanted to do that, but the issue is the gather: struct iommu_iotlb_gather { unsigned long start; unsigned long end; size_t pgsize; struct iommu_pages_list freelist; The struct is inlined so it must be declared. I do not want to include iommu-pages.h in this header. Once the struct itself is there it made sense to include the INIT too. FWIW I have a longstanding desire to split iommu.h into internal-driver-facing and external-user-facing files.. Thanks, Jason
diff --git a/drivers/iommu/iommu-pages.c b/drivers/iommu/iommu-pages.c index 31ff83ffaf0106..af8694b46417fa 100644 --- a/drivers/iommu/iommu-pages.c +++ b/drivers/iommu/iommu-pages.c @@ -67,18 +67,25 @@ void iommu_free_pages(void *virt) EXPORT_SYMBOL_GPL(iommu_free_pages); /** - * iommu_put_pages_list - free a list of pages. - * @head: the head of the lru list to be freed. + * iommu_put_pages_list_new - free a list of pages. + * @list: The list of pages to be freed * * Frees a list of pages allocated by iommu_alloc_pages_node(). */ -void iommu_put_pages_list(struct list_head *head) +void iommu_put_pages_list_new(struct iommu_pages_list *list) { - while (!list_empty(head)) { - struct page *p = list_entry(head->prev, struct page, lru); + struct page *p, *tmp; - list_del(&p->lru); + list_for_each_entry_safe(p, tmp, &list->pages, lru) __iommu_free_page(p); - } } -EXPORT_SYMBOL_GPL(iommu_put_pages_list); +EXPORT_SYMBOL_GPL(iommu_put_pages_list_new); + +void iommu_put_pages_list_old(struct list_head *head) +{ + struct page *p, *tmp; + + list_for_each_entry_safe(p, tmp, head, lru) + __iommu_free_page(p); +} +EXPORT_SYMBOL_GPL(iommu_put_pages_list_old); diff --git a/drivers/iommu/iommu-pages.h b/drivers/iommu/iommu-pages.h index e3c35aa14ad716..0acc26af7202df 100644 --- a/drivers/iommu/iommu-pages.h +++ b/drivers/iommu/iommu-pages.h @@ -7,12 +7,51 @@ #ifndef __IOMMU_PAGES_H #define __IOMMU_PAGES_H -#include <linux/types.h> -#include <linux/topology.h> +#include <linux/iommu.h> void *iommu_alloc_pages_node(int nid, gfp_t gfp, unsigned int order); void iommu_free_pages(void *virt); -void iommu_put_pages_list(struct list_head *head); +void iommu_put_pages_list_new(struct iommu_pages_list *list); +void iommu_put_pages_list_old(struct list_head *head); + +#define iommu_put_pages_list(head) \ + _Generic(head, \ + struct iommu_pages_list *: iommu_put_pages_list_new, \ + struct list_head *: iommu_put_pages_list_old)(head) + +/** + * iommu_pages_list_add - add the page to a iommu_pages_list + * @list: List to add the page to + * @virt: Address returned from iommu_alloc_pages_node() + */ +static inline void iommu_pages_list_add(struct iommu_pages_list *list, + void *virt) +{ + list_add_tail(&virt_to_page(virt)->lru, &list->pages); +} + +/** + * iommu_pages_list_splice - Put all the pages in list from into list to + * @from: Source list of pages + * @to: Destination list of pages + * + * from must be re-initialized after calling this function if it is to be + * used again. + */ +static inline void iommu_pages_list_splice(struct iommu_pages_list *from, + struct iommu_pages_list *to) +{ + list_splice(&from->pages, &to->pages); +} + +/** + * iommu_pages_list_empty - True if the list is empty + * @list: List to check + */ +static inline bool iommu_pages_list_empty(struct iommu_pages_list *list) +{ + return list_empty(&list->pages); +} /** * iommu_alloc_pages - allocate a zeroed page of a given order diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 38c65e92ecd091..e414951c0af83f 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -326,6 +326,18 @@ typedef unsigned int ioasid_t; /* Read but do not clear any dirty bits */ #define IOMMU_DIRTY_NO_CLEAR (1 << 0) +/* + * Pages allocated through iommu_alloc_pages_node() can be placed on this list + * using iommu_pages_list_add(). Note: ONLY pages from iommu_alloc_pages_node() + * can be used this way! + */ +struct iommu_pages_list { + struct list_head pages; +}; + +#define IOMMU_PAGES_LIST_INIT(name) \ + ((struct iommu_pages_list){ .pages = LIST_HEAD_INIT(name.pages) }) + #ifdef CONFIG_IOMMU_API /**
We want to get rid of struct page references outside the internal allocator implementation. The free list has the driver open code something like: list_add_tail(&virt_to_page(ptr)->lru, freelist); Move the above into a small inline and make the freelist into a wrapper type 'struct iommu_pages_list' so that the compiler can help check all the conversion. This struct has also proven helpful in some future ideas to convert to a singly linked list to get an extra pointer in the struct page, and to signal that the pages should be freed with RCU. Use a temporary _Generic so we don't need to rename the free function as the patches progress. Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> --- drivers/iommu/iommu-pages.c | 23 ++++++++++++------- drivers/iommu/iommu-pages.h | 45 ++++++++++++++++++++++++++++++++++--- include/linux/iommu.h | 12 ++++++++++ 3 files changed, 69 insertions(+), 11 deletions(-)