| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <deed9104-888b-96a0-ec69-23d0d4e222b0@nvidia.com>
Date: Wed, 21 Dec 2016 00:07:18 +0530
From: Kirti Wankhede <kwankhede@...dia.com>
To: Alex Williamson <alex.williamson@...hat.com>
CC: <cjia@...dia.com>, <linux-kernel@...r.kernel.org>,
<kvm@...r.kernel.org>
Subject: Re: [PATCH v2] vfio/type1: Restore mapping performance with mdev
support
On 12/20/2016 3:43 AM, Alex Williamson wrote:
> As part of the mdev support, type1 now gets a task reference per
> vfio_dma and uses that to get an mm reference for the task while
> working on accounting. That's correct, but it's not fast. For some
> paths, like vfio_pin_pages_remote(), we know we're only called from
> user context, so we can restore the lighter weight calls. In other
> cases, we're effectively already testing whether we're in the stored
> task context elsewhere, extend this vfio_lock_acct() as well.
>
> Signed-off-by: Alex Williamson <alex.williamson@...hat.com>
> ---
>
> v2: Use (mm == current->mm) test in vfio_lock_acct() as well rather
> than passing around is_current. It doesn't make sense to keep it
> in vaddr_get_pfn() and not use it elsewhere.
>
Thanks Alex. This change looks good to me.
Reviewed by: Kirti Wankhede <kwankhede@...dia.com>
Thanks,
Kirti
> drivers/vfio/vfio_iommu_type1.c | 98 ++++++++++++++++++++-------------------
> 1 file changed, 51 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 9815e45..7154987 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -268,28 +268,38 @@ static void vfio_lock_acct(struct task_struct *task, long npage)
> {
> struct vwork *vwork;
> struct mm_struct *mm;
> + bool is_current;
>
> if (!npage)
> return;
>
> - mm = get_task_mm(task);
> + is_current = (task->mm == current->mm);
> +
> + mm = is_current ? task->mm : get_task_mm(task);
> if (!mm)
> - return; /* process exited or nothing to do */
> + return; /* process exited */
>
> if (down_write_trylock(&mm->mmap_sem)) {
> mm->locked_vm += npage;
> up_write(&mm->mmap_sem);
> - mmput(mm);
> + if (!is_current)
> + mmput(mm);
> return;
> }
>
> + if (is_current) {
> + mm = get_task_mm(task);
> + if (!mm)
> + return;
> + }
> +
> /*
> * Couldn't get mmap_sem lock, so must setup to update
> * mm->locked_vm later. If locked_vm were atomic, we
> * wouldn't need this silliness
> */
> vwork = kmalloc(sizeof(struct vwork), GFP_KERNEL);
> - if (!vwork) {
> + if (WARN_ON(!vwork)) {
> mmput(mm);
> return;
> }
> @@ -393,77 +403,71 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned long vaddr,
> static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
> long npage, unsigned long *pfn_base)
> {
> - unsigned long limit;
> - bool lock_cap = ns_capable(task_active_pid_ns(dma->task)->user_ns,
> - CAP_IPC_LOCK);
> - struct mm_struct *mm;
> - long ret, i = 0, lock_acct = 0;
> + unsigned long limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
> + bool lock_cap = capable(CAP_IPC_LOCK);
> + long ret, pinned = 0, lock_acct = 0;
> bool rsvd;
> dma_addr_t iova = vaddr - dma->vaddr + dma->iova;
>
> - mm = get_task_mm(dma->task);
> - if (!mm)
> + /* This code path is only user initiated */
> + if (!current->mm)
> return -ENODEV;
>
> - ret = vaddr_get_pfn(mm, vaddr, dma->prot, pfn_base);
> + ret = vaddr_get_pfn(current->mm, vaddr, dma->prot, pfn_base);
> if (ret)
> - goto pin_pg_remote_exit;
> + return ret;
>
> + pinned++;
> rsvd = is_invalid_reserved_pfn(*pfn_base);
> - limit = task_rlimit(dma->task, RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>
> /*
> * Reserved pages aren't counted against the user, externally pinned
> * pages are already counted against the user.
> */
> if (!rsvd && !vfio_find_vpfn(dma, iova)) {
> - if (!lock_cap && mm->locked_vm + 1 > limit) {
> + if (!lock_cap && current->mm->locked_vm + 1 > limit) {
> put_pfn(*pfn_base, dma->prot);
> pr_warn("%s: RLIMIT_MEMLOCK (%ld) exceeded\n", __func__,
> limit << PAGE_SHIFT);
> - ret = -ENOMEM;
> - goto pin_pg_remote_exit;
> + return -ENOMEM;
> }
> lock_acct++;
> }
>
> - i++;
> - if (likely(!disable_hugepages)) {
> - /* Lock all the consecutive pages from pfn_base */
> - for (vaddr += PAGE_SIZE, iova += PAGE_SIZE; i < npage;
> - i++, vaddr += PAGE_SIZE, iova += PAGE_SIZE) {
> - unsigned long pfn = 0;
> + if (unlikely(disable_hugepages))
> + goto out;
>
> - ret = vaddr_get_pfn(mm, vaddr, dma->prot, &pfn);
> - if (ret)
> - break;
> + /* Lock all the consecutive pages from pfn_base */
> + for (vaddr += PAGE_SIZE, iova += PAGE_SIZE; pinned < npage;
> + pinned++, vaddr += PAGE_SIZE, iova += PAGE_SIZE) {
> + unsigned long pfn = 0;
>
> - if (pfn != *pfn_base + i ||
> - rsvd != is_invalid_reserved_pfn(pfn)) {
> + ret = vaddr_get_pfn(current->mm, vaddr, dma->prot, &pfn);
> + if (ret)
> + break;
> +
> + if (pfn != *pfn_base + pinned ||
> + rsvd != is_invalid_reserved_pfn(pfn)) {
> + put_pfn(pfn, dma->prot);
> + break;
> + }
> +
> + if (!rsvd && !vfio_find_vpfn(dma, iova)) {
> + if (!lock_cap &&
> + current->mm->locked_vm + lock_acct + 1 > limit) {
> put_pfn(pfn, dma->prot);
> + pr_warn("%s: RLIMIT_MEMLOCK (%ld) exceeded\n",
> + __func__, limit << PAGE_SHIFT);
> break;
> }
> -
> - if (!rsvd && !vfio_find_vpfn(dma, iova)) {
> - if (!lock_cap &&
> - mm->locked_vm + lock_acct + 1 > limit) {
> - put_pfn(pfn, dma->prot);
> - pr_warn("%s: RLIMIT_MEMLOCK (%ld) "
> - "exceeded\n", __func__,
> - limit << PAGE_SHIFT);
> - break;
> - }
> - lock_acct++;
> - }
> + lock_acct++;
> }
> }
>
> - vfio_lock_acct(dma->task, lock_acct);
> - ret = i;
> +out:
> + vfio_lock_acct(current, lock_acct);
>
> -pin_pg_remote_exit:
> - mmput(mm);
> - return ret;
> + return pinned;
> }
>
> static long vfio_unpin_pages_remote(struct vfio_dma *dma, dma_addr_t iova,
> @@ -473,10 +477,10 @@ static long vfio_unpin_pages_remote(struct vfio_dma *dma, dma_addr_t iova,
> long unlocked = 0, locked = 0;
> long i;
>
> - for (i = 0; i < npage; i++) {
> + for (i = 0; i < npage; i++, iova += PAGE_SIZE) {
> if (put_pfn(pfn++, dma->prot)) {
> unlocked++;
> - if (vfio_find_vpfn(dma, iova + (i << PAGE_SHIFT)))
> + if (vfio_find_vpfn(dma, iova))
> locked++;
> }
> }
>
Powered by blists - more mailing lists