答复: 答复: 答复: [PATCH V2] vfio dma_map/unmap: optimized for hugetlbfs pages
> > On 2020/8/28 下午10:24, Peter Xu wrote: > > On Fri, Aug 28, 2020 at 09:23:08AM +, Maoming (maoming, Cloud > Infrastructure Service Product Dept.) wrote: > >> In hugetlb_put_pfn(), I delete unpin_user_pages_dirty_lock() and use some > simple code to put hugetlb pages. > >> Is this right? > > I think we should still use the APIs because of the the same reason. > > However again I don't know the performance impact of that to your > > patch, but I still think that could be done inside gup itself when > > needed (e.g., a special path for hugetlbfs for [un]pinning continuous > > pages; though if that's the case that could be something to be discussed on > -mm then as a separate patch, imho). > > > > Thanks, > > > +1, we should make this as a generic optimization instead of VFIO > specific consider there're a lot of GUP users. > > Thanks > Thanks for your suggestions. You are right, I will fix it in the next version.
Re: 答复: 答复: [PATCH V2] vfio dma_map/unmap: optimized for hugetlbfs pages
On 2020/8/28 下午10:24, Peter Xu wrote: On Fri, Aug 28, 2020 at 09:23:08AM +, Maoming (maoming, Cloud Infrastructure Service Product Dept.) wrote: In hugetlb_put_pfn(), I delete unpin_user_pages_dirty_lock() and use some simple code to put hugetlb pages. Is this right? I think we should still use the APIs because of the the same reason. However again I don't know the performance impact of that to your patch, but I still think that could be done inside gup itself when needed (e.g., a special path for hugetlbfs for [un]pinning continuous pages; though if that's the case that could be something to be discussed on -mm then as a separate patch, imho). Thanks, +1, we should make this as a generic optimization instead of VFIO specific consider there're a lot of GUP users. Thanks
Re: 答复: 答复: [PATCH V2] vfio dma_map/unmap: optimized for hugetlbfs pages
On Fri, Aug 28, 2020 at 09:23:08AM +, Maoming (maoming, Cloud Infrastructure Service Product Dept.) wrote: > In hugetlb_put_pfn(), I delete unpin_user_pages_dirty_lock() and use some > simple code to put hugetlb pages. > Is this right? I think we should still use the APIs because of the the same reason. However again I don't know the performance impact of that to your patch, but I still think that could be done inside gup itself when needed (e.g., a special path for hugetlbfs for [un]pinning continuous pages; though if that's the case that could be something to be discussed on -mm then as a separate patch, imho). Thanks, -- Peter Xu
答复: 答复: [PATCH V2] vfio dma_map/unmap: optimized for hugetlbfs pages
On Wed, Aug 26, 2020 at 01:56:43PM +, Maoming (maoming, Cloud
Infrastructure Service Product Dept.) wrote:
> > + /*
> > +* Unlike THP, the splitting should not happen for hugetlb pages.
> > +* Since PG_reserved is not relevant for compound pages, and the pfn of
> > +* PAGE_SIZE page which in hugetlb pages is valid,
> > +* it is not necessary to check rsvd for hugetlb pages.
> > +* We do not need to alloc pages because of vaddr and we can finish all
> > +* work by a single operation to the head page.
> > +*/
> > + atomic_add(contiguous_npage, compound_pincount_ptr(head));
> > + page_ref_add(head, contiguous_npage);
> > + mod_node_page_state(page_pgdat(head), NR_FOLL_PIN_ACQUIRED,
> > +contiguous_npage);
>
> I think I asked this question in v1, but I didn't get any answer... So I'm
> trying again...
>
> Could I ask why manual referencing of pages is done here rather than
> using
> pin_user_pages_remote() just like what we've done with
> vaddr_get_pfn(), and let
> try_grab_page() to do the page reference and accountings?
>
> I feel like this at least is against the FOLL_PIN workflow of gup, because
> those FOLL_PIN paths were bypassed, afaict.
>
>
> Hi,
> My apologies for not answering your question.
> As I understand, pin_user_pages_remote() might spend much time.
> Because all PAGE_SIZE-pages in a hugetlb page are pinned one by one in
> pin_user_pages_remote() and try_grab_page().
> So I think maybe we can use these simple code to do all work.
> Am I wrong? And is there something else we can use? For example
> :pin_user_pages_fast()
Yeah I can understand your concern, however so far it's not about the perf but
correctness. Documentation/core-api/pin_user_pages.rst tells us that we should
always use pin_user_page*() APIs to pin DMA pages (with FOLL_LONGTERM). That's
something we should follow for now, otherwise the major logic of either
FOLL_PIN or FULL_LONGTERM could be bypassed without being noticed.
I'm not sure whether the perf issue is a big one. So have you tried the pin
page APIs first and did some measurement? There is indeed a tight loop in
follow_hugetlb_page() however not sure how much it'll affect VFIO_IOMMU_MAP_DMA
in general. Even if we want to do something, it seems to be more suitable to
be done inside follow_hugetlb_page() rather than in vfio, imho.
Another comment is about the design of the whole patch - I think Alex commented
on that too on the awkwardness on appending the hugetlbfs logic to the end of
the existing logic. Considering that current logic of vfio_pin_pages_remote()
is "let's pin some pages as long as continuous", not sure whether we can make
it into:
vfio_pin_pages_remote()
{
if (PageHuge(first_page))
vfio_pin_pages_hugetlbfs();
else
vfio_pin_pages_normal();
}
The thing is, if the 1st page is normal page, then the follow-up pages
shouldn't normally be hugetlbfs pages so they won't be physically continuous.
Vice versa. In other words, each call to vfio_pin_pages_remote() should only
handle only one type of page after all. So maybe we can diverge them at the
beginning of the call directly.
--
Peter Xu
Thanks for your suggestions. I will fix it.
And I have another question.
In hugetlb_put_pfn(), I delete unpin_user_pages_dirty_lock() and use some
simple code to put hugetlb pages.
Is this right?
/*
* put pfns for a hugetlb page
* @start:the PAGE_SIZE-page we start to put,can be any page in this hugetlb
page
* @npage:the number of PAGE_SIZE-pages need to put
* @prot:IOMMU_READ/WRITE
*/
static int hugetlb_put_pfn(unsigned long start, unsigned int npage, int prot)
{
struct page *page;
struct page *head;
if (!npage || !pfn_valid(start))
return 0;
page = pfn_to_page(start);
if (!page || !PageHuge(page))
return 0;
head = compound_head(page);
/*
* The last page should be in this hugetlb page.
* The number of putting pages should be equal to the number
* of getting pages.So the hugepage pinned refcount and the normal
* page refcount can not be smaller than npage.
*/
if ((head != compound_head(pfn_to_page(start + npage - 1)))
|| (page_ref_count(head) < npage)
|| (compound_pincount(page) < npage))
return 0;
if ((prot & IOMMU_WRITE) && !PageDirty(page))
set_page_dirty_lock(page);
atomic_sub(npage, compound_pincount_ptr(head));
if (page_ref_sub_and_test(head, npage))
__put_page(head);
mod_node_page_state(page_pgdat(head), NR_FOLL_PIN_RELEASED, npage);
return 1;
}
答复: [PATCH V2] vfio dma_map/unmap: optimized for hugetlbfs pages
Hi,
Thanks for taking a look.
Some replies below:
On Fri, 14 Aug 2020 10:37:29 +0800
Ming Mao wrote:
> In the original process of pinning/unpinning pages for VFIO-devices,
> to make sure the pages are contiguous, we have to check them one by one.
> As a result, dma_map/unmap could spend a long time.
> Using the hugetlb pages, we can avoid this problem.
> All pages in hugetlb pages are contiguous.And the hugetlb page should
> not be split.So we can delete the for loops and use some
> operations(such as atomic_add,page_ref_add) instead.
>
> Signed-off-by: Ming Mao
> ---
> drivers/vfio/vfio_iommu_type1.c | 233
> +++-
> 1 file changed, 230 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c
> b/drivers/vfio/vfio_iommu_type1.c index 5e556ac91..8957013c1 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -415,6 +415,46 @@ static int put_pfn(unsigned long pfn, int prot)
> return 0;
> }
>
> +/*
> + * put pfns for a hugetlb page
> + * @start:the PAGE_SIZE-page we start to put,can be any page in this
> +hugetlb page
> + * @npage:the number of PAGE_SIZE-pages need to put
> + * @prot:IOMMU_READ/WRITE
> + */
> +static int hugetlb_put_pfn(unsigned long start, unsigned int npage,
> +int prot) {
> + struct page *page;
> + struct page *head;
> +
> + if (!npage || !pfn_valid(start))
> + return 0;
> +
> + page = pfn_to_page(start);
> + if (!page || !PageHuge(page))
> + return 0;
> + head = compound_head(page);
> + /*
> + * The last page should be in this hugetlb page.
> + * The number of putting pages should be equal to the number
> + * of getting pages.So the hugepage pinned refcount and the normal
> + * page refcount can not be smaller than npage.
> + */
> + if ((head != compound_head(pfn_to_page(start + npage - 1)))
> + || (page_ref_count(head) < npage)
> + || (compound_pincount(page) < npage))
> + return 0;
> +
> + if ((prot & IOMMU_WRITE) && !PageDirty(page))
> + set_page_dirty_lock(page);
> +
> + atomic_sub(npage, compound_pincount_ptr(head));
> + if (page_ref_sub_and_test(head, npage))
> + __put_page(head);
> +
> + mod_node_page_state(page_pgdat(head), NR_FOLL_PIN_RELEASED, npage);
> + return 1;
> +}
> +
> static int follow_fault_pfn(struct vm_area_struct *vma, struct mm_struct *mm,
> unsigned long vaddr, unsigned long *pfn,
> bool write_fault)
> @@ -479,6 +519,105 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned
> long vaddr,
> return ret;
> }
>
> +static bool is_hugetlbpage(unsigned long pfn) {
> + struct page *page;
> +
> + if (!pfn_valid(pfn))
> + return false;
> +
> + page = pfn_to_page(pfn);
> + /* only check for hugetlb pages */
> + if (!page || !PageHuge(page))
> + return false;
> +
> + return true;
return page && PageHuge(page);
Yes,this is better. I will fix it.
> +}
> +
> +/*
> + * get the number of residual PAGE_SIZE-pages in a hugetlb page
> + * (including the page which pointed by this address)
> + * @address: we count residual pages from this address to the end of
> + * a hugetlb page
> + * @order: the order of the same hugetlb page */ static long
> +hugetlb_get_residual_pages(unsigned long address, unsigned int order)
> +{
> + unsigned long hugetlb_npage;
> + unsigned long hugetlb_mask;
> +
> + if (!order)
> + return -1;
Use a real errno please.
Yes, I will fix it.
> +
> + hugetlb_npage = _AC(1, UL) << order;
This doesn't seem an appropriate use of _AC(), 1UL << order should be fine.
Yes, I will fix it.
> + hugetlb_mask = (hugetlb_npage << PAGE_SHIFT) - 1;
> + address = ALIGN_DOWN(address, PAGE_SIZE);
hugetlb_mask doesn't need to be in bytes, it could be in pages (hugetlb_npage -
1), then we could simply convert address to pfn (address >> PAGE_SHIFT), then
we avoid the shift below:
return hugetlb_npage - ((address >> PAGE_SHIFT) & (hugetlb_npage - 1));
Yes,this is better. I will fix it.
> +
> + /*
> + * Since we count the page pointed by this address, the number of
> + * residual PAGE_SIZE-pages is greater than or equal to 1.
> + */
> + return hugetlb_npage - ((address & hugetlb_mask) >> PAGE_SHIFT); }
> +
> +static unsigned int
> +hugetlb_page_get_externally_pinned_num(struct vfio_dma *dma,
> + unsigned long start,
> + unsigned long npage)
> +{
> + struct vfio_pfn *vpfn;
> + struct rb_node *node;
> + unsigned long end = start + npage - 1;
> + unsigned int num = 0;
> +
> + if (!dma || !npage)
> + return 0;
> +
> + /* If we find a page in dma->pfn_list, this page has been pinned
> externally */
> + for (node = rb_first(&dma->pfn_list); n
Re: 答复: [PATCH V2] vfio dma_map/unmap: optimized for hugetlbfs pages
On Wed, Aug 26, 2020 at 01:56:43PM +, Maoming (maoming, Cloud
Infrastructure Service Product Dept.) wrote:
> > + /*
> > +* Unlike THP, the splitting should not happen for hugetlb pages.
> > +* Since PG_reserved is not relevant for compound pages, and the pfn of
> > +* PAGE_SIZE page which in hugetlb pages is valid,
> > +* it is not necessary to check rsvd for hugetlb pages.
> > +* We do not need to alloc pages because of vaddr and we can finish all
> > +* work by a single operation to the head page.
> > +*/
> > + atomic_add(contiguous_npage, compound_pincount_ptr(head));
> > + page_ref_add(head, contiguous_npage);
> > + mod_node_page_state(page_pgdat(head), NR_FOLL_PIN_ACQUIRED,
> > +contiguous_npage);
>
> I think I asked this question in v1, but I didn't get any answer... So I'm
> trying again...
>
> Could I ask why manual referencing of pages is done here rather than using
> pin_user_pages_remote() just like what we've done with vaddr_get_pfn(), and
> let
> try_grab_page() to do the page reference and accountings?
>
> I feel like this at least is against the FOLL_PIN workflow of gup, because
> those FOLL_PIN paths were bypassed, afaict.
>
>
> Hi,
> My apologies for not answering your question.
> As I understand, pin_user_pages_remote() might spend much time.
> Because all PAGE_SIZE-pages in a hugetlb page are pinned one by one in
> pin_user_pages_remote() and try_grab_page().
> So I think maybe we can use these simple code to do all work.
> Am I wrong? And is there something else we can use? For example
> :pin_user_pages_fast()
Yeah I can understand your concern, however so far it's not about the perf but
correctness. Documentation/core-api/pin_user_pages.rst tells us that we should
always use pin_user_page*() APIs to pin DMA pages (with FOLL_LONGTERM). That's
something we should follow for now, otherwise the major logic of either
FOLL_PIN or FULL_LONGTERM could be bypassed without being noticed.
I'm not sure whether the perf issue is a big one. So have you tried the pin
page APIs first and did some measurement? There is indeed a tight loop in
follow_hugetlb_page() however not sure how much it'll affect VFIO_IOMMU_MAP_DMA
in general. Even if we want to do something, it seems to be more suitable to
be done inside follow_hugetlb_page() rather than in vfio, imho.
Another comment is about the design of the whole patch - I think Alex commented
on that too on the awkwardness on appending the hugetlbfs logic to the end of
the existing logic. Considering that current logic of vfio_pin_pages_remote()
is "let's pin some pages as long as continuous", not sure whether we can make
it into:
vfio_pin_pages_remote()
{
if (PageHuge(first_page))
vfio_pin_pages_hugetlbfs();
else
vfio_pin_pages_normal();
}
The thing is, if the 1st page is normal page, then the follow-up pages
shouldn't normally be hugetlbfs pages so they won't be physically continuous.
Vice versa. In other words, each call to vfio_pin_pages_remote() should only
handle only one type of page after all. So maybe we can diverge them at the
beginning of the call directly.
--
Peter Xu
答复: [PATCH V2] vfio dma_map/unmap: optimized for hugetlbfs pages
-邮件原件-
发件人: Peter Xu [mailto:pet...@redhat.com]
发送时间: 2020年8月26日 4:59
收件人: Maoming (maoming, Cloud Infrastructure Service Product Dept.)
抄送: linux-kernel@vger.kernel.org; k...@vger.kernel.org;
alex.william...@redhat.com; coh...@redhat.com; Zhoujian (jay)
; Huangweidong (C) ;
aarca...@redhat.com
主题: Re: [PATCH V2] vfio dma_map/unmap: optimized for hugetlbfs pages
On Fri, Aug 14, 2020 at 10:37:29AM +0800, Ming Mao wrote:
> +static long hugetlb_page_vaddr_get_pfn(unsigned long vaddr, long npage,
> + unsigned long pfn)
> +{
> + long hugetlb_residual_npage;
> + long contiguous_npage;
> + struct page *head = compound_head(pfn_to_page(pfn));
> +
> + /*
> + * If pfn is valid,
> + * hugetlb_residual_npage is greater than or equal to 1.
> + */
> + hugetlb_residual_npage = hugetlb_get_residual_pages(vaddr,
> + compound_order(head));
> + if (hugetlb_residual_npage < 0)
> + return -1;
> +
> + /* The page of vaddr has been gotten by vaddr_get_pfn */
> + contiguous_npage = min_t(long, (hugetlb_residual_npage - 1), npage);
> + if (!contiguous_npage)
> + return 0;
> + /*
> + * Unlike THP, the splitting should not happen for hugetlb pages.
> + * Since PG_reserved is not relevant for compound pages, and the pfn of
> + * PAGE_SIZE page which in hugetlb pages is valid,
> + * it is not necessary to check rsvd for hugetlb pages.
> + * We do not need to alloc pages because of vaddr and we can finish all
> + * work by a single operation to the head page.
> + */
> + atomic_add(contiguous_npage, compound_pincount_ptr(head));
> + page_ref_add(head, contiguous_npage);
> + mod_node_page_state(page_pgdat(head), NR_FOLL_PIN_ACQUIRED,
> +contiguous_npage);
I think I asked this question in v1, but I didn't get any answer... So I'm
trying again...
Could I ask why manual referencing of pages is done here rather than using
pin_user_pages_remote() just like what we've done with vaddr_get_pfn(), and let
try_grab_page() to do the page reference and accountings?
I feel like this at least is against the FOLL_PIN workflow of gup, because
those FOLL_PIN paths were bypassed, afaict.
Hi,
My apologies for not answering your question.
As I understand, pin_user_pages_remote() might spend much time.
Because all PAGE_SIZE-pages in a hugetlb page are pinned one by one in
pin_user_pages_remote() and try_grab_page().
So I think maybe we can use these simple code to do all work.
Am I wrong? And is there something else we can use? For example
:pin_user_pages_fast()
> +
> + return contiguous_npage;
> +}
--
Peter Xu
Re: [PATCH V2] vfio dma_map/unmap: optimized for hugetlbfs pages
On Fri, Aug 14, 2020 at 10:37:29AM +0800, Ming Mao wrote:
> +static long hugetlb_page_vaddr_get_pfn(unsigned long vaddr, long npage,
> + unsigned long pfn)
> +{
> + long hugetlb_residual_npage;
> + long contiguous_npage;
> + struct page *head = compound_head(pfn_to_page(pfn));
> +
> + /*
> + * If pfn is valid,
> + * hugetlb_residual_npage is greater than or equal to 1.
> + */
> + hugetlb_residual_npage = hugetlb_get_residual_pages(vaddr,
> + compound_order(head));
> + if (hugetlb_residual_npage < 0)
> + return -1;
> +
> + /* The page of vaddr has been gotten by vaddr_get_pfn */
> + contiguous_npage = min_t(long, (hugetlb_residual_npage - 1), npage);
> + if (!contiguous_npage)
> + return 0;
> + /*
> + * Unlike THP, the splitting should not happen for hugetlb pages.
> + * Since PG_reserved is not relevant for compound pages, and the pfn of
> + * PAGE_SIZE page which in hugetlb pages is valid,
> + * it is not necessary to check rsvd for hugetlb pages.
> + * We do not need to alloc pages because of vaddr and we can finish all
> + * work by a single operation to the head page.
> + */
> + atomic_add(contiguous_npage, compound_pincount_ptr(head));
> + page_ref_add(head, contiguous_npage);
> + mod_node_page_state(page_pgdat(head), NR_FOLL_PIN_ACQUIRED,
> contiguous_npage);
I think I asked this question in v1, but I didn't get any answer... So I'm
trying again...
Could I ask why manual referencing of pages is done here rather than using
pin_user_pages_remote() just like what we've done with vaddr_get_pfn(), and let
try_grab_page() to do the page reference and accountings?
I feel like this at least is against the FOLL_PIN workflow of gup, because
those FOLL_PIN paths were bypassed, afaict.
> +
> + return contiguous_npage;
> +}
--
Peter Xu
Re: [PATCH V2] vfio dma_map/unmap: optimized for hugetlbfs pages
On Fri, 14 Aug 2020 10:37:29 +0800
Ming Mao wrote:
> In the original process of pinning/unpinning pages for VFIO-devices,
> to make sure the pages are contiguous, we have to check them one by one.
> As a result, dma_map/unmap could spend a long time.
> Using the hugetlb pages, we can avoid this problem.
> All pages in hugetlb pages are contiguous.And the hugetlb
> page should not be split.So we can delete the for loops and use
> some operations(such as atomic_add,page_ref_add) instead.
>
> Signed-off-by: Ming Mao
> ---
> drivers/vfio/vfio_iommu_type1.c | 233 +++-
> 1 file changed, 230 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 5e556ac91..8957013c1 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -415,6 +415,46 @@ static int put_pfn(unsigned long pfn, int prot)
> return 0;
> }
>
> +/*
> + * put pfns for a hugetlb page
> + * @start:the PAGE_SIZE-page we start to put,can be any page in this hugetlb
> page
> + * @npage:the number of PAGE_SIZE-pages need to put
> + * @prot:IOMMU_READ/WRITE
> + */
> +static int hugetlb_put_pfn(unsigned long start, unsigned int npage, int prot)
> +{
> + struct page *page;
> + struct page *head;
> +
> + if (!npage || !pfn_valid(start))
> + return 0;
> +
> + page = pfn_to_page(start);
> + if (!page || !PageHuge(page))
> + return 0;
> + head = compound_head(page);
> + /*
> + * The last page should be in this hugetlb page.
> + * The number of putting pages should be equal to the number
> + * of getting pages.So the hugepage pinned refcount and the normal
> + * page refcount can not be smaller than npage.
> + */
> + if ((head != compound_head(pfn_to_page(start + npage - 1)))
> + || (page_ref_count(head) < npage)
> + || (compound_pincount(page) < npage))
> + return 0;
> +
> + if ((prot & IOMMU_WRITE) && !PageDirty(page))
> + set_page_dirty_lock(page);
> +
> + atomic_sub(npage, compound_pincount_ptr(head));
> + if (page_ref_sub_and_test(head, npage))
> + __put_page(head);
> +
> + mod_node_page_state(page_pgdat(head), NR_FOLL_PIN_RELEASED, npage);
> + return 1;
> +}
> +
> static int follow_fault_pfn(struct vm_area_struct *vma, struct mm_struct *mm,
> unsigned long vaddr, unsigned long *pfn,
> bool write_fault)
> @@ -479,6 +519,105 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned
> long vaddr,
> return ret;
> }
>
> +static bool is_hugetlbpage(unsigned long pfn)
> +{
> + struct page *page;
> +
> + if (!pfn_valid(pfn))
> + return false;
> +
> + page = pfn_to_page(pfn);
> + /* only check for hugetlb pages */
> + if (!page || !PageHuge(page))
> + return false;
> +
> + return true;
return page && PageHuge(page);
> +}
> +
> +/*
> + * get the number of residual PAGE_SIZE-pages in a hugetlb page
> + * (including the page which pointed by this address)
> + * @address: we count residual pages from this address to the end of
> + * a hugetlb page
> + * @order: the order of the same hugetlb page
> + */
> +static long
> +hugetlb_get_residual_pages(unsigned long address, unsigned int order)
> +{
> + unsigned long hugetlb_npage;
> + unsigned long hugetlb_mask;
> +
> + if (!order)
> + return -1;
Use a real errno please.
> +
> + hugetlb_npage = _AC(1, UL) << order;
This doesn't seem an appropriate use of _AC(), 1UL << order should be
fine.
> + hugetlb_mask = (hugetlb_npage << PAGE_SHIFT) - 1;
> + address = ALIGN_DOWN(address, PAGE_SIZE);
hugetlb_mask doesn't need to be in bytes, it could be in pages
(hugetlb_npage - 1), then we could simply convert address to pfn
(address >> PAGE_SHIFT), then we avoid the shift below:
return hugetlb_npage - ((address >> PAGE_SHIFT) & (hugetlb_npage - 1));
> +
> + /*
> + * Since we count the page pointed by this address, the number of
> + * residual PAGE_SIZE-pages is greater than or equal to 1.
> + */
> + return hugetlb_npage - ((address & hugetlb_mask) >> PAGE_SHIFT);
> +}
> +
> +static unsigned int
> +hugetlb_page_get_externally_pinned_num(struct vfio_dma *dma,
> + unsigned long start,
> + unsigned long npage)
> +{
> + struct vfio_pfn *vpfn;
> + struct rb_node *node;
> + unsigned long end = start + npage - 1;
> + unsigned int num = 0;
> +
> + if (!dma || !npage)
> + return 0;
> +
> + /* If we find a page in dma->pfn_list, this page has been pinned
> externally */
> + for (node = rb_first(&dma->pfn_list); node; node = rb_next(node)) {
> + vpfn = rb_entry(node, struct vfio_pfn, node);
> + if ((vpfn->pfn >= start) && (vpfn->pfn <= end))
> +
[PATCH V2] vfio dma_map/unmap: optimized for hugetlbfs pages
In the original process of pinning/unpinning pages for VFIO-devices,
to make sure the pages are contiguous, we have to check them one by one.
As a result, dma_map/unmap could spend a long time.
Using the hugetlb pages, we can avoid this problem.
All pages in hugetlb pages are contiguous.And the hugetlb
page should not be split.So we can delete the for loops and use
some operations(such as atomic_add,page_ref_add) instead.
Signed-off-by: Ming Mao
---
drivers/vfio/vfio_iommu_type1.c | 233 +++-
1 file changed, 230 insertions(+), 3 deletions(-)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 5e556ac91..8957013c1 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -415,6 +415,46 @@ static int put_pfn(unsigned long pfn, int prot)
return 0;
}
+/*
+ * put pfns for a hugetlb page
+ * @start:the PAGE_SIZE-page we start to put,can be any page in this hugetlb
page
+ * @npage:the number of PAGE_SIZE-pages need to put
+ * @prot:IOMMU_READ/WRITE
+ */
+static int hugetlb_put_pfn(unsigned long start, unsigned int npage, int prot)
+{
+ struct page *page;
+ struct page *head;
+
+ if (!npage || !pfn_valid(start))
+ return 0;
+
+ page = pfn_to_page(start);
+ if (!page || !PageHuge(page))
+ return 0;
+ head = compound_head(page);
+ /*
+* The last page should be in this hugetlb page.
+* The number of putting pages should be equal to the number
+* of getting pages.So the hugepage pinned refcount and the normal
+* page refcount can not be smaller than npage.
+*/
+ if ((head != compound_head(pfn_to_page(start + npage - 1)))
+ || (page_ref_count(head) < npage)
+ || (compound_pincount(page) < npage))
+ return 0;
+
+ if ((prot & IOMMU_WRITE) && !PageDirty(page))
+ set_page_dirty_lock(page);
+
+ atomic_sub(npage, compound_pincount_ptr(head));
+ if (page_ref_sub_and_test(head, npage))
+ __put_page(head);
+
+ mod_node_page_state(page_pgdat(head), NR_FOLL_PIN_RELEASED, npage);
+ return 1;
+}
+
static int follow_fault_pfn(struct vm_area_struct *vma, struct mm_struct *mm,
unsigned long vaddr, unsigned long *pfn,
bool write_fault)
@@ -479,6 +519,105 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned
long vaddr,
return ret;
}
+static bool is_hugetlbpage(unsigned long pfn)
+{
+ struct page *page;
+
+ if (!pfn_valid(pfn))
+ return false;
+
+ page = pfn_to_page(pfn);
+ /* only check for hugetlb pages */
+ if (!page || !PageHuge(page))
+ return false;
+
+ return true;
+}
+
+/*
+ * get the number of residual PAGE_SIZE-pages in a hugetlb page
+ * (including the page which pointed by this address)
+ * @address: we count residual pages from this address to the end of
+ * a hugetlb page
+ * @order: the order of the same hugetlb page
+ */
+static long
+hugetlb_get_residual_pages(unsigned long address, unsigned int order)
+{
+ unsigned long hugetlb_npage;
+ unsigned long hugetlb_mask;
+
+ if (!order)
+ return -1;
+
+ hugetlb_npage = _AC(1, UL) << order;
+ hugetlb_mask = (hugetlb_npage << PAGE_SHIFT) - 1;
+ address = ALIGN_DOWN(address, PAGE_SIZE);
+
+ /*
+* Since we count the page pointed by this address, the number of
+* residual PAGE_SIZE-pages is greater than or equal to 1.
+*/
+ return hugetlb_npage - ((address & hugetlb_mask) >> PAGE_SHIFT);
+}
+
+static unsigned int
+hugetlb_page_get_externally_pinned_num(struct vfio_dma *dma,
+ unsigned long start,
+ unsigned long npage)
+{
+ struct vfio_pfn *vpfn;
+ struct rb_node *node;
+ unsigned long end = start + npage - 1;
+ unsigned int num = 0;
+
+ if (!dma || !npage)
+ return 0;
+
+ /* If we find a page in dma->pfn_list, this page has been pinned
externally */
+ for (node = rb_first(&dma->pfn_list); node; node = rb_next(node)) {
+ vpfn = rb_entry(node, struct vfio_pfn, node);
+ if ((vpfn->pfn >= start) && (vpfn->pfn <= end))
+ num++;
+ }
+
+ return num;
+}
+
+static long hugetlb_page_vaddr_get_pfn(unsigned long vaddr, long npage,
+ unsigned long pfn)
+{
+ long hugetlb_residual_npage;
+ long contiguous_npage;
+ struct page *head = compound_head(pfn_to_page(pfn));
+
+ /*
+* If pfn is valid,
+* hugetlb_residual_npage is greater than or equal to 1.
+*/
+ hugetlb_residual_npage = hugetlb_get_residual_pages(vaddr,
+ compound_order(head));
+ if (
