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.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 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 6A1D4C433FE for ; Fri, 11 Dec 2020 06:52:05 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id 8081C23F2A for ; Fri, 11 Dec 2020 06:52:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8081C23F2A Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=huawei.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 D3EC04B1FD; Fri, 11 Dec 2020 01:52:03 -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 fW1DyZ-qAUJQ; Fri, 11 Dec 2020 01:52:02 -0500 (EST) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id A8E9C4B246; Fri, 11 Dec 2020 01:52:02 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id B5C0D4B1FD for ; Fri, 11 Dec 2020 01:52:01 -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 8a0CJtXN+hva for ; Fri, 11 Dec 2020 01:51:59 -0500 (EST) Received: from szxga07-in.huawei.com (szxga07-in.huawei.com [45.249.212.35]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 5F5F64B1E3 for ; Fri, 11 Dec 2020 01:51:58 -0500 (EST) Received: from DGGEMS410-HUB.china.huawei.com (unknown [172.30.72.60]) by szxga07-in.huawei.com (SkyGuard) with ESMTP id 4CshJn1TbZz7CXq; Fri, 11 Dec 2020 14:51:21 +0800 (CST) Received: from [10.174.187.37] (10.174.187.37) by DGGEMS410-HUB.china.huawei.com (10.3.19.210) with Microsoft SMTP Server id 14.3.487.0; Fri, 11 Dec 2020 14:51:48 +0800 Subject: Re: [PATCH 1/7] vfio: iommu_type1: Clear added dirty bit when unwind pin To: Alex Williamson References: <20201210073425.25960-1-zhukeqian1@huawei.com> <20201210073425.25960-2-zhukeqian1@huawei.com> <20201210121646.24fb3cd8@omen.home> From: zhukeqian Message-ID: Date: Fri, 11 Dec 2020 14:51:47 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <20201210121646.24fb3cd8@omen.home> X-Originating-IP: [10.174.187.37] X-CFilter-Loop: Reflected 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 2020/12/11 3:16, Alex Williamson wrote: > 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, Hi Alex, We can allocate the bitmap when dirty tracking is active, do you accept this? Or we can set the dirty bitmap after all pins succeed, but this costs cpu time to locate vfio_dma with iova. Thanks, Keqian > > 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=-15.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 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 6DE0AC433FE for ; Fri, 11 Dec 2020 06:52:08 +0000 (UTC) Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (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 A037423F2A for ; Fri, 11 Dec 2020 06:52:07 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A037423F2A Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=huawei.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 whitealder.osuosl.org (Postfix) with ESMTP id C050E86FAE; Fri, 11 Dec 2020 06:52:06 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id uACrJ0bmZkF1; Fri, 11 Dec 2020 06:52:05 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id ADB5586F99; Fri, 11 Dec 2020 06:52:05 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 91C4BC0FA7; Fri, 11 Dec 2020 06:52:05 +0000 (UTC) Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id D0FD7C013B for ; Fri, 11 Dec 2020 06:52:03 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id B81F186F92 for ; Fri, 11 Dec 2020 06:52:03 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 8wvE5g7qL9zI for ; Fri, 11 Dec 2020 06:52:02 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from szxga07-in.huawei.com (szxga07-in.huawei.com [45.249.212.35]) by whitealder.osuosl.org (Postfix) with ESMTPS id B56A586F8F for ; Fri, 11 Dec 2020 06:52:01 +0000 (UTC) Received: from DGGEMS410-HUB.china.huawei.com (unknown [172.30.72.60]) by szxga07-in.huawei.com (SkyGuard) with ESMTP id 4CshJn1TbZz7CXq; Fri, 11 Dec 2020 14:51:21 +0800 (CST) Received: from [10.174.187.37] (10.174.187.37) by DGGEMS410-HUB.china.huawei.com (10.3.19.210) with Microsoft SMTP Server id 14.3.487.0; Fri, 11 Dec 2020 14:51:48 +0800 Subject: Re: [PATCH 1/7] vfio: iommu_type1: Clear added dirty bit when unwind pin To: Alex Williamson References: <20201210073425.25960-1-zhukeqian1@huawei.com> <20201210073425.25960-2-zhukeqian1@huawei.com> <20201210121646.24fb3cd8@omen.home> From: zhukeqian Message-ID: Date: Fri, 11 Dec 2020 14:51:47 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <20201210121646.24fb3cd8@omen.home> X-Originating-IP: [10.174.187.37] X-CFilter-Loop: Reflected 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 2020/12/11 3:16, Alex Williamson wrote: > 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, Hi Alex, We can allocate the bitmap when dirty tracking is active, do you accept this? Or we can set the dirty bitmap after all pins succeed, but this costs cpu time to locate vfio_dma with iova. Thanks, Keqian > > 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=-15.3 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,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 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 ABA48C4361B for ; Fri, 11 Dec 2020 06:54:22 +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 6827323EAF for ; Fri, 11 Dec 2020 06:54:22 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6827323EAF Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=huawei.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:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=LOOMP7m9lHhU6zHZlzQg2RI+RzxPk5LR9iup+OFNccQ=; b=Ip7lZIbib8m4ki7t3u2pQr/I8 oMq1XX6OuYKVhA6bGqqHgJDI8ExMvseCV0/Gh52w+HpH1Gk1FalyHQkmxmetQUqXgockKexF/NqJ5 fXFH/v7FkKy5rPJ6POCqya88wX8NLZ/j73Qwd7HzXeykjCMlWdMGPTy1ifUWN+V8cr+BQZrfwszwp EXR1zKddK7pA+4zco1VB/OD6fYD2Hu45O69jbVOgiBH+DK4BB4KTCo/2c9bVtIvcE7kx7OgQlCELX aeHZeqIucgAsvBQGr70Jbfp/1WRbbzHH3r1H2nFVYUMrVfrea3vpA3wDpGNfYfvP3Bi02H7S/fyzY L3K6TN6lQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kncHb-0000ZS-Is; Fri, 11 Dec 2020 06:52:03 +0000 Received: from szxga07-in.huawei.com ([45.249.212.35]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kncHY-0000YX-12 for linux-arm-kernel@lists.infradead.org; Fri, 11 Dec 2020 06:52:01 +0000 Received: from DGGEMS410-HUB.china.huawei.com (unknown [172.30.72.60]) by szxga07-in.huawei.com (SkyGuard) with ESMTP id 4CshJn1TbZz7CXq; Fri, 11 Dec 2020 14:51:21 +0800 (CST) Received: from [10.174.187.37] (10.174.187.37) by DGGEMS410-HUB.china.huawei.com (10.3.19.210) with Microsoft SMTP Server id 14.3.487.0; Fri, 11 Dec 2020 14:51:48 +0800 Subject: Re: [PATCH 1/7] vfio: iommu_type1: Clear added dirty bit when unwind pin To: Alex Williamson References: <20201210073425.25960-1-zhukeqian1@huawei.com> <20201210073425.25960-2-zhukeqian1@huawei.com> <20201210121646.24fb3cd8@omen.home> From: zhukeqian Message-ID: Date: Fri, 11 Dec 2020 14:51:47 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <20201210121646.24fb3cd8@omen.home> X-Originating-IP: [10.174.187.37] X-CFilter-Loop: Reflected X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201211_015200_623937_5E33F70E X-CRM114-Status: GOOD ( 21.39 ) 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 2020/12/11 3:16, Alex Williamson wrote: > 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, Hi Alex, We can allocate the bitmap when dirty tracking is active, do you accept this? Or we can set the dirty bitmap after all pins succeed, but this costs cpu time to locate vfio_dma with iova. Thanks, Keqian > > 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.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 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 5C76BC1B0D9 for ; Fri, 11 Dec 2020 06:53:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1DFB123F2A for ; Fri, 11 Dec 2020 06:53:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2436535AbgLKGxG (ORCPT ); Fri, 11 Dec 2020 01:53:06 -0500 Received: from szxga07-in.huawei.com ([45.249.212.35]:9867 "EHLO szxga07-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2436527AbgLKGwk (ORCPT ); Fri, 11 Dec 2020 01:52:40 -0500 Received: from DGGEMS410-HUB.china.huawei.com (unknown [172.30.72.60]) by szxga07-in.huawei.com (SkyGuard) with ESMTP id 4CshJn1TbZz7CXq; Fri, 11 Dec 2020 14:51:21 +0800 (CST) Received: from [10.174.187.37] (10.174.187.37) by DGGEMS410-HUB.china.huawei.com (10.3.19.210) with Microsoft SMTP Server id 14.3.487.0; Fri, 11 Dec 2020 14:51:48 +0800 Subject: Re: [PATCH 1/7] vfio: iommu_type1: Clear added dirty bit when unwind pin To: Alex Williamson References: <20201210073425.25960-1-zhukeqian1@huawei.com> <20201210073425.25960-2-zhukeqian1@huawei.com> <20201210121646.24fb3cd8@omen.home> 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 , , From: zhukeqian Message-ID: Date: Fri, 11 Dec 2020 14:51:47 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <20201210121646.24fb3cd8@omen.home> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.174.187.37] X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On 2020/12/11 3:16, Alex Williamson wrote: > 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, Hi Alex, We can allocate the bitmap when dirty tracking is active, do you accept this? Or we can set the dirty bitmap after all pins succeed, but this costs cpu time to locate vfio_dma with iova. Thanks, Keqian > > 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; >> } > > . >