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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 554C4C433EF for ; Sat, 5 Mar 2022 16:55:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231355AbiCEQ40 (ORCPT ); Sat, 5 Mar 2022 11:56:26 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37028 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229864AbiCEQ4Y (ORCPT ); Sat, 5 Mar 2022 11:56:24 -0500 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A53C34D61D for ; Sat, 5 Mar 2022 08:55:34 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 4135661462 for ; Sat, 5 Mar 2022 16:55:34 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id A24AFC004E1; Sat, 5 Mar 2022 16:55:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1646499333; bh=T6ex/K+aP5gcx0RmTjvu1joY7K/qYgJKPfflRHMP9e0=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=fHLgACTL3qKgFOJ74OGaB7rQnCO05s9vcDpNJI8FgXrAifSL7i0HTcZ8LLSbx0DhU iRSy3+dUG7Nf9DFW2VmTYq9ug1hzG3CTQ1/SoF2k6Kzv6lOyREGSWhSE2oM1qOcWFE W0pjIjCJT/IThzzRhXTQb5kTmOJybS7R0eR0U81M24cH8eQ+bf1Oe6yIWPNOn0ea5w WuHdZS1mt8qqcIWZ/v29p3Matl2kEBPHSvrZPF37lIzidk/k0RS7HGRBaEoRlUoa0/ NSGCXDp+l74oKWTbEhzDxw9fzFSVxmnnUoCr485IpVDtSnc9V42d4xaN1brTOrfmE0 Rm/CBmxT20Jjw== Received: from sofa.misterjones.org ([185.219.108.64] helo=billy-the-mountain.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1nQXgo-00CSYX-Va; Sat, 05 Mar 2022 16:55:31 +0000 Date: Sat, 05 Mar 2022 16:55:30 +0000 Message-ID: <878rtotk3h.wl-maz@kernel.org> From: Marc Zyngier To: David Matlack Cc: Paolo Bonzini , Huacai Chen , leksandar Markovic , Sean Christopherson , Vitaly Kuznetsov , Peter Xu , Wanpeng Li , Jim Mattson , Joerg Roedel , Peter Feiner , Andrew Jones , "Maciej S. Szmigiero" , kvm list Subject: Re: [PATCH 19/23] KVM: Allow for different capacities in kvm_mmu_memory_cache structs In-Reply-To: References: <20220203010051.2813563-1-dmatlack@google.com> <20220203010051.2813563-20-dmatlack@google.com> <8735k84i6f.wl-maz@kernel.org> 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 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: dmatlack@google.com, pbonzini@redhat.com, chenhuacai@kernel.org, aleksandar.qemu.devel@gmail.com, seanjc@google.com, vkuznets@redhat.com, peterx@redhat.com, wanpengli@tencent.com, jmattson@google.com, joro@8bytes.org, pfeiner@google.com, drjones@redhat.com, maciej.szmigiero@oracle.com, kvm@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Fri, 04 Mar 2022 21:59:12 +0000, David Matlack wrote: > > On Thu, Feb 24, 2022 at 11:20 AM David Matlack wrote: > > > > On Thu, Feb 24, 2022 at 3:29 AM Marc Zyngier wrote: > > > > > > On Thu, 03 Feb 2022 01:00:47 +0000, > > > David Matlack wrote: > > > > > > [...] > > > > > > > > > /* Cache some mmu pages needed inside spinlock regions */ > > > > - struct kvm_mmu_memory_cache mmu_page_cache; > > > > + DEFINE_KVM_MMU_MEMORY_CACHE(mmu_page_cache); > > > > > > I must say I'm really not a fan of the anonymous structure trick. I > > > can see why you are doing it that way, but it feels pretty brittle. > > > > Yeah I don't love it. It's really optimizing for minimizing the patch diff. > > > > The alternative I considered was to dynamically allocate the > > kvm_mmu_memory_cache structs. This would get rid of the anonymous > > struct and the objects array, and also eliminate the rather gross > > capacity hack in kvm_mmu_topup_memory_cache(). > > > > The downsides of this approach is more code and more failure paths if > > the allocation fails. > > I tried changing all kvm_mmu_memory_cache structs to be dynamically > allocated, but it created a lot of complexity to the setup/teardown > code paths in x86, arm64, mips, and riscv (the arches that use the > caches). I don't think this route is worth it, especially since these > structs don't *need* to be dynamically allocated. > > When you said the anonymous struct feels brittle, what did you have in > mind specifically? I can perfectly see someone using a kvm_mmu_memory_cache and searching for a bit why they end-up with memory corruption. Yes, this would be a rookie mistake, but this are some expectations all over the kernel that DEFINE_* and the corresponding structure are the same object. [...] > I see two alternatives to make this cleaner: > > 1. Dynamically allocate just this cache. The caches defined in > vcpu_arch will continue to use DEFINE_KVM_MMU_MEMORY_CACHE(). This > would get rid of the outer struct but require an extra memory > allocation. > 2. Move this cache to struct kvm_arch using > DEFINE_KVM_MMU_MEMORY_CACHE(). Then we don't need to stack allocate it > or dynamically allocate it. > > Do either of these approaches appeal to you more than the current one? Certainly, #2 feels more solid. Dynamic allocations (and the resulting pointer chasing) are usually costly in terms of performance, so I'd avoid it if at all possible. That being said, if it turns out that #2 isn't practical, I won't get in the way of your current approach. Moving kvm_mmu_memory_cache to core code was definitely a good cleanup, and I'm not overly excited with the perspective of *more* arch-specific code. Thanks, M. -- Without deviation from the norm, progress is not possible.