[Qemu-devel] [PATCH, v2] qemu/xendisk: properly update stats in ioreq_release()

[Qemu-devel] [PATCH, v2] qemu/xendisk: properly update stats in ioreq_release()

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 1638 bytes --]

While for the "normal" case (called from blk_send_response_all())
decrementing requests_finished is correct, doing so in the parse error
case is wrong; requests_inflight needs to be decremented instead.

Change in v2: Adjust coding style.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -154,7 +154,7 @@ static void ioreq_finish(struct ioreq *i
     blkdev->requests_finished++;
 }
 
-static void ioreq_release(struct ioreq *ioreq)
+static void ioreq_release(struct ioreq *ioreq, bool finish)
 {
     struct XenBlkDev *blkdev = ioreq->blkdev;
 
@@ -162,7 +162,11 @@ static void ioreq_release(struct ioreq *
     memset(ioreq, 0, sizeof(*ioreq));
     ioreq->blkdev = blkdev;
     QLIST_INSERT_HEAD(&blkdev->freelist, ioreq, list);
-    blkdev->requests_finished--;
+    if (finish) {
+        blkdev->requests_finished--;
+    } else {
+        blkdev->requests_inflight--;
+    }
 }
 
 /*
@@ -449,7 +453,7 @@ static void blk_send_response_all(struct
     while (!QLIST_EMPTY(&blkdev->finished)) {
         ioreq = QLIST_FIRST(&blkdev->finished);
         send_notify += blk_send_response_one(ioreq);
-        ioreq_release(ioreq);
+        ioreq_release(ioreq, true);
     }
     if (send_notify) {
         xen_be_send_notify(&blkdev->xendev);
@@ -505,7 +509,7 @@ static void blk_handle_requests(struct X
             if (blk_send_response_one(ioreq)) {
                 xen_be_send_notify(&blkdev->xendev);
             }
-            ioreq_release(ioreq);
+            ioreq_release(ioreq, false);
             continue;
         }
 




[-- Attachment #2: qemu-xendisk-accounting.patch --]
[-- Type: text/plain, Size: 1690 bytes --]

qemu/xendisk: properly update stats in ioreq_release()

While for the "normal" case (called from blk_send_response_all())
decrementing requests_finished is correct, doing so in the parse error
case is wrong; requests_inflight needs to be decremented instead.

Change in v2: Adjust coding style.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -154,7 +154,7 @@ static void ioreq_finish(struct ioreq *i
     blkdev->requests_finished++;
 }
 
-static void ioreq_release(struct ioreq *ioreq)
+static void ioreq_release(struct ioreq *ioreq, bool finish)
 {
     struct XenBlkDev *blkdev = ioreq->blkdev;
 
@@ -162,7 +162,11 @@ static void ioreq_release(struct ioreq *
     memset(ioreq, 0, sizeof(*ioreq));
     ioreq->blkdev = blkdev;
     QLIST_INSERT_HEAD(&blkdev->freelist, ioreq, list);
-    blkdev->requests_finished--;
+    if (finish) {
+        blkdev->requests_finished--;
+    } else {
+        blkdev->requests_inflight--;
+    }
 }
 
 /*
@@ -449,7 +453,7 @@ static void blk_send_response_all(struct
     while (!QLIST_EMPTY(&blkdev->finished)) {
         ioreq = QLIST_FIRST(&blkdev->finished);
         send_notify += blk_send_response_one(ioreq);
-        ioreq_release(ioreq);
+        ioreq_release(ioreq, true);
     }
     if (send_notify) {
         xen_be_send_notify(&blkdev->xendev);
@@ -505,7 +509,7 @@ static void blk_handle_requests(struct X
             if (blk_send_response_one(ioreq)) {
                 xen_be_send_notify(&blkdev->xendev);
             }
-            ioreq_release(ioreq);
+            ioreq_release(ioreq, false);
             continue;
         }
 

Re: [Qemu-devel] [PATCH, v2] qemu/xendisk: properly update stats in ioreq_release()

[Kevin Wolf]

Am 14.05.2012 12:30, schrieb Jan Beulich:
> While for the "normal" case (called from blk_send_response_all())
> decrementing requests_finished is correct, doing so in the parse error
> case is wrong; requests_inflight needs to be decremented instead.
> 
> Change in v2: Adjust coding style.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

I think it would be nicer if a struct ioreq had an explicit state so
that you don't have to pass the right state when releasing it. But
anyway, the patch looks correct to me:

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Re: [Qemu-devel] [PATCH, v2] qemu/xendisk: properly update stats in ioreq_release()

[Stefano Stabellini]

On Mon, 14 May 2012, Jan Beulich wrote:
> While for the "normal" case (called from blk_send_response_all())
> decrementing requests_finished is correct, doing so in the parse error
> case is wrong; requests_inflight needs to be decremented instead.
> 
> Change in v2: Adjust coding style.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Re: [Qemu-devel] [PATCH, v2] qemu/xendisk: properly update stats in ioreq_release()

[Kevin Wolf]

Am 14.05.2012 16:57, schrieb Stefano Stabellini:
> On Mon, 14 May 2012, Jan Beulich wrote:
>> While for the "normal" case (called from blk_send_response_all())
>> decrementing requests_finished is correct, doing so in the parse error
>> case is wrong; requests_inflight needs to be decremented instead.
>>
>> Change in v2: Adjust coding style.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Aren't you going to send a pull request yourself, Stefano?

Kevin

Re: [Qemu-devel] [PATCH, v2] qemu/xendisk: properly update stats in ioreq_release()

[Stefano Stabellini]

On Tue, 15 May 2012, Kevin Wolf wrote:
> Am 14.05.2012 16:57, schrieb Stefano Stabellini:
> > On Mon, 14 May 2012, Jan Beulich wrote:
> >> While for the "normal" case (called from blk_send_response_all())
> >> decrementing requests_finished is correct, doing so in the parse error
> >> case is wrong; requests_inflight needs to be decremented instead.
> >>
> >> Change in v2: Adjust coding style.
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> > 
> > Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> Aren't you going to send a pull request yourself, Stefano?

Yes, I am: I am collecting a series of Xen patches for 1.1 and I am
aiming at sending a single pull request by the end of the week.

http://marc.info/?l=qemu-devel&m=133701567429388&w=2

Re: [Qemu-devel] [PATCH, v2] qemu/xendisk: properly update stats in ioreq_release()

[Kevin Wolf]

Am 15.05.2012 12:16, schrieb Stefano Stabellini:
> On Tue, 15 May 2012, Kevin Wolf wrote:
>> Am 14.05.2012 16:57, schrieb Stefano Stabellini:
>>> On Mon, 14 May 2012, Jan Beulich wrote:
>>>> While for the "normal" case (called from blk_send_response_all())
>>>> decrementing requests_finished is correct, doing so in the parse error
>>>> case is wrong; requests_inflight needs to be decremented instead.
>>>>
>>>> Change in v2: Adjust coding style.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>
>> Aren't you going to send a pull request yourself, Stefano?
> 
> Yes, I am: I am collecting a series of Xen patches for 1.1 and I am
> aiming at sending a single pull request by the end of the week.
> 
> http://marc.info/?l=qemu-devel&m=133701567429388&w=2

Ok, great.

I was just wondering because I always understood an Acked-by as a
message to the maintainer who picks the patch up, so I wasn't sure if
you expected me to do it and the patch would fall through the cracks.

Kevin