Re: [PATCH] KVM: x86: use assign_bit() where applicable

[David Laight]

On Mon, 28 Sep 2026 17:58:57 -0700
Sean Christopherson <seanjc@xxxxxxxxxx> wrote:

> On Sun, Sep 20, 2026, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@xxxxxxx>
> > 
> > Convert open-coded if/else with set_bit/clear_bit and their
> > non-atomic __set_bit/__clear_bit variants to the assign_bit/__assign_bit
> > API.  
> 
> ...
> 
> > Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
> > ---
> >  arch/x86/kvm/hyperv.c  | 12 ++++--------
> >  arch/x86/kvm/svm/pmu.c |  6 ++----
> >  arch/x86/kvm/x86.c     | 11 +++--------
> >  3 files changed, 9 insertions(+), 20 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> > index 8d2669d8ef34..c131d9a3c550 100644
> > --- a/arch/x86/kvm/hyperv.c
> > +++ b/arch/x86/kvm/hyperv.c
> > @@ -114,17 +114,13 @@ static void synic_update_vector(struct kvm_vcpu_hv_synic *synic,
> >  	if (vector < HV_SYNIC_FIRST_VALID_VECTOR)
> >  		return;
> >  
> > -	if (synic_has_vector_connected(synic, vector))
> > -		__set_bit(vector, synic->vec_bitmap);
> > -	else
> > -		__clear_bit(vector, synic->vec_bitmap);
> > +	__assign_bit(vector, synic->vec_bitmap,
> > +		     synic_has_vector_connected(synic, vector));
> >  
> >  	auto_eoi_old = !bitmap_empty(synic->auto_eoi_bitmap, 256);
> >  
> > -	if (synic_has_vector_auto_eoi(synic, vector))
> > -		__set_bit(vector, synic->auto_eoi_bitmap);
> > -	else
> > -		__clear_bit(vector, synic->auto_eoi_bitmap);
> > +	__assign_bit(vector, synic->auto_eoi_bitmap,
> > +		     synic_has_vector_auto_eoi(synic, vector));  
> 
> Am I the only one that finds the assign_bit() code signficantly harder to follow?
> Maybe it's just that I haven't seen assign_bit() much, but I've come back to this
> patch several times, and I've had the same reaction every time.  IMO, this is a
> solution looking for a problem.
> 

The same is pretty much true of __set_bit() and even BIT().
What is wrong with:
	if (synic_has_vector_auto_eoi(synic, vector))
		vector |= 1u << synic->auto_eoi_bitmap;
	else
		vector &= ~(1u << synic->auto_eoi_bitmap);
'Does what is sway on the tin'.

David