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=-14.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS 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 5FB20C433C1 for ; Tue, 23 Mar 2021 17:13:48 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (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 F13E461925 for ; Tue, 23 Mar 2021 17:13:47 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F13E461925 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.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=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To:Subject:Cc:To: From:Message-ID:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=UPGHZ+c7+0v+18/H6I2yUMGRNP3pG8gQPWLP1kFCwgY=; b=CrR3ouMyETNRqgT6jsJX/KBmV BhLTgpmdwksVmfBHcHcu0n6Lj6XTj4jLwyRnRf/Nf0Lsn2AwvtVp8/xO0kvdsLQOejlJy9adDytzZ D/pW9J/QJibgJQzGUvYF3viuPBWaqqV9Yp3UkpsMndnJHlCJyp7SQx2qdwqYD9avw0f1Knyu0E+NW gbr90C+DYqOMdJCkmxdinme3RYeRJZzw5UhrU/6jqr5AAHWM6C4dbbWNuK9udu9P+AAQy8xCWKWNm MkWvQb1FKKiL9hCN+eRAtQbbXDsMFglDhDRCSN38bsA+l2AMHFezHsRSeaOdHv9reM4l46xSPNmMs D2cotkIDA==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lOkZk-00FN9q-4m; Tue, 23 Mar 2021 17:12:16 +0000 Received: from mail.kernel.org ([198.145.29.99]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lOkZc-00FN8k-Oy for linux-arm-kernel@lists.infradead.org; Tue, 23 Mar 2021 17:12:11 +0000 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 1AFAD6188B; Tue, 23 Mar 2021 17:12:07 +0000 (UTC) Received: from 78.163-31-62.static.virginmediabusiness.co.uk ([62.31.163.78] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94) (envelope-from ) id 1lOkZZ-003Lxl-2s; Tue, 23 Mar 2021 17:12:05 +0000 Date: Tue, 23 Mar 2021 17:12:04 +0000 Message-ID: <87sg4lkd5n.wl-maz@kernel.org> From: Marc Zyngier To: Yoan Picchi Cc: james.morse@arm.com, julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, catalin.marinas@arm.com, will@kernel.org Subject: Re: [PATCH 2/7] KVM: arm64: Add remote_tlb_flush counter for kvm_stat In-Reply-To: <20210319161711.24972-3-yoan.picchi@arm.com> References: <20210319161711.24972-1-yoan.picchi@arm.com> <20210319161711.24972-3-yoan.picchi@arm.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 62.31.163.78 X-SA-Exim-Rcpt-To: yoan.picchi@arm.com, james.morse@arm.com, julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, catalin.marinas@arm.com, will@kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210323_171209_238610_3F378A90 X-CRM114-Status: GOOD ( 28.75 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 Fri, 19 Mar 2021 16:17:06 +0000, Yoan Picchi wrote: > > Add a counter for remote tlb flushes. > I think flushing the tlb is important enough of a thing so that one using > kvm_stat should be aware if their code is trigering several flushes. > Beside the event is recorded in x86 and ppc as well so there might be > even more reasons that I can't think of. And does this stat mean the same thing across architectures? > Looking at where this is called, it mostly happen when someone is > updating the dirty pages while we are doing some operation on them > (like enabling dirty pages logging) How about swapping, KSM, VM teardown? > > There's one catch though, it is not always thread safe. Sometime it is > called under some lock, some other time it isn't. > We can't change the counter to an atomic_t as all the counters are > unsigned. We shouldn't add a lock as this could be adding a lock > (say, to kvm->arch) for a very minor thing and I would rather not pollute > anything without a better reason. That's why I ended up using cmpxchg > which according to LWN (https://lwn.net/Articles/695257/) is an old way > to do without atomic. It's less efficient than an atomic increment, but > this should happen very rarely anyway and is stil better than a lock. Are you actually worried about this stat being exact? > Signed-off-by: Yoan Picchi > --- > arch/arm64/kvm/guest.c | 1 + > arch/arm64/kvm/mmu.c | 11 +++++++++++ > 2 files changed, 12 insertions(+) > > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c > index 14b15fb8f..1029976ca 100644 > --- a/arch/arm64/kvm/guest.c > +++ b/arch/arm64/kvm/guest.c > @@ -40,6 +40,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = { > VCPU_STAT("mmio_exit_kernel", mmio_exit_kernel), > VCPU_STAT("regular_page_mapped", regular_page_mapped), > VCPU_STAT("huge_page_mapped", huge_page_mapped), > + VM_STAT("remote_tlb_flush", remote_tlb_flush), Two things: - having a VM_STAT stuck in the middle on a set of per-vcpu stats isn't great - what does "remote TLB flush" means for the ARM architecture, which uses broadcast invalidation? Slapping foreign concepts in the architecture code doesn't strike me as the best course of action Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel