From patchwork Mon May 13 17:58:41 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alex Elder X-Patchwork-Id: 2559711 Return-Path: X-Original-To: patchwork-ceph-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id AF24CDF2E5 for ; Mon, 13 May 2013 17:58:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753439Ab3EMR6o (ORCPT ); Mon, 13 May 2013 13:58:44 -0400 Received: from mail-oa0-f51.google.com ([209.85.219.51]:34590 "EHLO mail-oa0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752121Ab3EMR6o (ORCPT ); Mon, 13 May 2013 13:58:44 -0400 Received: by mail-oa0-f51.google.com with SMTP id f4so7831618oah.24 for ; Mon, 13 May 2013 10:58:43 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:message-id:date:from:user-agent:mime-version:to:cc :subject:references:in-reply-to:content-type :content-transfer-encoding:x-gm-message-state; bh=hcsc5qyJ/Ot5xDx40MYwcw5N+YdcNyqkwgnFhhMOXx8=; b=nvSPOm9eKS4U7+GK8l1Qpf55MbxSZuOkp/fE/JLjyyNkItK12okieJcBrsnEYiOOEG GJmH6huKD94J/7wARUea6J6eGKU96RqJkYHLiMmXlJa1R7QgBS7B9QkXx/Cxbslo5fec 3/y6Rj3qsxkap5qKhv+sB6Krf1od2EJcQ4VXGk6gFJgmWt9JfoKNQH2z8lgR54LXxbjl 0Rhkagw5ovUSKCGuUaDmaFBCzgU0YvJ17wi+8d+MyFTIhA9CY7MB0I3JWs7IHfPNJStu OiT8oHEctvORzDfNRtKImjsATdGXH6pRUpkJWHWsKyvec2utX5hxu9bDKp4A/JlSOjqy LADg== X-Received: by 10.60.57.201 with SMTP id k9mr13479695oeq.30.1368467923116; Mon, 13 May 2013 10:58:43 -0700 (PDT) Received: from [172.22.22.4] (c-71-195-31-37.hsd1.mn.comcast.net. [71.195.31.37]) by mx.google.com with ESMTPSA id ri8sm17733971oeb.0.2013.05.13.10.58.41 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 13 May 2013 10:58:42 -0700 (PDT) Message-ID: <519129D1.7050807@inktank.com> Date: Mon, 13 May 2013 12:58:41 -0500 From: Alex Elder User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130329 Thunderbird/17.0.5 MIME-Version: 1.0 To: Josh Durgin CC: ceph-devel@vger.kernel.org Subject: [PATCH] rbd: fix parent request size assumption References: <518E81E1.2000806@inktank.com> <518E9B84.20100@inktank.com> In-Reply-To: <518E9B84.20100@inktank.com> X-Gm-Message-State: ALoCoQm8mtxjsJqSV2EzRNFRyYe3ManDOoBEVBHr+Gx1KhMxvPtmjHcep38AgF5Sfcph2LIgrvRa Sender: ceph-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org I have inserted this patch into a series I posted over the weekend to address an issue that Josh's comment raised in the review for: [PATCH] rbd: support reading parent page data for writes This patch, along with the updated series, is available in the "review/wip-flatten-1" branch of the ceph-client git repository. The code that reads object data from the parent for a copyup on write request currently assumes that the size of that request is the size of a "full" object from the original target image. That is not necessarily the case. The parent overlap could reduce the request size below that. To fix that assumption we need to record the number of pages in the copyup_pages array, for both an image request and an object request. Rename a local variable in rbd_img_obj_parent_read_full_callback() to reflect we're recording the length of the parent read request, not the size of the target object. This resolves: http://tracker.ceph.com/issues/5038 Signed-off-by: Alex Elder Reviewed-by: Josh Durgin --- drivers/block/rbd.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) rbd_assert(obj_request->type == OBJ_REQUEST_BIO); @@ -2129,12 +2131,14 @@ rbd_img_obj_copyup_callback(struct rbd_obj_request *obj_request) rbd_dev = img_request->rbd_dev; rbd_assert(rbd_dev); - length = (u64)1 << rbd_dev->header.obj_order; - page_count = (u32)calc_pages_for(0, length); - rbd_assert(obj_request->copyup_pages); - ceph_release_page_vector(obj_request->copyup_pages, page_count); + pages = obj_request->copyup_pages; + rbd_assert(pages != NULL); obj_request->copyup_pages = NULL; + page_count = obj_request->copyup_page_count; + rbd_assert(page_count); + obj_request->copyup_page_count = 0; + ceph_release_page_vector(pages, page_count); /* * We want the transfer count to reflect the size of the @@ -2158,9 +2162,9 @@ rbd_img_obj_parent_read_full_callback(struct rbd_img_request *img_request) struct ceph_osd_client *osdc; struct rbd_device *rbd_dev; struct page **pages; + u32 page_count; int result; - u64 obj_size; - u64 xferred; + u64 parent_length; rbd_assert(img_request_child_test(img_request)); @@ -2169,19 +2173,21 @@ rbd_img_obj_parent_read_full_callback(struct rbd_img_request *img_request) pages = img_request->copyup_pages; rbd_assert(pages != NULL); img_request->copyup_pages = NULL; + page_count = img_request->copyup_page_count; + rbd_assert(page_count); + img_request->copyup_page_count = 0; orig_request = img_request->obj_request; rbd_assert(orig_request != NULL); rbd_assert(orig_request->type == OBJ_REQUEST_BIO); result = img_request->result; - obj_size = img_request->length; - xferred = img_request->xferred; + parent_length = img_request->length; + rbd_assert(parent_length == img_request->xferred); rbd_img_request_put(img_request); rbd_assert(orig_request->img_request); rbd_dev = orig_request->img_request->rbd_dev; rbd_assert(rbd_dev); - rbd_assert(obj_size == (u64)1 << rbd_dev->header.obj_order); if (result) goto out_err; @@ -2195,11 +2201,12 @@ rbd_img_obj_parent_read_full_callback(struct rbd_img_request *img_request) goto out_err; orig_request->osd_req = osd_req; orig_request->copyup_pages = pages; + orig_request->copyup_page_count = page_count; /* Initialize the copyup op */ osd_req_op_cls_init(osd_req, 0, CEPH_OSD_OP_CALL, "rbd", "copyup"); - osd_req_op_cls_request_data_pages(osd_req, 0, pages, obj_size, 0, + osd_req_op_cls_request_data_pages(osd_req, 0, pages, parent_length, 0, false, false); /* Then the original write request op */ @@ -2312,6 +2319,7 @@ static int rbd_img_obj_parent_read_full(struct rbd_obj_request *obj_request) if (result) goto out_err; parent_request->copyup_pages = pages; + parent_request->copyup_page_count = page_count; parent_request->callback = rbd_img_obj_parent_read_full_callback; result = rbd_img_request_submit(parent_request); @@ -2319,6 +2327,7 @@ static int rbd_img_obj_parent_read_full(struct rbd_obj_request *obj_request) return 0; parent_request->copyup_pages = NULL; + parent_request->copyup_page_count = 0; parent_request->obj_request = NULL; rbd_obj_request_put(obj_request); out_err: diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index 51c45e7..597b9bb 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -224,6 +224,7 @@ struct rbd_obj_request { }; }; struct page **copyup_pages; + u32 copyup_page_count; struct ceph_osd_request *osd_req; @@ -256,6 +257,7 @@ struct rbd_img_request { struct rbd_obj_request *obj_request; /* obj req initiator */ }; struct page **copyup_pages; + u32 copyup_page_count; spinlock_t completion_lock;/* protects next_completion */ u32 next_completion; rbd_img_callback_t callback; @@ -2119,7 +2121,7 @@ rbd_img_obj_copyup_callback(struct rbd_obj_request *obj_request) { struct rbd_img_request *img_request; struct rbd_device *rbd_dev; - u64 length; + struct page **pages; u32 page_count;