Re: [RFC] iommu: Use put_pages_list
On Thu, Oct 14, 2021 at 05:17:18PM +0100, Robin Murphy wrote: > On 2021-10-14 12:52, John Garry wrote: > > On 14/10/2021 12:20, Matthew Wilcox wrote: > > > I'm going to keep pinging this patch weekly. > > > > > > On Thu, Oct 07, 2021 at 07:17:02PM +0100, Matthew Wilcox wrote: > > > > ping? > > > > Robin, Were you checking this? You mentioned "I got > > side-tracked trying to make io-pgtable use that freelist properly" in > > another thread, which seems related. > > Ooh, thanks for the heads-up John - I'm still only just starting to catch up > on my mailing list folders since I got back off holiday. > > Indeed I already started untangling the freelist handling in the flush queue > code (to make the move into iommu-dma smaller). Once I'd figured out how it > worked I did wonder whether there was any more "standard" field to borrow, > since page->freelist did seem very much in the minority. If page->lru is it > then great! From a quick skim of the patch I think I'd only have a few > trivial review comments to make - certainly no objection to the fundamental > change itself (indeed I hit a point in io-pgtable-arm where adding to the > pointer chain got rather awkward, so having proper lists to splice would be > lovely). Great to hear! > Matthew - is this something getting in the way of mm development, or just a > nice cleanup? I'd be happy either to pursue merging it on its own, or to > pick it up and work it into a series with my stuff. This is probably going to get in the way of MM development in ~6 months time. I'm happy for you to pick it up and put it in a series of your own! BTW, the optimisation of the implementation of put_pages_list() is sitting in akpm's tree, so if you see a performance problem, please give that a try. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC] iommu: Use put_pages_list
On 2021-10-14 12:52, John Garry wrote: On 14/10/2021 12:20, Matthew Wilcox wrote: I'm going to keep pinging this patch weekly. On Thu, Oct 07, 2021 at 07:17:02PM +0100, Matthew Wilcox wrote: ping? Robin, Were you checking this? You mentioned "I got side-tracked trying to make io-pgtable use that freelist properly" in another thread, which seems related. Ooh, thanks for the heads-up John - I'm still only just starting to catch up on my mailing list folders since I got back off holiday. Indeed I already started untangling the freelist handling in the flush queue code (to make the move into iommu-dma smaller). Once I'd figured out how it worked I did wonder whether there was any more "standard" field to borrow, since page->freelist did seem very much in the minority. If page->lru is it then great! From a quick skim of the patch I think I'd only have a few trivial review comments to make - certainly no objection to the fundamental change itself (indeed I hit a point in io-pgtable-arm where adding to the pointer chain got rather awkward, so having proper lists to splice would be lovely). Matthew - is this something getting in the way of mm development, or just a nice cleanup? I'd be happy either to pursue merging it on its own, or to pick it up and work it into a series with my stuff. Cheers, Robin. Thanks, John On Thu, Sep 30, 2021 at 05:20:42PM +0100, Matthew Wilcox (Oracle) wrote: page->freelist is for the use of slab. We already have the ability to free a list of pages in the core mm, but it requires the use of a list_head and for the pages to be chained together through page->lru. Switch the iommu code over to using free_pages_list(). Signed-off-by: Matthew Wilcox (Oracle) --- drivers/iommu/amd/io_pgtable.c | 99 +++--- drivers/iommu/dma-iommu.c | 11 +--- drivers/iommu/intel/iommu.c | 89 +++--- include/linux/iommu.h | 3 +- 4 files changed, 77 insertions(+), 125 deletions(-) ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC] iommu: Use put_pages_list
On 14/10/2021 12:20, Matthew Wilcox wrote: I'm going to keep pinging this patch weekly. On Thu, Oct 07, 2021 at 07:17:02PM +0100, Matthew Wilcox wrote: ping? Robin, Were you checking this? You mentioned "I got side-tracked trying to make io-pgtable use that freelist properly" in another thread, which seems related. Thanks, John On Thu, Sep 30, 2021 at 05:20:42PM +0100, Matthew Wilcox (Oracle) wrote: page->freelist is for the use of slab. We already have the ability to free a list of pages in the core mm, but it requires the use of a list_head and for the pages to be chained together through page->lru. Switch the iommu code over to using free_pages_list(). Signed-off-by: Matthew Wilcox (Oracle) --- drivers/iommu/amd/io_pgtable.c | 99 +++--- drivers/iommu/dma-iommu.c | 11 +--- drivers/iommu/intel/iommu.c| 89 +++--- include/linux/iommu.h | 3 +- 4 files changed, 77 insertions(+), 125 deletions(-) ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC] iommu: Use put_pages_list
I'm going to keep pinging this patch weekly.
On Thu, Oct 07, 2021 at 07:17:02PM +0100, Matthew Wilcox wrote:
> ping?
>
> On Thu, Sep 30, 2021 at 05:20:42PM +0100, Matthew Wilcox (Oracle) wrote:
> > page->freelist is for the use of slab. We already have the ability
> > to free a list of pages in the core mm, but it requires the use of a
> > list_head and for the pages to be chained together through page->lru.
> > Switch the iommu code over to using free_pages_list().
> >
> > Signed-off-by: Matthew Wilcox (Oracle)
> > ---
> > drivers/iommu/amd/io_pgtable.c | 99 +++---
> > drivers/iommu/dma-iommu.c | 11 +---
> > drivers/iommu/intel/iommu.c| 89 +++---
> > include/linux/iommu.h | 3 +-
> > 4 files changed, 77 insertions(+), 125 deletions(-)
> >
> > diff --git a/drivers/iommu/amd/io_pgtable.c b/drivers/iommu/amd/io_pgtable.c
> > index 182c93a43efd..8dfa6ee58b76 100644
> > --- a/drivers/iommu/amd/io_pgtable.c
> > +++ b/drivers/iommu/amd/io_pgtable.c
> > @@ -74,49 +74,37 @@ static u64 *first_pte_l7(u64 *pte, unsigned long
> > *page_size,
> > *
> >
> > /
> >
> > -static void free_page_list(struct page *freelist)
> > -{
> > - while (freelist != NULL) {
> > - unsigned long p = (unsigned long)page_address(freelist);
> > -
> > - freelist = freelist->freelist;
> > - free_page(p);
> > - }
> > -}
> > -
> > -static struct page *free_pt_page(unsigned long pt, struct page *freelist)
> > +static void free_pt_page(unsigned long pt, struct list_head *list)
> > {
> > struct page *p = virt_to_page((void *)pt);
> >
> > - p->freelist = freelist;
> > -
> > - return p;
> > + list_add_tail(&p->lru, list);
> > }
> >
> > #define DEFINE_FREE_PT_FN(LVL, FN)
> > \
> > -static struct page *free_pt_##LVL (unsigned long __pt, struct page
> > *freelist) \
> > -{
> > \
> > - unsigned long p;
> > \
> > - u64 *pt;
> > \
> > - int i;
> > \
> > -
> > \
> > - pt = (u64 *)__pt;
> > \
> > -
> > \
> > - for (i = 0; i < 512; ++i) {
> > \
> > - /* PTE present? */
> > \
> > - if (!IOMMU_PTE_PRESENT(pt[i]))
> > \
> > - continue;
> > \
> > -
> > \
> > - /* Large PTE? */
> > \
> > - if (PM_PTE_LEVEL(pt[i]) == 0 ||
> > \
> > - PM_PTE_LEVEL(pt[i]) == 7)
> > \
> > - continue;
> > \
> > -
> > \
> > - p = (unsigned long)IOMMU_PTE_PAGE(pt[i]);
> > \
> > - freelist = FN(p, freelist);
> > \
> > - }
> > \
> > -
> > \
> > - return free_pt_page((unsigned long)pt, freelist);
> > \
> > +static void free_pt_##LVL (unsigned long __pt, struct list_head *list)
> > \
> > +{ \
> > + unsigned long p;\
> > + u64 *pt;\
> > + int i; \
> > + \
> > + pt = (u64 *)__pt; \
> > + \
> > + for (i = 0; i < 512; ++i) { \
> > + /* PTE present? */ \
> > + if (!IOMMU_PTE_PRESENT(pt[i])) \
> > + continue; \
> > + \
> > + /* Large PTE? */\
> > + if (PM_PT
Re: [RFC] iommu: Use put_pages_list
ping?
On Thu, Sep 30, 2021 at 05:20:42PM +0100, Matthew Wilcox (Oracle) wrote:
> page->freelist is for the use of slab. We already have the ability
> to free a list of pages in the core mm, but it requires the use of a
> list_head and for the pages to be chained together through page->lru.
> Switch the iommu code over to using free_pages_list().
>
> Signed-off-by: Matthew Wilcox (Oracle)
> ---
> drivers/iommu/amd/io_pgtable.c | 99 +++---
> drivers/iommu/dma-iommu.c | 11 +---
> drivers/iommu/intel/iommu.c| 89 +++---
> include/linux/iommu.h | 3 +-
> 4 files changed, 77 insertions(+), 125 deletions(-)
>
> diff --git a/drivers/iommu/amd/io_pgtable.c b/drivers/iommu/amd/io_pgtable.c
> index 182c93a43efd..8dfa6ee58b76 100644
> --- a/drivers/iommu/amd/io_pgtable.c
> +++ b/drivers/iommu/amd/io_pgtable.c
> @@ -74,49 +74,37 @@ static u64 *first_pte_l7(u64 *pte, unsigned long
> *page_size,
> *
>
> /
>
> -static void free_page_list(struct page *freelist)
> -{
> - while (freelist != NULL) {
> - unsigned long p = (unsigned long)page_address(freelist);
> -
> - freelist = freelist->freelist;
> - free_page(p);
> - }
> -}
> -
> -static struct page *free_pt_page(unsigned long pt, struct page *freelist)
> +static void free_pt_page(unsigned long pt, struct list_head *list)
> {
> struct page *p = virt_to_page((void *)pt);
>
> - p->freelist = freelist;
> -
> - return p;
> + list_add_tail(&p->lru, list);
> }
>
> #define DEFINE_FREE_PT_FN(LVL, FN)
> \
> -static struct page *free_pt_##LVL (unsigned long __pt, struct page
> *freelist)\
> -{
> \
> - unsigned long p;
> \
> - u64 *pt;
> \
> - int i;
> \
> -
> \
> - pt = (u64 *)__pt;
> \
> -
> \
> - for (i = 0; i < 512; ++i) {
> \
> - /* PTE present? */
> \
> - if (!IOMMU_PTE_PRESENT(pt[i]))
> \
> - continue;
> \
> -
> \
> - /* Large PTE? */
> \
> - if (PM_PTE_LEVEL(pt[i]) == 0 ||
> \
> - PM_PTE_LEVEL(pt[i]) == 7)
> \
> - continue;
> \
> -
> \
> - p = (unsigned long)IOMMU_PTE_PAGE(pt[i]);
> \
> - freelist = FN(p, freelist);
> \
> - }
> \
> -
> \
> - return free_pt_page((unsigned long)pt, freelist);
> \
> +static void free_pt_##LVL (unsigned long __pt, struct list_head *list)
> \
> +{\
> + unsigned long p;\
> + u64 *pt;\
> + int i; \
> + \
> + pt = (u64 *)__pt; \
> + \
> + for (i = 0; i < 512; ++i) { \
> + /* PTE present? */ \
> + if (!IOMMU_PTE_PRESENT(pt[i])) \
> + continue; \
> + \
> + /* Large PTE? */\
> + if (PM_PTE_LEVEL(pt[i]) == 0 || \
> + PM_PTE_LEVEL(pt[i]) == 7) \
> + continue; \
> +
