| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <57712A4C.7020708@oracle.com>
Date: Mon, 27 Jun 2016 21:29:48 +0800
From: Bob Liu <bob.liu@...cle.com>
To: Bob Liu <bob.liu@...cle.com>
CC: linux-kernel@...r.kernel.org, xen-devel@...ts.xenproject.org,
konrad.wilk@...cle.com, roger.pau@...rix.com
Subject: Re: [PATCH] xen-blkfront: save uncompleted reqs in blkfront_resume()
On 06/27/2016 04:33 PM, Bob Liu wrote:
> Uncompleted reqs used to be 'saved and resubmitted' in blkfront_recover() during
> migration, but that's too later after multi-queue introduced.
>
> After a migrate to another host (which may not have multiqueue support), the
> number of rings (block hardware queues) may be changed and the ring and shadow
> structure will also be reallocated.
> So that blkfront_recover() can't 'save and resubmit' the real uncompleted reqs
> because shadow structure has been reallocated.
>
> This patch fixes this issue by moving the 'save and resubmit' logic out of
Fix: Just moved the 'save' logic to earlier place:blkfront_resume(), the 'resubmit' was no change and still in blkfront_recover().
> blkfront_recover() to earlier place:blkfront_resume().
>
> Signed-off-by: Bob Liu <bob.liu@...cle.com>
> ---
> drivers/block/xen-blkfront.c | 91 +++++++++++++++++++-------------------------
> 1 file changed, 40 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 2e6d1e9..fcc5b4e 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -207,6 +207,9 @@ struct blkfront_info
> struct blk_mq_tag_set tag_set;
> struct blkfront_ring_info *rinfo;
> unsigned int nr_rings;
> + /* Save uncomplete reqs and bios for migration. */
> + struct list_head requests;
> + struct bio_list bio_list;
> };
>
> static unsigned int nr_minors;
> @@ -2002,69 +2005,22 @@ static int blkif_recover(struct blkfront_info *info)
> {
> unsigned int i, r_index;
> struct request *req, *n;
> - struct blk_shadow *copy;
> int rc;
> struct bio *bio, *cloned_bio;
> - struct bio_list bio_list, merge_bio;
> unsigned int segs, offset;
> int pending, size;
> struct split_bio *split_bio;
> - struct list_head requests;
>
> blkfront_gather_backend_features(info);
> segs = info->max_indirect_segments ? : BLKIF_MAX_SEGMENTS_PER_REQUEST;
> blk_queue_max_segments(info->rq, segs);
> - bio_list_init(&bio_list);
> - INIT_LIST_HEAD(&requests);
>
> for (r_index = 0; r_index < info->nr_rings; r_index++) {
> - struct blkfront_ring_info *rinfo;
> -
> - rinfo = &info->rinfo[r_index];
> - /* Stage 1: Make a safe copy of the shadow state. */
> - copy = kmemdup(rinfo->shadow, sizeof(rinfo->shadow),
> - GFP_NOIO | __GFP_REPEAT | __GFP_HIGH);
> - if (!copy)
> - return -ENOMEM;
> -
> - /* Stage 2: Set up free list. */
> - memset(&rinfo->shadow, 0, sizeof(rinfo->shadow));
> - for (i = 0; i < BLK_RING_SIZE(info); i++)
> - rinfo->shadow[i].req.u.rw.id = i+1;
> - rinfo->shadow_free = rinfo->ring.req_prod_pvt;
> - rinfo->shadow[BLK_RING_SIZE(info)-1].req.u.rw.id = 0x0fffffff;
> + struct blkfront_ring_info *rinfo = &info->rinfo[r_index];
>
> rc = blkfront_setup_indirect(rinfo);
> - if (rc) {
> - kfree(copy);
> + if (rc)
> return rc;
> - }
> -
> - for (i = 0; i < BLK_RING_SIZE(info); i++) {
> - /* Not in use? */
> - if (!copy[i].request)
> - continue;
> -
> - /*
> - * Get the bios in the request so we can re-queue them.
> - */
> - if (copy[i].request->cmd_flags &
> - (REQ_FLUSH | REQ_FUA | REQ_DISCARD | REQ_SECURE)) {
> - /*
> - * Flush operations don't contain bios, so
> - * we need to requeue the whole request
> - */
> - list_add(©[i].request->queuelist, &requests);
> - continue;
> - }
> - merge_bio.head = copy[i].request->bio;
> - merge_bio.tail = copy[i].request->biotail;
> - bio_list_merge(&bio_list, &merge_bio);
> - copy[i].request->bio = NULL;
> - blk_end_request_all(copy[i].request, 0);
> - }
> -
> - kfree(copy);
> }
> xenbus_switch_state(info->xbdev, XenbusStateConnected);
>
> @@ -2079,7 +2035,7 @@ static int blkif_recover(struct blkfront_info *info)
> kick_pending_request_queues(rinfo);
> }
>
> - list_for_each_entry_safe(req, n, &requests, queuelist) {
> + list_for_each_entry_safe(req, n, &info->requests, queuelist) {
> /* Requeue pending requests (flush or discard) */
> list_del_init(&req->queuelist);
> BUG_ON(req->nr_phys_segments > segs);
> @@ -2087,7 +2043,7 @@ static int blkif_recover(struct blkfront_info *info)
> }
> blk_mq_kick_requeue_list(info->rq);
>
> - while ((bio = bio_list_pop(&bio_list)) != NULL) {
> + while ((bio = bio_list_pop(&info->bio_list)) != NULL) {
> /* Traverse the list of pending bios and re-queue them */
> if (bio_segments(bio) > segs) {
> /*
> @@ -2133,9 +2089,42 @@ static int blkfront_resume(struct xenbus_device *dev)
> {
> struct blkfront_info *info = dev_get_drvdata(&dev->dev);
> int err = 0;
> + unsigned int i, j;
>
> dev_dbg(&dev->dev, "blkfront_resume: %s\n", dev->nodename);
>
> + bio_list_init(&info->bio_list);
> + INIT_LIST_HEAD(&info->requests);
> + for (i = 0; i < info->nr_rings; i++) {
> + struct blkfront_ring_info *rinfo = &info->rinfo[i];
> + struct bio_list merge_bio;
> + struct blk_shadow *shadow = rinfo->shadow;
> +
> + for (j = 0; j < BLK_RING_SIZE(info); j++) {
> + /* Not in use? */
> + if (!shadow[j].request)
> + continue;
> +
> + /*
> + * Get the bios in the request so we can re-queue them.
> + */
> + if (shadow[j].request->cmd_flags &
> + (REQ_FLUSH | REQ_FUA | REQ_DISCARD | REQ_SECURE)) {
> + /*
> + * Flush operations don't contain bios, so
> + * we need to requeue the whole request
> + */
> + list_add(&shadow[j].request->queuelist, &info->requests);
> + continue;
> + }
> + merge_bio.head = shadow[j].request->bio;
> + merge_bio.tail = shadow[j].request->biotail;
> + bio_list_merge(&info->bio_list, &merge_bio);
> + shadow[j].request->bio = NULL;
> + blk_mq_end_request(shadow[j].request, 0);
> + }
> + }
> +
> blkif_free(info, info->connected == BLKIF_STATE_CONNECTED);
>
> err = negotiate_mq(info);
>
Powered by blists - more mailing lists