From patchwork Fri Jul 11 09:47:28 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pavel Shilovsky X-Patchwork-Id: 4532451 Return-Path: X-Original-To: patchwork-cifs-client@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 10666BEEAA for ; Fri, 11 Jul 2014 09:47:33 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 8E2B220173 for ; Fri, 11 Jul 2014 09:47:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3561C2015E for ; Fri, 11 Jul 2014 09:47:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751791AbaGKJr3 (ORCPT ); Fri, 11 Jul 2014 05:47:29 -0400 Received: from mail-pd0-f180.google.com ([209.85.192.180]:42638 "EHLO mail-pd0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751469AbaGKJr2 (ORCPT ); Fri, 11 Jul 2014 05:47:28 -0400 Received: by mail-pd0-f180.google.com with SMTP id fp1so1132319pdb.11 for ; Fri, 11 Jul 2014 02:47:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:content-type; bh=8fHW3AqhU/2x4qMvkY//i21hZdXof0lMcPkVDPKWxfU=; b=A8ER3NvrpZhsj3qfLuhd5DwyCURaPsKrU5+XcbNEr1rK5wYK+XIzSnxWfMqRuV6X9/ cHMhx0xQP/YJdMxcbcLre4Q1oKJXg6t7MqntoBxonYcm2ql7IxnFGLeyVGje/hddWRw8 AerNwuKWyd3DqTSaVLqswuURcJZ9Z8bJDy8mJ3J/JkbZstlaJsPyMjeavIXprhTosZOv N8zQZGF35AgSk4aLc7YVhgZfZc9yLpke86QSfin0b7aCIeTPQcRrXMYxrwceXwlGq+ju 6CU7iOSgRAY3h9BSE+VZgJLu3EYq2RkjiqVyetUH21je66eoZPm/GvVUwe5+5QsqylnU HTKw== MIME-Version: 1.0 X-Received: by 10.70.33.207 with SMTP id t15mr23154888pdi.12.1405072048341; Fri, 11 Jul 2014 02:47:28 -0700 (PDT) Received: by 10.70.90.136 with HTTP; Fri, 11 Jul 2014 02:47:28 -0700 (PDT) In-Reply-To: References: <1404984647-8710-1-git-send-email-pshilovsky@samba.org> <1404984647-8710-7-git-send-email-pshilovsky@samba.org> Date: Fri, 11 Jul 2014 13:47:28 +0400 X-Google-Sender-Auth: ghLpyR9U15VXJUqDHQ53_A09YSo Message-ID: Subject: Re: [PATCH 6/7] CIFS: Optimize cifs_user_read() in a short read case on reconnects From: Pavel Shilovsky To: linux-cifs Sender: linux-cifs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,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 2014-07-10 13:40 GMT+04:00 Pavel Shilovsky : > 2014-07-10 13:30 GMT+04:00 Pavel Shilovsky : >> by filling the output buffer with a data got from a partially received >> response and requesting the remaining data from the server. >> >> Signed-off-by: Pavel Shilovsky >> --- >> fs/cifs/file.c | 23 ++++++++++++++++++++--- >> 1 file changed, 20 insertions(+), 3 deletions(-) >> >> diff --git a/fs/cifs/file.c b/fs/cifs/file.c >> index 6896cb5..d7246cb 100644 >> --- a/fs/cifs/file.c >> +++ b/fs/cifs/file.c >> @@ -3029,13 +3029,30 @@ again: >> else if (rdata->result == -EAGAIN) { >> /* resend call if it's a retryable error */ >> struct list_head tmp_list; >> + unsigned int got_bytes = rdata->got_bytes; >> >> list_del_init(&rdata->list); >> INIT_LIST_HEAD(&tmp_list); >> >> - rc = cifs_send_async_read(rdata->offset, >> - rdata->bytes, rdata->cfile, >> - cifs_sb, &tmp_list); >> + /* >> + * Got a part of data and then reconnect has >> + * happened -- discard anything left and return >> + * a short read. >> + */ > ^^^ > Ah, wrong version of the comment is posted. Sorry. The right one: > + /* > + * Got a part of data and then reconnect has > + * happened -- fill the buffer and continue > + * reading. > + */ > > will repost this after a feedback. > >> + if (got_bytes && got_bytes < rdata->bytes) { >> + rc = cifs_readdata_to_iov(rdata, to); >> + if (rc) { >> + kref_put(&rdata->refcount, >> + cifs_uncached_readdata_release); >> + continue; >> + } >> + } >> + >> + rc = cifs_send_async_read( >> + rdata->offset + got_bytes, >> + rdata->bytes - got_bytes, >> + rdata->cfile, cifs_sb, >> + &tmp_list); >> >> list_splice(&tmp_list, &rdata_list); >> Also realized that the patch doesn't aware about signed connections and doesn't take into account that we should update stats in "EAGAIN && got_bytes" case. So, this diff should be appended to the patch as well: --- --- Thoughts? diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index 9f13b62..7d4361f 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -1561,6 +1561,12 @@ cifs_readv_callback(struct mid_q_entry *mid) case MID_REQUEST_SUBMITTED: case MID_RETRY_NEEDED: rdata->result = -EAGAIN; + if (server->sign && rdata->got_bytes) + /* reset bytes number since we can not check a sign */ + rdata->got_bytes = 0; + /* FIXME: should this be counted toward the initiating task? */ + task_io_account_read(rdata->got_bytes); + cifs_stats_bytes_read(tcon, rdata->got_bytes); break; default: rdata->result = -EIO; diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 17c0c31..768cddb 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -1730,6 +1730,12 @@ smb2_readv_callback(struct mid_q_entry *mid) case MID_REQUEST_SUBMITTED: case MID_RETRY_NEEDED: rdata->result = -EAGAIN; + if (server->sign && rdata->got_bytes) + /* reset bytes number since we can not check a sign */ + rdata->got_bytes = 0; + /* FIXME: should this be counted toward the initiating task? */ + task_io_account_read(rdata->got_bytes); + cifs_stats_bytes_read(tcon, rdata->got_bytes); break; default: if (rdata->result != -ENODATA)