From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pd0-f172.google.com (mail-pd0-f172.google.com [209.85.192.172]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 6CED61A0257 for ; Mon, 28 Jul 2014 19:12:14 +1000 (EST) Received: by mail-pd0-f172.google.com with SMTP id ft15so9597274pdb.31 for ; Mon, 28 Jul 2014 02:12:12 -0700 (PDT) Message-ID: <53D613E7.2090806@ozlabs.ru> Date: Mon, 28 Jul 2014 19:12:07 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 To: Benjamin Herrenschmidt Subject: Re: [PATCH v3 04/18] vfio: powerpc: Move locked_vm accounting to a helper References: <1406191691-31441-1-git-send-email-aik@ozlabs.ru> <1406191691-31441-5-git-send-email-aik@ozlabs.ru> <1406508410.4935.24.camel@pasglop> In-Reply-To: <1406508410.4935.24.camel@pasglop> Content-Type: text/plain; charset=koi8-r Cc: Paul Mackerras , linuxppc-dev@lists.ozlabs.org, Gavin Shan List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 07/28/2014 10:46 AM, Benjamin Herrenschmidt wrote: > On Thu, 2014-07-24 at 18:47 +1000, Alexey Kardashevskiy wrote: >> Additional DMA windows support is coming and accounting will include them >> so let's move this code to a helper for reuse. > > This code looks a LOT like the one you added in the previous patch > (ie. kvmppc_account_memlimit()), though in a different place. I > wonder if we should re-arrange things so that there is really > only one version of it.. One copy is in VFIO driver, another in KVM, they meet in VFIO KVM device (which may miss) or IOMMU code (drivers/iommu/iommu.c or arch/powerpc/kernel/iommu.c) but this math has nothing to do with IOMMU. I fail to see good common place for this. > >> Signed-off-by: Alexey Kardashevskiy >> --- >> drivers/vfio/vfio_iommu_spapr_tce.c | 54 ++++++++++++++++++++++++------------- >> 1 file changed, 36 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c >> index a84788b..c8f7284 100644 >> --- a/drivers/vfio/vfio_iommu_spapr_tce.c >> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c >> @@ -47,10 +47,40 @@ struct tce_container { >> bool enabled; >> }; >> >> +/* >> + * Checks ulimit in order not to let the user space to pin all >> + * available memory for TCE tables. >> + */ >> +static long tce_iommu_account_memlimit(struct iommu_table *tbl, bool inc) >> +{ >> + unsigned long ret = 0, locked, lock_limit; >> + long npages; >> + >> + if (!current->mm) >> + return -ESRCH; /* process exited */ >> + >> + npages = (tbl->it_size << IOMMU_PAGE_SHIFT_4K) >> PAGE_SHIFT; >> + if (!inc) >> + npages = -npages; >> + >> + down_write(¤t->mm->mmap_sem); >> + locked = current->mm->locked_vm + npages; >> + lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT; >> + if (locked > lock_limit && !capable(CAP_IPC_LOCK)) { >> + pr_warn("RLIMIT_MEMLOCK (%ld) exceeded\n", >> + rlimit(RLIMIT_MEMLOCK)); >> + ret = -ENOMEM; >> + } else { >> + current->mm->locked_vm += npages; >> + } >> + up_write(¤t->mm->mmap_sem); >> + >> + return ret; >> +} >> + >> static int tce_iommu_enable(struct tce_container *container) >> { >> int ret = 0; >> - unsigned long locked, lock_limit, npages; >> struct iommu_table *tbl = container->tbl; >> >> if (!container->tbl) >> @@ -80,20 +110,11 @@ static int tce_iommu_enable(struct tce_container *container) >> * that would effectively kill the guest at random points, much better >> * enforcing the limit based on the max that the guest can map. >> */ >> - down_write(¤t->mm->mmap_sem); >> - npages = (tbl->it_size << IOMMU_PAGE_SHIFT_4K) >> PAGE_SHIFT; >> - locked = current->mm->locked_vm + npages; >> - lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT; >> - if (locked > lock_limit && !capable(CAP_IPC_LOCK)) { >> - pr_warn("RLIMIT_MEMLOCK (%ld) exceeded\n", >> - rlimit(RLIMIT_MEMLOCK)); >> - ret = -ENOMEM; >> - } else { >> + ret = tce_iommu_account_memlimit(tbl, true); >> + if (ret) >> + return ret; >> >> - current->mm->locked_vm += npages; >> - container->enabled = true; >> - } >> - up_write(¤t->mm->mmap_sem); >> + container->enabled = true; >> >> return ret; >> } >> @@ -108,10 +129,7 @@ static void tce_iommu_disable(struct tce_container *container) >> if (!container->tbl || !current->mm) >> return; >> >> - down_write(¤t->mm->mmap_sem); >> - current->mm->locked_vm -= (container->tbl->it_size << >> - IOMMU_PAGE_SHIFT_4K) >> PAGE_SHIFT; >> - up_write(¤t->mm->mmap_sem); >> + tce_iommu_account_memlimit(container->tbl, false); >> } >> >> static void *tce_iommu_open(unsigned long arg) > > -- Alexey