* [PATCH v2 0/2] virtio-blk: Convert QEMUBH callback to "bitops.h" API
@ 2021-05-07 17:06 Philippe Mathieu-Daudé
2021-05-07 17:06 ` [PATCH v2 1/2] bitops.h: Improve find_xxx_bit() documentation Philippe Mathieu-Daudé
2021-05-07 17:06 ` [PATCH v2 2/2] virtio-blk: Convert QEMUBH callback to "bitops.h" API Philippe Mathieu-Daudé
0 siblings, 2 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-07 17:06 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stefan Hajnoczi, Sergio Lopez, Richard Henderson,
Max Reitz, Chai Wen, qemu-block, Philippe Mathieu-Daudé
This series follow a suggestion from Stefan to use the bitops
API in virtio-blk:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg805139.html
Since v1:
- improved "bitops.h" docstring
- addressed Richard's review comments
Philippe Mathieu-Daudé (2):
bitops.h: Improve find_xxx_bit() documentation
virtio-blk: Convert QEMUBH callback to "bitops.h" API
include/qemu/bitops.h | 15 ++++++++++++---
hw/block/dataplane/virtio-blk.c | 19 ++++---------------
2 files changed, 16 insertions(+), 18 deletions(-)
--
2.26.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] bitops.h: Improve find_xxx_bit() documentation
2021-05-07 17:06 [PATCH v2 0/2] virtio-blk: Convert QEMUBH callback to "bitops.h" API Philippe Mathieu-Daudé
@ 2021-05-07 17:06 ` Philippe Mathieu-Daudé
2021-05-07 17:24 ` Richard Henderson
2021-05-07 17:38 ` Eric Blake
2021-05-07 17:06 ` [PATCH v2 2/2] virtio-blk: Convert QEMUBH callback to "bitops.h" API Philippe Mathieu-Daudé
1 sibling, 2 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-07 17:06 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stefan Hajnoczi, Sergio Lopez, Richard Henderson,
Max Reitz, Chai Wen, qemu-block, Philippe Mathieu-Daudé
Document the following functions return the bitmap size
if not matching bit is found:
- find_first_bit
- find_next_bit
- find_last_bit
- find_first_zero_bit
- find_next_zero_bit
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
include/qemu/bitops.h | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index 3acbf3384c6..eb0fc5416a9 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -140,7 +140,8 @@ static inline int test_bit(long nr, const unsigned long *addr)
* @addr: The address to start the search at
* @size: The maximum size to search
*
- * Returns the bit number of the first set bit, or size.
+ * Returns the bit number of the last set bit,
+ * or @size if there is no set bit in the bitmap.
*/
unsigned long find_last_bit(const unsigned long *addr,
unsigned long size);
@@ -150,6 +151,9 @@ unsigned long find_last_bit(const unsigned long *addr,
* @addr: The address to base the search on
* @offset: The bitnumber to start searching at
* @size: The bitmap size in bits
+ *
+ * Returns the bit number of the next set bit,
+ * or @size if there is no set bit in the bitmap.
*/
unsigned long find_next_bit(const unsigned long *addr,
unsigned long size,
@@ -160,6 +164,9 @@ unsigned long find_next_bit(const unsigned long *addr,
* @addr: The address to base the search on
* @offset: The bitnumber to start searching at
* @size: The bitmap size in bits
+ *
+ * Returns the bit number of the next cleared bit,
+ * or @size if there is no clear bit in the bitmap.
*/
unsigned long find_next_zero_bit(const unsigned long *addr,
@@ -171,7 +178,8 @@ unsigned long find_next_zero_bit(const unsigned long *addr,
* @addr: The address to start the search at
* @size: The maximum size to search
*
- * Returns the bit number of the first set bit.
+ * Returns the bit number of the first set bit,
+ * or @size if there is no set bit in the bitmap.
*/
static inline unsigned long find_first_bit(const unsigned long *addr,
unsigned long size)
@@ -194,7 +202,8 @@ static inline unsigned long find_first_bit(const unsigned long *addr,
* @addr: The address to start the search at
* @size: The maximum size to search
*
- * Returns the bit number of the first cleared bit.
+ * Returns the bit number of the first cleared bit,
+ * or @size if there is no clear bit in the bitmap.
*/
static inline unsigned long find_first_zero_bit(const unsigned long *addr,
unsigned long size)
--
2.26.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] virtio-blk: Convert QEMUBH callback to "bitops.h" API
2021-05-07 17:06 [PATCH v2 0/2] virtio-blk: Convert QEMUBH callback to "bitops.h" API Philippe Mathieu-Daudé
2021-05-07 17:06 ` [PATCH v2 1/2] bitops.h: Improve find_xxx_bit() documentation Philippe Mathieu-Daudé
@ 2021-05-07 17:06 ` Philippe Mathieu-Daudé
2021-05-07 17:26 ` Richard Henderson
2021-05-10 13:25 ` Stefan Hajnoczi
1 sibling, 2 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-07 17:06 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stefan Hajnoczi, Sergio Lopez, Richard Henderson,
Max Reitz, Chai Wen, qemu-block, Philippe Mathieu-Daudé
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 <stefanha@redhat.com>
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@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);
}
}
--
2.26.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] bitops.h: Improve find_xxx_bit() documentation
2021-05-07 17:06 ` [PATCH v2 1/2] bitops.h: Improve find_xxx_bit() documentation Philippe Mathieu-Daudé
@ 2021-05-07 17:24 ` Richard Henderson
2021-05-07 17:38 ` Eric Blake
1 sibling, 0 replies; 8+ messages in thread
From: Richard Henderson @ 2021-05-07 17:24 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Kevin Wolf, Sergio Lopez, qemu-block, Stefan Hajnoczi, Max Reitz,
Chai Wen
On 5/7/21 10:06 AM, Philippe Mathieu-Daudé wrote:
> Document the following functions return the bitmap size
> if not matching bit is found:
>
> - find_first_bit
> - find_next_bit
> - find_last_bit
> - find_first_zero_bit
> - find_next_zero_bit
>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@redhat.com>
> ---
> include/qemu/bitops.h | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] virtio-blk: Convert QEMUBH callback to "bitops.h" API
2021-05-07 17:06 ` [PATCH v2 2/2] virtio-blk: Convert QEMUBH callback to "bitops.h" API Philippe Mathieu-Daudé
@ 2021-05-07 17:26 ` Richard Henderson
2021-05-10 13:25 ` Stefan Hajnoczi
1 sibling, 0 replies; 8+ messages in thread
From: Richard Henderson @ 2021-05-07 17:26 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Kevin Wolf, Sergio Lopez, qemu-block, Stefan Hajnoczi, Max Reitz,
Chai Wen
On 5/7/21 10:06 AM, 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<stefanha@redhat.com>
> Suggested-by: Richard Henderson<richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@redhat.com>
> ---
> hw/block/dataplane/virtio-blk.c | 19 ++++---------------
> 1 file changed, 4 insertions(+), 15 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] bitops.h: Improve find_xxx_bit() documentation
2021-05-07 17:06 ` [PATCH v2 1/2] bitops.h: Improve find_xxx_bit() documentation Philippe Mathieu-Daudé
2021-05-07 17:24 ` Richard Henderson
@ 2021-05-07 17:38 ` Eric Blake
1 sibling, 0 replies; 8+ messages in thread
From: Eric Blake @ 2021-05-07 17:38 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Kevin Wolf, Stefan Hajnoczi, Sergio Lopez, Richard Henderson,
Max Reitz, Chai Wen, qemu-block
On 5/7/21 12:06 PM, Philippe Mathieu-Daudé wrote:
> Document the following functions return the bitmap size
> if not matching bit is found:
s/not/no/
>
> - find_first_bit
> - find_next_bit
> - find_last_bit
> - find_first_zero_bit
> - find_next_zero_bit
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> include/qemu/bitops.h | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> @@ -150,6 +151,9 @@ unsigned long find_last_bit(const unsigned long *addr,
> * @addr: The address to base the search on
> * @offset: The bitnumber to start searching at
> * @size: The bitmap size in bits
> + *
> + * Returns the bit number of the next set bit,
> + * or @size if there is no set bit in the bitmap.
> */
> unsigned long find_next_bit(const unsigned long *addr,
Misleading - there might be a set bit prior to @offset. Better might be:
or @size if there are no further set bits in the bitmap.
> unsigned long size,
> @@ -160,6 +164,9 @@ unsigned long find_next_bit(const unsigned long *addr,
> * @addr: The address to base the search on
> * @offset: The bitnumber to start searching at
> * @size: The bitmap size in bits
> + *
> + * Returns the bit number of the next cleared bit,
> + * or @size if there is no clear bit in the bitmap.
> */
>
> unsigned long find_next_zero_bit(const unsigned long *addr,
similarly,
or @size if there are no further clear bits in the bitmap.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] virtio-blk: Convert QEMUBH callback to "bitops.h" API
2021-05-07 17:06 ` [PATCH v2 2/2] virtio-blk: Convert QEMUBH callback to "bitops.h" API Philippe Mathieu-Daudé
2021-05-07 17:26 ` Richard Henderson
@ 2021-05-10 13:25 ` Stefan Hajnoczi
2021-05-10 14:05 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 8+ messages in thread
From: Stefan Hajnoczi @ 2021-05-10 13:25 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, qemu-block, Sergio Lopez, Richard Henderson,
qemu-devel, Max Reitz, Chai Wen
[-- Attachment #1: Type: text/plain, Size: 2019 bytes --]
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 <stefanha@redhat.com>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@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.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] virtio-blk: Convert QEMUBH callback to "bitops.h" API
2021-05-10 13:25 ` Stefan Hajnoczi
@ 2021-05-10 14:05 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-10 14:05 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Kevin Wolf, qemu-block, Sergio Lopez, Richard Henderson,
qemu-devel, Max Reitz, Chai Wen
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 <stefanha@redhat.com>
>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@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).
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-05-10 14:05 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-07 17:06 [PATCH v2 0/2] virtio-blk: Convert QEMUBH callback to "bitops.h" API Philippe Mathieu-Daudé
2021-05-07 17:06 ` [PATCH v2 1/2] bitops.h: Improve find_xxx_bit() documentation Philippe Mathieu-Daudé
2021-05-07 17:24 ` Richard Henderson
2021-05-07 17:38 ` Eric Blake
2021-05-07 17:06 ` [PATCH v2 2/2] virtio-blk: Convert QEMUBH callback to "bitops.h" API Philippe Mathieu-Daudé
2021-05-07 17:26 ` Richard Henderson
2021-05-10 13:25 ` Stefan Hajnoczi
2021-05-10 14:05 ` Philippe Mathieu-Daudé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.