[Xen-devel] [PATCH 3/3] xen-blkback: fix shutdown race

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

Move the call to xen_blkif_put after we have freed the request,
otherwise we have a race between the release of the request and the
cleanup done in xen_blkif_free.

Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Cc: David Vrabel <david.vrabel@xxxxxxxxxx>
Cc: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
Cc: Matt Rushton <mrushton@xxxxxxxxxx>
Cc: Matt Wilson <msw@xxxxxxxxxx>
Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
---
 drivers/block/xen-blkback/blkback.c |   28 +++++++++++++++++++++-------
 1 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/block/xen-blkback/blkback.c 
b/drivers/block/xen-blkback/blkback.c
index dcfe49f..8200aa0 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -985,17 +985,31 @@ static void __end_block_io_op(struct pending_req 
*pending_req, int error)
         * the proper response on the ring.
         */
        if (atomic_dec_and_test(&pending_req->pendcnt)) {
-               xen_blkbk_unmap(pending_req->blkif,
+               struct xen_blkif *blkif = pending_req->blkif;
+
+               xen_blkbk_unmap(blkif,
                                pending_req->segments,
                                pending_req->nr_pages);
-               make_response(pending_req->blkif, pending_req->id,
+               make_response(blkif, pending_req->id,
                              pending_req->operation, pending_req->status);
-               xen_blkif_put(pending_req->blkif);
-               if (atomic_read(&pending_req->blkif->refcnt) <= 2) {
-                       if (atomic_read(&pending_req->blkif->drain))
-                               complete(&pending_req->blkif->drain_complete);
+               free_req(blkif, pending_req);
+               /*
+                * Make sure the request is freed before releasing blkif,
+                * or there could be a race between free_req and the
+                * cleanup done in xen_blkif_free during shutdown.
+                *
+                * NB: The fact that we might try to wake up pending_free_wq
+                * before drain_complete (in case there's a drain going on)
+                * it's not a problem with our current implementation
+                * because we can assure there's no thread waiting on
+                * pending_free_wq if there's a drain going on, but it has
+                * to be taken into account if the current model is changed.
+                */
+               xen_blkif_put(blkif);
+               if (atomic_read(&blkif->refcnt) <= 2) {
+                       if (atomic_read(&blkif->drain))
+                               complete(&blkif->drain_complete);
                }
-               free_req(pending_req->blkif, pending_req);
        }
 }
 
-- 
1.7.7.5 (Apple Git-26)


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel