From patchwork Wed Nov 27 08:49:09 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rutger ter Borg X-Patchwork-Id: 3246981 Return-Path: X-Original-To: patchwork-ceph-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 54266C045B for ; Wed, 27 Nov 2013 08:49:29 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 573162056F for ; Wed, 27 Nov 2013 08:49:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C27DC2057E for ; Wed, 27 Nov 2013 08:49:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754045Ab3K0ItX (ORCPT ); Wed, 27 Nov 2013 03:49:23 -0500 Received: from plane.gmane.org ([80.91.229.3]:46108 "EHLO plane.gmane.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753947Ab3K0ItW (ORCPT ); Wed, 27 Nov 2013 03:49:22 -0500 Received: from list by plane.gmane.org with local (Exim 4.69) (envelope-from ) id 1VlaoC-00064z-Fw for ceph-devel@vger.kernel.org; Wed, 27 Nov 2013 09:49:20 +0100 Received: from gl-95017.prolocation.net ([62.204.95.17]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Wed, 27 Nov 2013 09:49:20 +0100 Received: from rutger by gl-95017.prolocation.net with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Wed, 27 Nov 2013 09:49:20 +0100 X-Injected-Via-Gmane: http://gmane.org/ To: ceph-devel@vger.kernel.org From: Rutger ter Borg Subject: Re: [PATCH] fix librados aio read buffer handling Date: Wed, 27 Nov 2013 09:49:09 +0100 Lines: 119 Message-ID: References: <524A0098.4060005@inktank.com> Mime-Version: 1.0 X-Complaints-To: usenet@ger.gmane.org X-Gmane-NNTP-Posting-Host: gl-95017.prolocation.net User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20131103 Icedove/17.0.10 In-Reply-To: <524A0098.4060005@inktank.com> Sender: ceph-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 2013-10-01 00:52, Josh Durgin wrote: > > I'm fine applying this now (with one fix). It's a nice cleanup > even if things change more soon. > > For the C interface, the return value stored in the AioCompletionImpl > needs to be the length read, so the caller can tell if a short read > occurred (this is only possible when trying to read past the end of an > object). This was being set in C_aio_Ack::finish(), but was removed by > this patch. Please find attached a revised patch against 0.72.1. I've added a fix for setting the return value in C_aio_Ack::finish(), if (c->bl.length() > 0) { c->rval = c->bl.length(); } Cheers, Rutger diff -u -p ceph-0.72.1/src/librados/AioCompletionImpl.h ceph-0.72.1-patched/src/librados/AioCompletionImpl.h --- ceph-0.72.1/src/librados/AioCompletionImpl.h 2013-11-22 10:15:58.000000000 +0100 +++ ceph-0.72.1-patched/src/librados/AioCompletionImpl.h 2013-11-25 11:32:06.178467454 +0100 @@ -39,8 +39,7 @@ struct librados::AioCompletionImpl { // for read bool is_read; - bufferlist bl, *pbl; - char *buf; + bufferlist bl; unsigned maxlen; IoCtxImpl *io; @@ -54,7 +53,7 @@ struct librados::AioCompletionImpl { callback_safe(0), callback_complete_arg(0), callback_safe_arg(0), - is_read(false), pbl(0), buf(0), maxlen(0), + is_read(false), maxlen(0), io(NULL), aio_write_seq(0), aio_write_list_item(this) { } int set_complete_callback(void *cb_arg, rados_callback_t cb) { diff -u -p ceph-0.72.1/src/librados/IoCtxImpl.cc ceph-0.72.1-patched/src/librados/IoCtxImpl.cc --- ceph-0.72.1/src/librados/IoCtxImpl.cc 2013-11-22 10:15:58.000000000 +0100 +++ ceph-0.72.1-patched/src/librados/IoCtxImpl.cc 2013-11-25 11:40:41.952777192 +0100 @@ -570,7 +570,6 @@ int librados::IoCtxImpl::aio_operate_rea c->is_read = true; c->io = this; - c->pbl = pbl; Mutex::Locker l(*lock); objecter->read(oid, oloc, @@ -612,11 +611,10 @@ int librados::IoCtxImpl::aio_read(const c->is_read = true; c->io = this; - c->pbl = pbl; Mutex::Locker l(*lock); objecter->read(oid, oloc, - off, len, snapid, &c->bl, 0, + off, len, snapid, pbl, 0, onack, &c->objver); return 0; } @@ -632,8 +630,9 @@ int librados::IoCtxImpl::aio_read(const c->is_read = true; c->io = this; - c->buf = buf; c->maxlen = len; + c->bl.clear(); + c->bl.push_back( buffer::create_static( len, buf ) ); Mutex::Locker l(*lock); objecter->read(oid, oloc, @@ -668,7 +667,6 @@ int librados::IoCtxImpl::aio_sparse_read c->is_read = true; c->io = this; - c->pbl = NULL; onack->m_ops.sparse_read(off, len, m, data_bl, NULL); @@ -1179,14 +1177,9 @@ void librados::IoCtxImpl::C_aio_Ack::fin c->safe = true; c->cond.Signal(); - if (c->buf && c->bl.length() > 0) { - unsigned l = MIN(c->bl.length(), c->maxlen); - c->bl.copy(0, l, c->buf); + if (c->bl.length() > 0) { c->rval = c->bl.length(); } - if (c->pbl) { - *c->pbl = c->bl; - } if (c->callback_complete) { c->io->client->finisher.queue(new C_AioComplete(c));