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 943CBC433EF for ; Thu, 23 Jun 2022 21:52:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229751AbiFWVwp (ORCPT ); Thu, 23 Jun 2022 17:52:45 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34042 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229545AbiFWVwo (ORCPT ); Thu, 23 Jun 2022 17:52:44 -0400 Received: from mail-pl1-x62a.google.com (mail-pl1-x62a.google.com [IPv6:2607:f8b0:4864:20::62a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 56886609D7 for ; Thu, 23 Jun 2022 14:52:43 -0700 (PDT) Received: by mail-pl1-x62a.google.com with SMTP id q18so373222pld.13 for ; Thu, 23 Jun 2022 14:52:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=WP+rwC4TGDAfQrrpdmxtx5HaFMWvuSkA5tpFa2Z0KTo=; b=FaIYrEPF7pH2Sg//kKmJsf4O808sp8hU3xWztRq7PW/NTqPPLAtvMC+D62zDoo+Hfo nD4lTJVJCNX6DnRqCnafQOpaZ8N3Ie155liv3aGLaVYMelGO1Mt51ztufJGskpU8fqgH soA/5thJgCOY4jOHBZt46Rh5pmOj6zNvlG4mmfvzHKkPscBNoRjStOvA/rY0qDp7PcGE /RPTM4wLwtu067izgHGTSTyu49NPFzY1X6noKkMnKFZzk623WewYhW1yMNqhhGq5xNVU /0m14dFYj73PPdK7f1clf1vXrU0z0O/T7MeJ4hLanIN4KJJk1axvx7cDOqXDjF0as+QH 21vA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=WP+rwC4TGDAfQrrpdmxtx5HaFMWvuSkA5tpFa2Z0KTo=; b=ZsWEgpk9RB4i9zpcrMZ/xU4kRsVZ1qqqqkX2f2tgjKNiYAwt44ndv1zbChRKontZ4v NbmxXpxLje0IvNPLIRhzgdG7bdGfzjFjAlvykvgQIdelTvMeVTcxojBRDFTk0733Prr7 GY6zaGni28VgAlpOqFxh5/D2Ss/wUEkrEjOjPqJ+bMC1rZApaJiZr+Fk7azYmlS9jddK iS+rTStsFMPo4/JRvOaoOPHNrnQxvt9lSSBq2HB6vkY8LXvfUpTt6sqz2xRhvyh0LHyd KQ0Fk6DBraxMu47BhK63vx9qTLHdfm/zz0BFb+lf3H6Moul1gxSLpIPa1ZTNmbSsfT4n Ixjw== X-Gm-Message-State: AJIora+JpFsGQuOIE/pxm95lmpV+HPwbBvSwIOHusqlyQ6nKEqphb1Dl ZRFNVDa11gkDi4pZJl9NNXzIqg== X-Google-Smtp-Source: AGRyM1uSglzJGxysU1/dykORfD2OPQF8J5jiG2z0akLmO9sFwpadT8seYG0Mf7zVPB0bl/0++f3GGA== X-Received: by 2002:a17:903:110d:b0:168:c610:9a80 with SMTP id n13-20020a170903110d00b00168c6109a80mr40966937plh.12.1656021162711; Thu, 23 Jun 2022 14:52:42 -0700 (PDT) Received: from google.com (123.65.230.35.bc.googleusercontent.com. [35.230.65.123]) by smtp.gmail.com with ESMTPSA id h4-20020a17090adb8400b001ecb28cfbfesm199878pjv.51.2022.06.23.14.52.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Jun 2022 14:52:42 -0700 (PDT) Date: Thu, 23 Jun 2022 21:52:38 +0000 From: Sean Christopherson To: Peter Xu Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Paolo Bonzini , Andrew Morton , David Hildenbrand , "Dr . David Alan Gilbert" , Andrea Arcangeli , Linux MM Mailing List Subject: Re: [PATCH 2/4] kvm: Merge "atomic" and "write" in __gfn_to_pfn_memslot() Message-ID: References: <20220622213656.81546-1-peterx@redhat.com> <20220622213656.81546-3-peterx@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jun 23, 2022, Peter Xu wrote: > On Thu, Jun 23, 2022 at 08:29:13PM +0000, Sean Christopherson wrote: > > This is what I came up with for splitting @async into a pure input (no_wait) and > > a return value (KVM_PFN_ERR_NEEDS_IO). > > The attached patch looks good to me. It's just that.. > > [...] > > > kvm_pfn_t __gfn_to_pfn_memslot(const struct kvm_memory_slot *slot, gfn_t gfn, > > - bool atomic, bool *async, bool write_fault, > > + bool atomic, bool no_wait, bool write_fault, > > bool *writable, hva_t *hva) > > .. with this patch on top we'll have 3 booleans already. With the new one > to add separated as suggested then it'll hit 4. > > Let's say one day we'll have that struct, but.. are you sure you think > keeping four booleans around is nicer than having a flag, no matter whether > we'd like to have a struct or not? No. > kvm_pfn_t __gfn_to_pfn_memslot(const struct kvm_memory_slot *slot, gfn_t gfn, > bool atomic, bool no_wait, bool write_fault, > bool interruptible, bool *writable, hva_t *hva); > > What if the booleans goes to 5, 6, or more? > > /me starts to wonder what'll be the magic number that we'll start to think > a bitmask flag will be more lovely here. :) For the number to really matter, it'd have to be comically large, e.g. 100+. This is all on-stack memory, so it's as close to free as can we can get. Overhead in terms of (un)marshalling is likely a wash for flags versus bools. Bools pack in nicely, so until there are a _lot_ of bools, memory is a non-issue. That leaves readability, which isn't dependent on the number so much as it is on the usage, and will be highly subjective based on the final code. In other words, I'm not dead set against flags, but I would like to see a complete cleanup before making a decision. My gut reaction is to use bools, as it makes consumption cleaner in most cases, e.g. if (!(xxx->write_fault || writable)) return false; versus if (!((xxx->flags & KVM_GTP_WRITE) || writable)) return false; but again I'm not going to say never until I actually see the end result.