[PATCH] kvm: x86: simplify kvm_vector_to_index()

[PATCH] kvm: x86: simplify kvm_vector_to_index()

[Yury Norov]

Use find_nth_bit() and make the function almost a one-liner.

Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 arch/x86/kvm/lapic.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 73418dc0ebb2..6c4ec016de6a 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1106,16 +1106,10 @@ EXPORT_SYMBOL_GPL(kvm_apic_match_dest);
 int kvm_vector_to_index(u32 vector, u32 dest_vcpus,
 		       const unsigned long *bitmap, u32 bitmap_size)
 {
-	u32 mod;
-	int i, idx = -1;
-
-	mod = vector % dest_vcpus;
-
-	for (i = 0; i <= mod; i++) {
-		idx = find_next_bit(bitmap, bitmap_size, idx + 1);
-		BUG_ON(idx == bitmap_size);
-	}
+	u32 mod = vector % dest_vcpus;
+	int idx = find_nth_bit(bitmap, bitmap_size, mod);
 
+	BUG_ON(idx >= bitmap_size);
 	return idx;
 }
 
-- 
2.43.0

Re: [PATCH] kvm: x86: simplify kvm_vector_to_index()

[Sean Christopherson]

On Sat, 19 Jul 2025 21:58:45 -0400, Yury Norov wrote:
> Use find_nth_bit() and make the function almost a one-liner.

Applied to kvm-x86 misc, thanks!

P.S. I'm amazed you could decipher the intent of the code.  Even with your
     patch, it took me 10+ minutes to understand the "logic".

[1/1] kvm: x86: simplify kvm_vector_to_index()
      https://github.com/kvm-x86/linux/commit/cc63f918a215

--
https://github.com/kvm-x86/linux/tree/next

Re: [PATCH] kvm: x86: simplify kvm_vector_to_index()

[Yury Norov]

On Tue, Aug 19, 2025 at 04:12:11PM -0700, Sean Christopherson wrote:
> On Sat, 19 Jul 2025 21:58:45 -0400, Yury Norov wrote:
> > Use find_nth_bit() and make the function almost a one-liner.
> 
> Applied to kvm-x86 misc, thanks!
> 
> P.S. I'm amazed you could decipher the intent of the code.  Even with your
>      patch, it took me 10+ minutes to understand the "logic".

Thanks Sean. :)

> [1/1] kvm: x86: simplify kvm_vector_to_index()
>       https://github.com/kvm-x86/linux/commit/cc63f918a215
> 
> --
> https://github.com/kvm-x86/linux/tree/next

Re: [PATCH] kvm: x86: simplify kvm_vector_to_index()

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 963 bytes --]

On 20.08.25 15:42, Yury Norov wrote:
> On Tue, Aug 19, 2025 at 04:12:11PM -0700, Sean Christopherson wrote:
>> On Sat, 19 Jul 2025 21:58:45 -0400, Yury Norov wrote:
>>> Use find_nth_bit() and make the function almost a one-liner.
>>
>> Applied to kvm-x86 misc, thanks!
>>
>> P.S. I'm amazed you could decipher the intent of the code.  Even with your
>>       patch, it took me 10+ minutes to understand the "logic".
> 
> Thanks Sean. :)
> 
>> [1/1] kvm: x86: simplify kvm_vector_to_index()
>>        https://github.com/kvm-x86/linux/commit/cc63f918a215

Is this really correct?

The original code has:

	for (i = 0; i <= mod; i++)

(note the "<=").

So it will find the (mod + 1)th bit set, so shouldn't it use

	idx = find_nth_bit(bitmap, bitmap_size, (vector % dest_vcpus) + 1);

instead?

My remark assumes that find_nth_bit(bitmap, bitmap_size, 1) will return the
same value as find_first_bit(bitmap, bitmap_size).


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH] kvm: x86: simplify kvm_vector_to_index()

[Yury Norov]

On Wed, Aug 20, 2025 at 04:01:22PM +0200, Juergen Gross wrote:
> On 20.08.25 15:42, Yury Norov wrote:
> > On Tue, Aug 19, 2025 at 04:12:11PM -0700, Sean Christopherson wrote:
> > > On Sat, 19 Jul 2025 21:58:45 -0400, Yury Norov wrote:
> > > > Use find_nth_bit() and make the function almost a one-liner.
> > > 
> > > Applied to kvm-x86 misc, thanks!
> > > 
> > > P.S. I'm amazed you could decipher the intent of the code.  Even with your
> > >       patch, it took me 10+ minutes to understand the "logic".
> > 
> > Thanks Sean. :)
> > 
> > > [1/1] kvm: x86: simplify kvm_vector_to_index()
> > >        https://github.com/kvm-x86/linux/commit/cc63f918a215
> 
> Is this really correct?
> 
> The original code has:
> 
> 	for (i = 0; i <= mod; i++)
> 
> (note the "<=").
> 
> So it will find the (mod + 1)th bit set, so shouldn't it use
> 
> 	idx = find_nth_bit(bitmap, bitmap_size, (vector % dest_vcpus) + 1);
> 
> instead?
> 
> My remark assumes that find_nth_bit(bitmap, bitmap_size, 1) will return the
> same value as find_first_bit(bitmap, bitmap_size).

find_nth_bit indexes those bits starting from 0, so 

find_nth_bit(bitmap, nbits, 0) == find_first_bit(bitmap, nbits)
find_nth_bit(bitmap, nbits, 1) == find_next_bit(bitmap, nbits,
                                        find_first_bit(bitmap, nbits))

And so on. Check test_find_nth_bit() for the examples.

Also, bitmap_size has a different meaning, so let's refer 'nbits'
instead.

Re: [PATCH] kvm: x86: simplify kvm_vector_to_index()

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 1436 bytes --]

On 20.08.25 16:12, Yury Norov wrote:
> On Wed, Aug 20, 2025 at 04:01:22PM +0200, Juergen Gross wrote:
>> On 20.08.25 15:42, Yury Norov wrote:
>>> On Tue, Aug 19, 2025 at 04:12:11PM -0700, Sean Christopherson wrote:
>>>> On Sat, 19 Jul 2025 21:58:45 -0400, Yury Norov wrote:
>>>>> Use find_nth_bit() and make the function almost a one-liner.
>>>>
>>>> Applied to kvm-x86 misc, thanks!
>>>>
>>>> P.S. I'm amazed you could decipher the intent of the code.  Even with your
>>>>        patch, it took me 10+ minutes to understand the "logic".
>>>
>>> Thanks Sean. :)
>>>
>>>> [1/1] kvm: x86: simplify kvm_vector_to_index()
>>>>         https://github.com/kvm-x86/linux/commit/cc63f918a215
>>
>> Is this really correct?
>>
>> The original code has:
>>
>> 	for (i = 0; i <= mod; i++)
>>
>> (note the "<=").
>>
>> So it will find the (mod + 1)th bit set, so shouldn't it use
>>
>> 	idx = find_nth_bit(bitmap, bitmap_size, (vector % dest_vcpus) + 1);
>>
>> instead?
>>
>> My remark assumes that find_nth_bit(bitmap, bitmap_size, 1) will return the
>> same value as find_first_bit(bitmap, bitmap_size).
> 
> find_nth_bit indexes those bits starting from 0, so
Ah, okay. That was the part I wasn't sure about, hence the addition of
my assumption regarding the semantics of find_nth_bit() (looking into the
__find_nth_bit() code didn't make that obvious at once).

Thanks for the clarification.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]