]> git.kernelconcepts.de Git - karo-tx-linux.git/commitdiff
Merge branch 'stable/for-jens-4.2' of git://git.kernel.org/pub/scm/linux/kernel/git...
authorJens Axboe <axboe@fb.com>
Mon, 27 Jul 2015 17:58:41 +0000 (11:58 -0600)
committerJens Axboe <axboe@fb.com>
Mon, 27 Jul 2015 17:58:41 +0000 (11:58 -0600)
Konrad writes:

"There are three bugs that have been found in the xen-blkfront (and
backend). Two of them have the stable tree CC-ed. They have been found
where an guest is migrating to a host that is missing
'feature-persistent' support (from one that has it enabled). We end up
hitting an BUG() in the driver code."

1  2 
drivers/block/xen-blkback/blkback.c
drivers/block/xen-blkfront.c

index ced96777b677b9bcddd65bae004a7a51b5cf0dc3,73c04040c8c8a6be1eac0a852d6fb100fa0230cd..954c0029fb3babc49d1a1f490f9d420934701e30
@@@ -272,6 -272,17 +272,6 @@@ static void put_persistent_gnt(struct x
        atomic_dec(&blkif->persistent_gnt_in_use);
  }
  
 -static void free_persistent_gnts_unmap_callback(int result,
 -                                              struct gntab_unmap_queue_data *data)
 -{
 -      struct completion *c = data->data;
 -
 -      /* BUG_ON used to reproduce existing behaviour,
 -         but is this the best way to deal with this? */
 -      BUG_ON(result);
 -      complete(c);
 -}
 -
  static void free_persistent_gnts(struct xen_blkif *blkif, struct rb_root *root,
                                   unsigned int num)
  {
        struct rb_node *n;
        int segs_to_unmap = 0;
        struct gntab_unmap_queue_data unmap_data;
 -      struct completion unmap_completion;
  
 -      init_completion(&unmap_completion);
 -
 -      unmap_data.data = &unmap_completion;
 -      unmap_data.done = &free_persistent_gnts_unmap_callback;
        unmap_data.pages = pages;
        unmap_data.unmap_ops = unmap;
        unmap_data.kunmap_ops = NULL;
                        !rb_next(&persistent_gnt->node)) {
  
                        unmap_data.count = segs_to_unmap;
 -                      gnttab_unmap_refs_async(&unmap_data);
 -                      wait_for_completion(&unmap_completion);
 +                      BUG_ON(gnttab_unmap_refs_sync(&unmap_data));
  
                        put_free_pages(blkif, pages, segs_to_unmap);
                        segs_to_unmap = 0;
@@@ -319,13 -336,8 +319,13 @@@ void xen_blkbk_unmap_purged_grants(stru
        struct gnttab_unmap_grant_ref unmap[BLKIF_MAX_SEGMENTS_PER_REQUEST];
        struct page *pages[BLKIF_MAX_SEGMENTS_PER_REQUEST];
        struct persistent_gnt *persistent_gnt;
 -      int ret, segs_to_unmap = 0;
 +      int segs_to_unmap = 0;
        struct xen_blkif *blkif = container_of(work, typeof(*blkif), persistent_purge_work);
 +      struct gntab_unmap_queue_data unmap_data;
 +
 +      unmap_data.pages = pages;
 +      unmap_data.unmap_ops = unmap;
 +      unmap_data.kunmap_ops = NULL;
  
        while(!list_empty(&blkif->persistent_purge_list)) {
                persistent_gnt = list_first_entry(&blkif->persistent_purge_list,
                pages[segs_to_unmap] = persistent_gnt->page;
  
                if (++segs_to_unmap == BLKIF_MAX_SEGMENTS_PER_REQUEST) {
 -                      ret = gnttab_unmap_refs(unmap, NULL, pages,
 -                              segs_to_unmap);
 -                      BUG_ON(ret);
 +                      unmap_data.count = segs_to_unmap;
 +                      BUG_ON(gnttab_unmap_refs_sync(&unmap_data));
                        put_free_pages(blkif, pages, segs_to_unmap);
                        segs_to_unmap = 0;
                }
                kfree(persistent_gnt);
        }
        if (segs_to_unmap > 0) {
 -              ret = gnttab_unmap_refs(unmap, NULL, pages, segs_to_unmap);
 -              BUG_ON(ret);
 +              unmap_data.count = segs_to_unmap;
 +              BUG_ON(gnttab_unmap_refs_sync(&unmap_data));
                put_free_pages(blkif, pages, segs_to_unmap);
        }
  }
@@@ -369,8 -382,8 +369,8 @@@ static void purge_persistent_gnt(struc
                return;
        }
  
-       if (work_pending(&blkif->persistent_purge_work)) {
-               pr_alert_ratelimited("Scheduled work from previous purge is still pending, cannot purge list\n");
+       if (work_busy(&blkif->persistent_purge_work)) {
+               pr_alert_ratelimited("Scheduled work from previous purge is still busy, cannot purge list\n");
                return;
        }
  
@@@ -736,7 -749,7 +736,7 @@@ static void xen_blkbk_unmap_and_respond
        struct grant_page **pages = req->segments;
        unsigned int invcount;
  
 -      invcount = xen_blkbk_unmap_prepare(blkif, pages, req->nr_pages,
 +      invcount = xen_blkbk_unmap_prepare(blkif, pages, req->nr_segs,
                                           req->unmap, req->unmap_pages);
  
        work->data = req;
@@@ -922,7 -935,7 +922,7 @@@ static int xen_blkbk_map_seg(struct pen
        int rc;
  
        rc = xen_blkbk_map(pending_req->blkif, pending_req->segments,
 -                         pending_req->nr_pages,
 +                         pending_req->nr_segs,
                           (pending_req->operation != BLKIF_OP_READ));
  
        return rc;
@@@ -938,7 -951,7 +938,7 @@@ static int xen_blkbk_parse_indirect(str
        int indirect_grefs, rc, n, nseg, i;
        struct blkif_request_segment *segments = NULL;
  
 -      nseg = pending_req->nr_pages;
 +      nseg = pending_req->nr_segs;
        indirect_grefs = INDIRECT_PAGES(nseg);
        BUG_ON(indirect_grefs > BLKIF_MAX_INDIRECT_PAGES_PER_REQUEST);
  
@@@ -1258,7 -1271,7 +1258,7 @@@ static int dispatch_rw_block_io(struct 
        pending_req->id        = req->u.rw.id;
        pending_req->operation = req_operation;
        pending_req->status    = BLKIF_RSP_OKAY;
 -      pending_req->nr_pages  = nseg;
 +      pending_req->nr_segs   = nseg;
  
        if (req->operation != BLKIF_OP_INDIRECT) {
                preq.dev               = req->u.rw.handle;
  
   fail_flush:
        xen_blkbk_unmap(blkif, pending_req->segments,
 -                      pending_req->nr_pages);
 +                      pending_req->nr_segs);
   fail_response:
        /* Haven't submitted any bio's yet. */
        make_response(blkif, req->u.rw.id, req_operation, BLKIF_RSP_ERROR);
index 6d89ed35d80c0caaf8bf57ba82c7e9f3a9194bb9,44b33d39441b1235150a87486359cea2f304d329..7a8a73f1fc0462feab5bad706573ff6eb4536ef7
@@@ -179,6 -179,7 +179,7 @@@ static DEFINE_SPINLOCK(minor_lock)
        ((_segs + SEGS_PER_INDIRECT_FRAME - 1)/SEGS_PER_INDIRECT_FRAME)
  
  static int blkfront_setup_indirect(struct blkfront_info *info);
+ static int blkfront_gather_backend_features(struct blkfront_info *info);
  
  static int get_id_from_freelist(struct blkfront_info *info)
  {
@@@ -1074,6 -1075,12 +1075,6 @@@ static void blkif_completion(struct blk
                s->req.u.indirect.nr_segments : s->req.u.rw.nr_segments;
  
        if (bret->operation == BLKIF_OP_READ && info->feature_persistent) {
 -              /*
 -               * Copy the data received from the backend into the bvec.
 -               * Since bv_offset can be different than 0, and bv_len different
 -               * than PAGE_SIZE, we have to keep track of the current offset,
 -               * to be sure we are copying the data from the right shared page.
 -               */
                for_each_sg(s->sg, sg, nseg, i) {
                        BUG_ON(sg->offset + sg->length > PAGE_SIZE);
                        shared_data = kmap_atomic(
                                 * Add the used indirect page back to the list of
                                 * available pages for indirect grefs.
                                 */
-                               indirect_page = pfn_to_page(s->indirect_grants[i]->pfn);
-                               list_add(&indirect_page->lru, &info->indirect_pages);
+                               if (!info->feature_persistent) {
+                                       indirect_page = pfn_to_page(s->indirect_grants[i]->pfn);
+                                       list_add(&indirect_page->lru, &info->indirect_pages);
+                               }
                                s->indirect_grants[i]->gref = GRANT_INVALID_REF;
                                list_add_tail(&s->indirect_grants[i]->node, &info->grants);
                        }
@@@ -1519,7 -1528,7 +1522,7 @@@ static int blkif_recover(struct blkfron
        info->shadow_free = info->ring.req_prod_pvt;
        info->shadow[BLK_RING_SIZE(info)-1].req.u.rw.id = 0x0fffffff;
  
-       rc = blkfront_setup_indirect(info);
+       rc = blkfront_gather_backend_features(info);
        if (rc) {
                kfree(copy);
                return rc;
@@@ -1720,20 -1729,13 +1723,13 @@@ static void blkfront_setup_discard(stru
  
  static int blkfront_setup_indirect(struct blkfront_info *info)
  {
-       unsigned int indirect_segments, segs;
+       unsigned int segs;
        int err, i;
  
-       err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
-                           "feature-max-indirect-segments", "%u", &indirect_segments,
-                           NULL);
-       if (err) {
-               info->max_indirect_segments = 0;
+       if (info->max_indirect_segments == 0)
                segs = BLKIF_MAX_SEGMENTS_PER_REQUEST;
-       } else {
-               info->max_indirect_segments = min(indirect_segments,
-                                                 xen_blkif_max_segments);
+       else
                segs = info->max_indirect_segments;
-       }
  
        err = fill_grant_buffer(info, (segs + INDIRECT_GREFS(segs)) * BLK_RING_SIZE(info));
        if (err)
@@@ -1796,6 -1798,68 +1792,68 @@@ out_of_memory
        return -ENOMEM;
  }
  
+ /*
+  * Gather all backend feature-*
+  */
+ static int blkfront_gather_backend_features(struct blkfront_info *info)
+ {
+       int err;
+       int barrier, flush, discard, persistent;
+       unsigned int indirect_segments;
+       info->feature_flush = 0;
+       err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
+                       "feature-barrier", "%d", &barrier,
+                       NULL);
+       /*
+        * If there's no "feature-barrier" defined, then it means
+        * we're dealing with a very old backend which writes
+        * synchronously; nothing to do.
+        *
+        * If there are barriers, then we use flush.
+        */
+       if (!err && barrier)
+               info->feature_flush = REQ_FLUSH | REQ_FUA;
+       /*
+        * And if there is "feature-flush-cache" use that above
+        * barriers.
+        */
+       err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
+                       "feature-flush-cache", "%d", &flush,
+                       NULL);
+       if (!err && flush)
+               info->feature_flush = REQ_FLUSH;
+       err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
+                       "feature-discard", "%d", &discard,
+                       NULL);
+       if (!err && discard)
+               blkfront_setup_discard(info);
+       err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
+                       "feature-persistent", "%u", &persistent,
+                       NULL);
+       if (err)
+               info->feature_persistent = 0;
+       else
+               info->feature_persistent = persistent;
+       err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
+                           "feature-max-indirect-segments", "%u", &indirect_segments,
+                           NULL);
+       if (err)
+               info->max_indirect_segments = 0;
+       else
+               info->max_indirect_segments = min(indirect_segments,
+                                                 xen_blkif_max_segments);
+       return blkfront_setup_indirect(info);
+ }
  /*
   * Invoked when the backend is finally 'ready' (and has told produced
   * the details about the physical device - #sectors, size, etc).
@@@ -1807,7 -1871,6 +1865,6 @@@ static void blkfront_connect(struct blk
        unsigned int physical_sector_size;
        unsigned int binfo;
        int err;
-       int barrier, flush, discard, persistent;
  
        switch (info->connected) {
        case BLKIF_STATE_CONNECTED:
        if (err != 1)
                physical_sector_size = sector_size;
  
-       info->feature_flush = 0;
-       err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
-                           "feature-barrier", "%d", &barrier,
-                           NULL);
-       /*
-        * If there's no "feature-barrier" defined, then it means
-        * we're dealing with a very old backend which writes
-        * synchronously; nothing to do.
-        *
-        * If there are barriers, then we use flush.
-        */
-       if (!err && barrier)
-               info->feature_flush = REQ_FLUSH | REQ_FUA;
-       /*
-        * And if there is "feature-flush-cache" use that above
-        * barriers.
-        */
-       err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
-                           "feature-flush-cache", "%d", &flush,
-                           NULL);
-       if (!err && flush)
-               info->feature_flush = REQ_FLUSH;
-       err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
-                           "feature-discard", "%d", &discard,
-                           NULL);
-       if (!err && discard)
-               blkfront_setup_discard(info);
-       err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
-                           "feature-persistent", "%u", &persistent,
-                           NULL);
-       if (err)
-               info->feature_persistent = 0;
-       else
-               info->feature_persistent = persistent;
-       err = blkfront_setup_indirect(info);
+       err = blkfront_gather_backend_features(info);
        if (err) {
                xenbus_dev_fatal(info->xbdev, err, "setup_indirect at %s",
                                 info->xbdev->otherend);