On Thu, Oct 05, 2023, Anish Moorthy wrote: > On Wed, Oct 4, 2023 at 6:44 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > > > Eh, the shortlog basically says "do work" with a lot of fancy words. It really > > just boils down to: > > > > KVM: Let callers of __gfn_to_pfn_memslot() opt-out of USERFAULT_ON_MISSING > > > > On Fri, Sep 08, 2023, Anish Moorthy wrote: > > > Change the "atomic" parameter of __gfn_to_pfn_memslot() to an enum which > > > > I've pushed back on more booleans multiple times, but IMO this is even worse. > > E.g. what does an "upgrade" to atomic even mean? > > Oh, that bad huh? Based on what you mentioned earlier about some > callers of __gfn_to_pfn_memslot() needing to opt out of the memslot > flag, it seemed like there were basically three ways (wrt to @atomic) > that function needed to be called > > 1. With atomic = true > 2. With atomic = false, and some way to make sure that's respected > whatever the memslot flag says > 3. With atomic = false, but respecting the memslot flag (ie, changing > to atomic = true when it's set). > > An "upgrade" in this context was meant to describe case 3 (when the > memslot flag is set). Anyways despite terminology issues, the idea of > an enum encapsulating those three cases seems like, essentially, the > right thing to do. Though of course, let me know if you disagree :) The problem is that the three possibilities aren't directly related. The existing use of atomic truly means "this call can't sleep/block". The userfault-on-missing case has nothing to do with sleeping being illegal, the behavior of @atomic just happens to align exactly with what is needed *today*. E.g. if there was a flavor of gup() that could fault in memory without sleeping, that could be used for the @atomic case but not the userfault-on-missing case. I doubt such a variation will ever exist, but "that probably won't happen" isn't a good reason to conflate the two things. > > Since we have line of sight to getting out of boolean hell via David's series, > > just bite the bullet for now. Deciphering the callers will suck, but not really > > anymore than it already sucks. > > Sorry, what exactly are you suggesting via "bite the bullet" here? Add another boolean, the "bool can_do_userfault" from the diff that got snipped.
