From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F38E5C433DB for ; Sun, 7 Feb 2021 21:35:40 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id AB8D364E2A for ; Sun, 7 Feb 2021 21:35:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org AB8D364E2A Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=3AmQ++dUACFc0ejYqpw+ITBoW38s+gvwBX3YzrDsxSk=; b=pES3Y3bN/iAM/PoJLFdoZxNxu L6oLom33C9rBwBSxcxgkZYqYK9B07oCbaW4/bCOgsKTPAE2Yy3ME3EQaMEy0vDS+WVYjdsy3UHPTk xEMsKPdM+wNWzWMP9AI8+cFpQaelDE45Qtb/peVDlTwY7OjREFn1dRBJ5jJd4+tuY9PrDI6mR97Ul 1YI2sj9Xnit7v7p1M30WMXc4HbHCWIScIV8HOMbIleeTbHJJI0CTyMANu3TplHEk2+vcJ8jUkRvZV oFjDVzHTmckWjxsGdfK3ZotwSU02lYyBXQFsUNYQe7fS7l7BxMeZKhUJA+X5+b86HTyoLWrZAUQcR a1SZLcSlg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1l8rhH-00021X-RJ; Sun, 07 Feb 2021 21:34:23 +0000 Received: from casper.infradead.org ([2001:8b0:10b:1236::1]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1l8rhF-000214-Hf for linux-arm-kernel@merlin.infradead.org; Sun, 07 Feb 2021 21:34:21 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=Y+S/LX44a9NNx3dGKU3W10H4coPDrc53Kmvv4eVUILs=; b=jk3k6ovvjVTWnfOKs4KKL28gHs E3infYdAisvXmp2e28FqZSi8PowAudg/J4HUU+RQZkOd4jyrho7b1D2lBJL/Zi0gyDGl6Zhkz6+CX 61rDbpkJo3h7u9i/iGE0BIu1tHx5KkSe0b78+GMS62RmlOZcEpWcN6GRwbMQI2HBmJ9ukrRvmeHN8 T92alHBcH65ZJXdEW+4Ldz5OtgSFwNgd11YIKUjsk39oW/3MyqppThjjUZmLKNFhLTFE34L8rgcYl 1AScMZodOOkOZCxEDymnpEc3MrtSAHSFauRuDhn3zomqsOxWHJFoMLUqZ1wtHGze+DL2xmOnj+hr4 7Qc/yQog==; Received: from willy by casper.infradead.org with local (Exim 4.94 #2 (Red Hat Linux)) id 1l8rh3-0059pm-0q; Sun, 07 Feb 2021 21:34:11 +0000 Date: Sun, 7 Feb 2021 21:34:09 +0000 From: Matthew Wilcox To: Zhou Wang Subject: Re: [RFC PATCH v3 1/2] mempinfd: Add new syscall to provide memory pin Message-ID: <20210207213409.GL308988@casper.infradead.org> References: <1612685884-19514-1-git-send-email-wangzhou1@hisilicon.com> <1612685884-19514-2-git-send-email-wangzhou1@hisilicon.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1612685884-19514-2-git-send-email-wangzhou1@hisilicon.com> X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: song.bao.hua@hisilicon.com, jean-philippe@linaro.org, kevin.tian@intel.com, Sihang Chen , jgg@ziepe.ca, linux-api@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, iommu@lists.linux-foundation.org, Alexander Viro , eric.auger@redhat.com, gregkh@linuxfoundation.org, zhangfei.gao@linaro.org, Andrew Morton , liguozhu@hisilicon.com, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Sun, Feb 07, 2021 at 04:18:03PM +0800, Zhou Wang wrote: > SVA(share virtual address) offers a way for device to share process virtual > address space safely, which makes more convenient for user space device > driver coding. However, IO page faults may happen when doing DMA > operations. As the latency of IO page fault is relatively big, DMA > performance will be affected severely when there are IO page faults. > >From a long term view, DMA performance will be not stable. > > In high-performance I/O cases, accelerators might want to perform > I/O on a memory without IO page faults which can result in dramatically > increased latency. Current memory related APIs could not achieve this > requirement, e.g. mlock can only avoid memory to swap to backup device, > page migration can still trigger IO page fault. Well ... we have two requirements. The application wants to not take page faults. The system wants to move the application to a different NUMA node in order to optimise overall performance. Why should the application's desires take precedence over the kernel's desires? And why should it be done this way rather than by the sysadmin using numactl to lock the application to a particular node? > +struct mem_pin_container { > + struct xarray array; > + struct mutex lock; > +}; I don't understand what the lock actually protects. > +struct pin_pages { > + unsigned long first; > + unsigned long nr_pages; > + struct page **pages; > +}; I don't think you need 'first', and I think you can embed the pages array into this struct, removing one allocation. > + xa_for_each(&priv->array, idx, p) { > + unpin_user_pages(p->pages, p->nr_pages); > + xa_erase(&priv->array, p->first); > + vfree(p->pages); > + kfree(p); > + } > + > + mutex_destroy(&priv->lock); > + xa_destroy(&priv->array); If you just called xa_erase() on every element of the array, you don't need to call xa_destroy(). > + if (!can_do_mlock()) > + return -EPERM; You check for can_do_mlock(), but you don't account the pages to this rlimit. > + first = (addr->addr & PAGE_MASK) >> PAGE_SHIFT; You don't need to mask off the bits, the shift will remove them. > + last = ((addr->addr + addr->size - 1) & PAGE_MASK) >> PAGE_SHIFT; DIV_ROUND_UP()? > + pages = vmalloc(nr_pages * sizeof(struct page *)); kvmalloc(). vmalloc() always allocates at least a page, so we want to use kmalloc if the size is small. Also, use array_size() -- I know this can't overflow, but let's be clear > + ret = pin_user_pages_fast(addr->addr & PAGE_MASK, nr_pages, > + flags | FOLL_LONGTERM, pages); > + if (ret != nr_pages) { > + pr_err("mempinfd: Failed to pin page\n"); No. You mustn't allow the user to be able to generate messages to syslog, just by passing garbage to a syscall. > + ret = xa_insert(&priv->array, p->first, p, GFP_KERNEL); > + if (ret) > + goto unpin_pages; Hmm. So we can't pin two ranges which start at the same address, but we can pin two overlapping ranges. Is that OK? _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel