Re: [PATCH 19/23] KVM: Allow for different capacities in kvm_mmu_memory_cache structs

[Marc Zyngier <maz@kernel.org>]

On Wed, 09 Mar 2022 21:49:01 +0000,
David Matlack <dmatlack@google.com> wrote:
> 
> On Mon, Mar 7, 2022 at 3:49 PM David Matlack <dmatlack@google.com> wrote:
> >
> > On Sat, Mar 5, 2022 at 8:55 AM Marc Zyngier <maz@kernel.org> wrote:
> > >
> > > On Fri, 04 Mar 2022 21:59:12 +0000,
> > > David Matlack <dmatlack@google.com> wrote:
> > > > 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.
> >
> > Ok I'll play with #2. Thanks for the feedback.
> 
> #2 is very clean to implement but it ends up being a bit silly. It
> increases the size of struct kvm_arch by 336 bytes for all VMs but
> only ever gets used during kvm_vgic_map_resources(), which is only
> called the first time a vCPU is run (according to the comment in
> kvm_arch_vcpu_run_pid_change()). I think stack allocation makes the
> most sense for this object, I don't think it's worth dancing around
> that solely to avoid the inner struct grottiness.

Fair enough, and thanks for having had a look. I'll look at the next
version once you post it.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.