On 5/10/21 3:25 PM, Stefan Hajnoczi wrote: > On Fri, May 07, 2021 at 07:06:34PM +0200, Philippe Mathieu-Daudé wrote: >> By directly using find_first_bit() and find_next_bit from the >> "bitops.h" API to iterate over the bitmap, we can remove the >> bitmap[] variable-length array copy on the stack and the complex >> manual bit testing/clearing logic. >> >> Suggested-by: Stefan Hajnoczi <stefa...@redhat.com> >> Suggested-by: Richard Henderson <richard.hender...@linaro.org> >> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com> >> --- >> hw/block/dataplane/virtio-blk.c | 19 ++++--------------- >> 1 file changed, 4 insertions(+), 15 deletions(-) >> >> diff --git a/hw/block/dataplane/virtio-blk.c >> b/hw/block/dataplane/virtio-blk.c >> index e9050c8987e..a7b5bda06fc 100644 >> --- a/hw/block/dataplane/virtio-blk.c >> +++ b/hw/block/dataplane/virtio-blk.c >> @@ -60,23 +60,12 @@ static void notify_guest_bh(void *opaque) >> { >> VirtIOBlockDataPlane *s = opaque; >> unsigned nvqs = s->conf->num_queues; >> - unsigned long bitmap[BITS_TO_LONGS(nvqs)]; >> - unsigned j; >> >> - memcpy(bitmap, s->batch_notify_vqs, sizeof(bitmap)); >> - memset(s->batch_notify_vqs, 0, sizeof(bitmap)); >> + for (unsigned long i = find_first_bit(s->batch_notify_vqs, nvqs); >> + i < nvqs; i = find_next_bit(s->batch_notify_vqs, nvqs, i)) { >> + VirtQueue *vq = virtio_get_queue(s->vdev, i); >> >> - for (j = 0; j < nvqs; j += BITS_PER_LONG) { >> - unsigned long bits = bitmap[j / BITS_PER_LONG]; >> - >> - while (bits != 0) { >> - unsigned i = j + ctzl(bits); >> - VirtQueue *vq = virtio_get_queue(s->vdev, i); >> - >> - virtio_notify_irqfd(s->vdev, vq); >> - >> - bits &= bits - 1; /* clear right-most bit */ >> - } >> + virtio_notify_irqfd(s->vdev, vq); >> } >> } > > Bits in s->batch_notify_vqs[] must be cleared. Otherwise we may raise > spurious irqs next time this function is called. The memset() can be > moved after the loop.
Doh... good catch! I missed it when removing the previous test_and_clear_bit() call (from v1).