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=-13.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham 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 C9FBFC4361B for ; Thu, 10 Dec 2020 19:17:03 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id 221AE23D9A for ; Thu, 10 Dec 2020 19:17:02 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 221AE23D9A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvmarm-bounces@lists.cs.columbia.edu Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 4A1644B1CD; Thu, 10 Dec 2020 14:17:02 -0500 (EST) X-Virus-Scanned: at lists.cs.columbia.edu Authentication-Results: mm01.cs.columbia.edu (amavisd-new); dkim=softfail (fail, message has been altered) header.i=@redhat.com Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Wi-W6+FrZp47; Thu, 10 Dec 2020 14:17:01 -0500 (EST) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 136404B1BA; Thu, 10 Dec 2020 14:17:01 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 4A63F4B1A2 for ; Thu, 10 Dec 2020 14:17:00 -0500 (EST) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id oIYqt7CqanfU for ; Thu, 10 Dec 2020 14:16:59 -0500 (EST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mm01.cs.columbia.edu (Postfix) with ESMTP id F2E8A4B1A0 for ; Thu, 10 Dec 2020 14:16:58 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1607627818; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5kmp9gy5nxGFdrl4LVG8Ly3qZOFyWyiuhjDzmpydZu0=; b=ckpziQfxpdcKMa6couIMEWdCA3gmujRCVrN6Fx6hMsEIjCporHE48ICAn4mHLcdCDtfwh4 1cxkFf9o21rw3gJylVLUC5NS5E6cSl9RrdltYe5My4Pq3sACVoDLnd6VwogScSEBGN1o90 X1y7XvYznCvWyYGc5kCvKDzG93rRl3Y= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-231-RbUqo1cuMGOk8bK4pmTycw-1; Thu, 10 Dec 2020 14:16:54 -0500 X-MC-Unique: RbUqo1cuMGOk8bK4pmTycw-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id CF75710054FF; Thu, 10 Dec 2020 19:16:50 +0000 (UTC) Received: from omen.home (ovpn-112-10.phx2.redhat.com [10.3.112.10]) by smtp.corp.redhat.com (Postfix) with ESMTP id B078B5D6D3; Thu, 10 Dec 2020 19:16:47 +0000 (UTC) Date: Thu, 10 Dec 2020 12:16:46 -0700 From: Alex Williamson To: Keqian Zhu Subject: Re: [PATCH 1/7] vfio: iommu_type1: Clear added dirty bit when unwind pin Message-ID: <20201210121646.24fb3cd8@omen.home> In-Reply-To: <20201210073425.25960-2-zhukeqian1@huawei.com> References: <20201210073425.25960-1-zhukeqian1@huawei.com> <20201210073425.25960-2-zhukeqian1@huawei.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Cc: Andrew Morton , kvm@vger.kernel.org, Marc Zyngier , Joerg Roedel , Cornelia Huck , linux-kernel@vger.kernel.org, Sean Christopherson , Alexios Zavras , iommu@lists.linux-foundation.org, Mark Brown , Catalin Marinas , Thomas Gleixner , Will Deacon , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, Robin Murphy X-BeenThere: kvmarm@lists.cs.columbia.edu X-Mailman-Version: 2.1.14 Precedence: list List-Id: Where KVM/ARM decisions are made List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu On Thu, 10 Dec 2020 15:34:19 +0800 Keqian Zhu wrote: > Currently we do not clear added dirty bit of bitmap when unwind > pin, so if pin failed at halfway, we set unnecessary dirty bit > in bitmap. Clearing added dirty bit when unwind pin, userspace > will see less dirty page, which can save much time to handle them. > > Note that we should distinguish the bits added by pin and the bits > already set before pin, so introduce bitmap_added to record this. > > Signed-off-by: Keqian Zhu > --- > drivers/vfio/vfio_iommu_type1.c | 33 ++++++++++++++++++++++----------- > 1 file changed, 22 insertions(+), 11 deletions(-) > > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > index 67e827638995..f129d24a6ec3 100644 > --- a/drivers/vfio/vfio_iommu_type1.c > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -637,7 +637,11 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > struct vfio_iommu *iommu = iommu_data; > struct vfio_group *group; > int i, j, ret; > + unsigned long pgshift = __ffs(iommu->pgsize_bitmap); > unsigned long remote_vaddr; > + unsigned long bitmap_offset; > + unsigned long *bitmap_added; > + dma_addr_t iova; > struct vfio_dma *dma; > bool do_accounting; > > @@ -650,6 +654,12 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > > mutex_lock(&iommu->lock); > > + bitmap_added = bitmap_zalloc(npage, GFP_KERNEL); > + if (!bitmap_added) { > + ret = -ENOMEM; > + goto pin_done; > + } This is allocated regardless of whether dirty tracking is enabled, so this adds overhead to the common case in order to reduce user overhead (not correctness) in the rare condition that dirty tracking is enabled, and the even rarer condition that there's a fault during that case. This is not a good trade-off. Thanks, Alex > + > /* Fail if notifier list is empty */ > if (!iommu->notifier.head) { > ret = -EINVAL; > @@ -664,7 +674,6 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > do_accounting = !IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu); > > for (i = 0; i < npage; i++) { > - dma_addr_t iova; > struct vfio_pfn *vpfn; > > iova = user_pfn[i] << PAGE_SHIFT; > @@ -699,14 +708,10 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > } > > if (iommu->dirty_page_tracking) { > - unsigned long pgshift = __ffs(iommu->pgsize_bitmap); > - > - /* > - * Bitmap populated with the smallest supported page > - * size > - */ > - bitmap_set(dma->bitmap, > - (iova - dma->iova) >> pgshift, 1); > + /* Populated with the smallest supported page size */ > + bitmap_offset = (iova - dma->iova) >> pgshift; > + if (!test_and_set_bit(bitmap_offset, dma->bitmap)) > + set_bit(i, bitmap_added); > } > } > ret = i; > @@ -722,14 +727,20 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > pin_unwind: > phys_pfn[i] = 0; > for (j = 0; j < i; j++) { > - dma_addr_t iova; > - > iova = user_pfn[j] << PAGE_SHIFT; > dma = vfio_find_dma(iommu, iova, PAGE_SIZE); > vfio_unpin_page_external(dma, iova, do_accounting); > phys_pfn[j] = 0; > + > + if (test_bit(j, bitmap_added)) { > + bitmap_offset = (iova - dma->iova) >> pgshift; > + clear_bit(bitmap_offset, dma->bitmap); > + } > } > pin_done: > + if (bitmap_added) > + bitmap_free(bitmap_added); > + > mutex_unlock(&iommu->lock); > return ret; > } _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm 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=-13.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable 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 7EAFAC4361B for ; Thu, 10 Dec 2020 19:17:08 +0000 (UTC) Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (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 F342623D9A for ; Thu, 10 Dec 2020 19:17:07 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F342623D9A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=iommu-bounces@lists.linux-foundation.org Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 980EE873FD; Thu, 10 Dec 2020 19:17:07 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id eR4IqpL+ovaj; Thu, 10 Dec 2020 19:17:05 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id B8A19873A0; Thu, 10 Dec 2020 19:17:05 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id A0933C0FA7; Thu, 10 Dec 2020 19:17:05 +0000 (UTC) Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 898ADC013B for ; Thu, 10 Dec 2020 19:17:04 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 6F5DD876CD for ; Thu, 10 Dec 2020 19:17:04 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 6tznGkyloSSy for ; Thu, 10 Dec 2020 19:17:03 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by fraxinus.osuosl.org (Postfix) with ESMTPS id 0450C876BA for ; Thu, 10 Dec 2020 19:17:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1607627821; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5kmp9gy5nxGFdrl4LVG8Ly3qZOFyWyiuhjDzmpydZu0=; b=axKyn+LJrMqfY454zQlSy3IVHE+xvRB7PdwYNQ6WIwai40lz7WTUfpryFWKenycs86ye1Z G2KMaddk3dK7CqkzBWEDK3jd1yOIiAiw63dSP0oSlQKHRBVX+xFRK8m4LEBT0ciNKjwe71 +g5bD/la9FHH7WMGK1UEoL64dMzxdCg= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-231-RbUqo1cuMGOk8bK4pmTycw-1; Thu, 10 Dec 2020 14:16:54 -0500 X-MC-Unique: RbUqo1cuMGOk8bK4pmTycw-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id CF75710054FF; Thu, 10 Dec 2020 19:16:50 +0000 (UTC) Received: from omen.home (ovpn-112-10.phx2.redhat.com [10.3.112.10]) by smtp.corp.redhat.com (Postfix) with ESMTP id B078B5D6D3; Thu, 10 Dec 2020 19:16:47 +0000 (UTC) Date: Thu, 10 Dec 2020 12:16:46 -0700 From: Alex Williamson To: Keqian Zhu Subject: Re: [PATCH 1/7] vfio: iommu_type1: Clear added dirty bit when unwind pin Message-ID: <20201210121646.24fb3cd8@omen.home> In-Reply-To: <20201210073425.25960-2-zhukeqian1@huawei.com> References: <20201210073425.25960-1-zhukeqian1@huawei.com> <20201210073425.25960-2-zhukeqian1@huawei.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Cc: jiangkunkun@huawei.com, Andrew Morton , kvm@vger.kernel.org, Suzuki K Poulose , Marc Zyngier , Cornelia Huck , linux-kernel@vger.kernel.org, Sean Christopherson , Alexios Zavras , iommu@lists.linux-foundation.org, Mark Brown , James Morse , Julien Thierry , Catalin Marinas , wanghaibin.wang@huawei.com, Thomas Gleixner , Will Deacon , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, Robin Murphy X-BeenThere: iommu@lists.linux-foundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Development issues for Linux IOMMU support List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: iommu-bounces@lists.linux-foundation.org Sender: "iommu" On Thu, 10 Dec 2020 15:34:19 +0800 Keqian Zhu wrote: > Currently we do not clear added dirty bit of bitmap when unwind > pin, so if pin failed at halfway, we set unnecessary dirty bit > in bitmap. Clearing added dirty bit when unwind pin, userspace > will see less dirty page, which can save much time to handle them. > > Note that we should distinguish the bits added by pin and the bits > already set before pin, so introduce bitmap_added to record this. > > Signed-off-by: Keqian Zhu > --- > drivers/vfio/vfio_iommu_type1.c | 33 ++++++++++++++++++++++----------- > 1 file changed, 22 insertions(+), 11 deletions(-) > > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > index 67e827638995..f129d24a6ec3 100644 > --- a/drivers/vfio/vfio_iommu_type1.c > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -637,7 +637,11 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > struct vfio_iommu *iommu = iommu_data; > struct vfio_group *group; > int i, j, ret; > + unsigned long pgshift = __ffs(iommu->pgsize_bitmap); > unsigned long remote_vaddr; > + unsigned long bitmap_offset; > + unsigned long *bitmap_added; > + dma_addr_t iova; > struct vfio_dma *dma; > bool do_accounting; > > @@ -650,6 +654,12 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > > mutex_lock(&iommu->lock); > > + bitmap_added = bitmap_zalloc(npage, GFP_KERNEL); > + if (!bitmap_added) { > + ret = -ENOMEM; > + goto pin_done; > + } This is allocated regardless of whether dirty tracking is enabled, so this adds overhead to the common case in order to reduce user overhead (not correctness) in the rare condition that dirty tracking is enabled, and the even rarer condition that there's a fault during that case. This is not a good trade-off. Thanks, Alex > + > /* Fail if notifier list is empty */ > if (!iommu->notifier.head) { > ret = -EINVAL; > @@ -664,7 +674,6 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > do_accounting = !IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu); > > for (i = 0; i < npage; i++) { > - dma_addr_t iova; > struct vfio_pfn *vpfn; > > iova = user_pfn[i] << PAGE_SHIFT; > @@ -699,14 +708,10 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > } > > if (iommu->dirty_page_tracking) { > - unsigned long pgshift = __ffs(iommu->pgsize_bitmap); > - > - /* > - * Bitmap populated with the smallest supported page > - * size > - */ > - bitmap_set(dma->bitmap, > - (iova - dma->iova) >> pgshift, 1); > + /* Populated with the smallest supported page size */ > + bitmap_offset = (iova - dma->iova) >> pgshift; > + if (!test_and_set_bit(bitmap_offset, dma->bitmap)) > + set_bit(i, bitmap_added); > } > } > ret = i; > @@ -722,14 +727,20 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > pin_unwind: > phys_pfn[i] = 0; > for (j = 0; j < i; j++) { > - dma_addr_t iova; > - > iova = user_pfn[j] << PAGE_SHIFT; > dma = vfio_find_dma(iommu, iova, PAGE_SIZE); > vfio_unpin_page_external(dma, iova, do_accounting); > phys_pfn[j] = 0; > + > + if (test_bit(j, bitmap_added)) { > + bitmap_offset = (iova - dma->iova) >> pgshift; > + clear_bit(bitmap_offset, dma->bitmap); > + } > } > pin_done: > + if (bitmap_added) > + bitmap_free(bitmap_added); > + > mutex_unlock(&iommu->lock); > return ret; > } _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu 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=-13.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable 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 9A3CEC433FE for ; Thu, 10 Dec 2020 19:18:32 +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 460E223B26 for ; Thu, 10 Dec 2020 19:18:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 460E223B26 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com 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:MIME-Version:References:In-Reply-To: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=X/CT5HsY5h1xYDgeLqfE60gs3IC6BGiu5wbAjQOAL8A=; b=OSx781327CXy6bvJYSVAKB0Js hyEbSxg+3KGPl60jB8VQ6cVHmER2b0W+0Ra2n5UoX/un98oaRf3jm454OqtGml9R3bZImZDDoytLd 9huIaN7sRVPew1H3lSDf6WEdiPgMy11QD5zA1qPKkJWkfbYgsdnmhVRZxKfJ9FQ/lYjZy/8NQMzDv 1Xz9WqpPKc4kGnJtph7bAZ1w35v+FMAvp30RUtSzyOIkEfeDG9V3r3Sfo3x4ZJ32PcjX/CeVvo1f5 qUAd5EHeSmE87AOWwFXw3sn6wjSIJhK7azswRpYBAAAsUrb4zMi47Oce6KNVFIwfwTzqwo6BCjXc+ nGVb1adsQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1knRR9-0006LU-CP; Thu, 10 Dec 2020 19:17:11 +0000 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1knRR5-0006Kn-6Z for linux-arm-kernel@lists.infradead.org; Thu, 10 Dec 2020 19:17:09 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1607627823; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5kmp9gy5nxGFdrl4LVG8Ly3qZOFyWyiuhjDzmpydZu0=; b=codGFKds9+CYLxJT2zeoETzlBB4T1dwlfHQlDzp/9W+37wXakXUS5H/JVS8FWZo989LzzL uIw3aVc53BIY7Nld9yRtD+Tz1ola8wXeQFj/3rHdR8ewkEetIA5ZBESXLS82WI2jQImuWW CgN669mw0oA9mAl4kWAsD4DDCSrH28Y= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-231-RbUqo1cuMGOk8bK4pmTycw-1; Thu, 10 Dec 2020 14:16:54 -0500 X-MC-Unique: RbUqo1cuMGOk8bK4pmTycw-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id CF75710054FF; Thu, 10 Dec 2020 19:16:50 +0000 (UTC) Received: from omen.home (ovpn-112-10.phx2.redhat.com [10.3.112.10]) by smtp.corp.redhat.com (Postfix) with ESMTP id B078B5D6D3; Thu, 10 Dec 2020 19:16:47 +0000 (UTC) Date: Thu, 10 Dec 2020 12:16:46 -0700 From: Alex Williamson To: Keqian Zhu Subject: Re: [PATCH 1/7] vfio: iommu_type1: Clear added dirty bit when unwind pin Message-ID: <20201210121646.24fb3cd8@omen.home> In-Reply-To: <20201210073425.25960-2-zhukeqian1@huawei.com> References: <20201210073425.25960-1-zhukeqian1@huawei.com> <20201210073425.25960-2-zhukeqian1@huawei.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201210_141707_425782_02A49FF0 X-CRM114-Status: GOOD ( 25.78 ) 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: jiangkunkun@huawei.com, Andrew Morton , kvm@vger.kernel.org, Suzuki K Poulose , Marc Zyngier , Joerg Roedel , Cornelia Huck , linux-kernel@vger.kernel.org, Sean Christopherson , Alexios Zavras , iommu@lists.linux-foundation.org, Mark Brown , James Morse , Julien Thierry , Catalin Marinas , wanghaibin.wang@huawei.com, Thomas Gleixner , Will Deacon , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, Robin Murphy 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 Thu, 10 Dec 2020 15:34:19 +0800 Keqian Zhu wrote: > Currently we do not clear added dirty bit of bitmap when unwind > pin, so if pin failed at halfway, we set unnecessary dirty bit > in bitmap. Clearing added dirty bit when unwind pin, userspace > will see less dirty page, which can save much time to handle them. > > Note that we should distinguish the bits added by pin and the bits > already set before pin, so introduce bitmap_added to record this. > > Signed-off-by: Keqian Zhu > --- > drivers/vfio/vfio_iommu_type1.c | 33 ++++++++++++++++++++++----------- > 1 file changed, 22 insertions(+), 11 deletions(-) > > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > index 67e827638995..f129d24a6ec3 100644 > --- a/drivers/vfio/vfio_iommu_type1.c > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -637,7 +637,11 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > struct vfio_iommu *iommu = iommu_data; > struct vfio_group *group; > int i, j, ret; > + unsigned long pgshift = __ffs(iommu->pgsize_bitmap); > unsigned long remote_vaddr; > + unsigned long bitmap_offset; > + unsigned long *bitmap_added; > + dma_addr_t iova; > struct vfio_dma *dma; > bool do_accounting; > > @@ -650,6 +654,12 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > > mutex_lock(&iommu->lock); > > + bitmap_added = bitmap_zalloc(npage, GFP_KERNEL); > + if (!bitmap_added) { > + ret = -ENOMEM; > + goto pin_done; > + } This is allocated regardless of whether dirty tracking is enabled, so this adds overhead to the common case in order to reduce user overhead (not correctness) in the rare condition that dirty tracking is enabled, and the even rarer condition that there's a fault during that case. This is not a good trade-off. Thanks, Alex > + > /* Fail if notifier list is empty */ > if (!iommu->notifier.head) { > ret = -EINVAL; > @@ -664,7 +674,6 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > do_accounting = !IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu); > > for (i = 0; i < npage; i++) { > - dma_addr_t iova; > struct vfio_pfn *vpfn; > > iova = user_pfn[i] << PAGE_SHIFT; > @@ -699,14 +708,10 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > } > > if (iommu->dirty_page_tracking) { > - unsigned long pgshift = __ffs(iommu->pgsize_bitmap); > - > - /* > - * Bitmap populated with the smallest supported page > - * size > - */ > - bitmap_set(dma->bitmap, > - (iova - dma->iova) >> pgshift, 1); > + /* Populated with the smallest supported page size */ > + bitmap_offset = (iova - dma->iova) >> pgshift; > + if (!test_and_set_bit(bitmap_offset, dma->bitmap)) > + set_bit(i, bitmap_added); > } > } > ret = i; > @@ -722,14 +727,20 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > pin_unwind: > phys_pfn[i] = 0; > for (j = 0; j < i; j++) { > - dma_addr_t iova; > - > iova = user_pfn[j] << PAGE_SHIFT; > dma = vfio_find_dma(iommu, iova, PAGE_SIZE); > vfio_unpin_page_external(dma, iova, do_accounting); > phys_pfn[j] = 0; > + > + if (test_bit(j, bitmap_added)) { > + bitmap_offset = (iova - dma->iova) >> pgshift; > + clear_bit(bitmap_offset, dma->bitmap); > + } > } > pin_done: > + if (bitmap_added) > + bitmap_free(bitmap_added); > + > mutex_unlock(&iommu->lock); > return ret; > } _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel 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=-15.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=unavailable 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 B780EC4361B for ; Thu, 10 Dec 2020 19:19:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 71B0A23D9A for ; Thu, 10 Dec 2020 19:19:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2393357AbgLJTSf (ORCPT ); Thu, 10 Dec 2020 14:18:35 -0500 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:34764 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2392857AbgLJTSb (ORCPT ); Thu, 10 Dec 2020 14:18:31 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1607627818; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5kmp9gy5nxGFdrl4LVG8Ly3qZOFyWyiuhjDzmpydZu0=; b=ckpziQfxpdcKMa6couIMEWdCA3gmujRCVrN6Fx6hMsEIjCporHE48ICAn4mHLcdCDtfwh4 1cxkFf9o21rw3gJylVLUC5NS5E6cSl9RrdltYe5My4Pq3sACVoDLnd6VwogScSEBGN1o90 X1y7XvYznCvWyYGc5kCvKDzG93rRl3Y= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-231-RbUqo1cuMGOk8bK4pmTycw-1; Thu, 10 Dec 2020 14:16:54 -0500 X-MC-Unique: RbUqo1cuMGOk8bK4pmTycw-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id CF75710054FF; Thu, 10 Dec 2020 19:16:50 +0000 (UTC) Received: from omen.home (ovpn-112-10.phx2.redhat.com [10.3.112.10]) by smtp.corp.redhat.com (Postfix) with ESMTP id B078B5D6D3; Thu, 10 Dec 2020 19:16:47 +0000 (UTC) Date: Thu, 10 Dec 2020 12:16:46 -0700 From: Alex Williamson To: Keqian Zhu Cc: , , , , , Cornelia Huck , Marc Zyngier , Will Deacon , Robin Murphy , Joerg Roedel , Catalin Marinas , James Morse , Suzuki K Poulose , Sean Christopherson , Julien Thierry , Mark Brown , "Thomas Gleixner" , Andrew Morton , Alexios Zavras , , Subject: Re: [PATCH 1/7] vfio: iommu_type1: Clear added dirty bit when unwind pin Message-ID: <20201210121646.24fb3cd8@omen.home> In-Reply-To: <20201210073425.25960-2-zhukeqian1@huawei.com> References: <20201210073425.25960-1-zhukeqian1@huawei.com> <20201210073425.25960-2-zhukeqian1@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Thu, 10 Dec 2020 15:34:19 +0800 Keqian Zhu wrote: > Currently we do not clear added dirty bit of bitmap when unwind > pin, so if pin failed at halfway, we set unnecessary dirty bit > in bitmap. Clearing added dirty bit when unwind pin, userspace > will see less dirty page, which can save much time to handle them. > > Note that we should distinguish the bits added by pin and the bits > already set before pin, so introduce bitmap_added to record this. > > Signed-off-by: Keqian Zhu > --- > drivers/vfio/vfio_iommu_type1.c | 33 ++++++++++++++++++++++----------- > 1 file changed, 22 insertions(+), 11 deletions(-) > > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > index 67e827638995..f129d24a6ec3 100644 > --- a/drivers/vfio/vfio_iommu_type1.c > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -637,7 +637,11 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > struct vfio_iommu *iommu = iommu_data; > struct vfio_group *group; > int i, j, ret; > + unsigned long pgshift = __ffs(iommu->pgsize_bitmap); > unsigned long remote_vaddr; > + unsigned long bitmap_offset; > + unsigned long *bitmap_added; > + dma_addr_t iova; > struct vfio_dma *dma; > bool do_accounting; > > @@ -650,6 +654,12 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > > mutex_lock(&iommu->lock); > > + bitmap_added = bitmap_zalloc(npage, GFP_KERNEL); > + if (!bitmap_added) { > + ret = -ENOMEM; > + goto pin_done; > + } This is allocated regardless of whether dirty tracking is enabled, so this adds overhead to the common case in order to reduce user overhead (not correctness) in the rare condition that dirty tracking is enabled, and the even rarer condition that there's a fault during that case. This is not a good trade-off. Thanks, Alex > + > /* Fail if notifier list is empty */ > if (!iommu->notifier.head) { > ret = -EINVAL; > @@ -664,7 +674,6 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > do_accounting = !IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu); > > for (i = 0; i < npage; i++) { > - dma_addr_t iova; > struct vfio_pfn *vpfn; > > iova = user_pfn[i] << PAGE_SHIFT; > @@ -699,14 +708,10 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > } > > if (iommu->dirty_page_tracking) { > - unsigned long pgshift = __ffs(iommu->pgsize_bitmap); > - > - /* > - * Bitmap populated with the smallest supported page > - * size > - */ > - bitmap_set(dma->bitmap, > - (iova - dma->iova) >> pgshift, 1); > + /* Populated with the smallest supported page size */ > + bitmap_offset = (iova - dma->iova) >> pgshift; > + if (!test_and_set_bit(bitmap_offset, dma->bitmap)) > + set_bit(i, bitmap_added); > } > } > ret = i; > @@ -722,14 +727,20 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data, > pin_unwind: > phys_pfn[i] = 0; > for (j = 0; j < i; j++) { > - dma_addr_t iova; > - > iova = user_pfn[j] << PAGE_SHIFT; > dma = vfio_find_dma(iommu, iova, PAGE_SIZE); > vfio_unpin_page_external(dma, iova, do_accounting); > phys_pfn[j] = 0; > + > + if (test_bit(j, bitmap_added)) { > + bitmap_offset = (iova - dma->iova) >> pgshift; > + clear_bit(bitmap_offset, dma->bitmap); > + } > } > pin_done: > + if (bitmap_added) > + bitmap_free(bitmap_added); > + > mutex_unlock(&iommu->lock); > return ret; > }