Re: [PATCH v3 1/3] KVM: Implement dirty quota-based throttling of vcpus

[Sean Christopherson <seanjc@google.com>]

On Thu, Mar 31, 2022, Shivam Kumar wrote:
> > > +	if (!dirty_quota || (pages_dirtied < dirty_quota))
> > > +		return 1;
> > I don't love returning 0/1 from a function that suggests it returns a bool, but
> > I do agree it's better than actually returning a bool.  I also don't have a better
> > name, so I'm just whining in the hope that Paolo or someone else has an idea :-)
> I've seen plenty of check functions returning 0/1 but please do let me know
> if there's a convention to use a bool in such scenarios.

The preferred style for KVM is to return a bool for helpers that are obviously
testing something, e.g. functions with names is "is_valid", "check_request", etc...
But we also very strongly prefer not returning bools from functions that have
side effects or can fail, i.e. don't use a bool to indicate success.

KVM has a third, gross use case of 0/1, where 0 means "exit to userspace" and 1
means "re-enter the guest".  Unfortunately, it's so ubiquitous that replacing it
with a proper enum is all but guaranteed to introduce bugs, and the 0/1 behavior
allows KVM to do things liek "if (!some_function())".

This helper falls into this last category of KVM's special 0/1 handling.  The
reason I don't love the name is the "check" part, which also puts it into "this
is a check helper".  But returning a bool would be even worse because the helper
does more than just check the quota, it also fills in the exit reason.