From patchwork Thu Aug 22 22:06:48 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Howells X-Patchwork-Id: 13774267 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id D683EC3DA4A for ; Thu, 22 Aug 2024 22:07:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6685F80069; Thu, 22 Aug 2024 18:07:14 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5F0C28005A; Thu, 22 Aug 2024 18:07:14 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 41E4880069; Thu, 22 Aug 2024 18:07:14 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 1C5C18005A for ; Thu, 22 Aug 2024 18:07:14 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 9A77D41AED for ; Thu, 22 Aug 2024 22:07:13 +0000 (UTC) X-FDA: 82481267946.19.77AF4BC Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf08.hostedemail.com (Postfix) with ESMTP id CE5F816000E for ; Thu, 22 Aug 2024 22:07:10 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=Q92o6JHn; spf=pass (imf08.hostedemail.com: domain of dhowells@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=dhowells@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1724364366; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=OFJyiLxnfmQ2OqbyJZkssrtZt5imQFY02UGhZZvMd3E=; b=KZkKtfJ/fudz/mok0PSqH1p27QAUPtiZA9BI8On/mC53jy23C2ww/eJA7pO051dnr8OAhW 7egk+sOos2Xt1ibobLbeqlZSgXdIjDjsdE9wv6lwAuCZYppLICjGDvaLQpI0QaqoE7HXfZ dhnGpgYFI/SPQ0GtyZ/xtECT3CvTtB0= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=Q92o6JHn; spf=pass (imf08.hostedemail.com: domain of dhowells@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=dhowells@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1724364366; a=rsa-sha256; cv=none; b=NOpXm7j7ftILDmnyqZg7IRoO7QWkZ2i+Ju/FlqMzgaEZpGglUI0cot/zz1oCKLzCwcp3aR V5OcWk+HPPMrkO9d+W62wcRY9Ve0u+Y2yEddx3it2sjis7v2X6M1t8KN/VrulEoD+S4QuP SzfCh8DdW2ueVgNCjINolrJ351/9S/c= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1724364430; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=OFJyiLxnfmQ2OqbyJZkssrtZt5imQFY02UGhZZvMd3E=; b=Q92o6JHn2nmqhF1FXs/F9+XxkuKvkxPpPr5i9u3jmNynhhyu1Dm16fZx+hsXK/xyQKLOLb fbM7JNNzNELVKp7VrFWaiwSnzwnk7HyVAWf8TSOINcOEfQ5/Iw3uZRShwbgmvat66GBqJl p0VtKUdyCixA6jHbn+lO3kX9X3leYFo= Received: from mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-135-HQAqW0I7OYm7uNn-XQRs4g-1; Thu, 22 Aug 2024 18:07:08 -0400 X-MC-Unique: HQAqW0I7OYm7uNn-XQRs4g-1 Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 7A7BE1955D48; Thu, 22 Aug 2024 22:07:05 +0000 (UTC) Received: from warthog.procyon.org.uk.com (unknown [10.42.28.30]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 0C2BB19560A3; Thu, 22 Aug 2024 22:06:59 +0000 (UTC) From: David Howells To: Christian Brauner Cc: David Howells , Pankaj Raghav , Jeff Layton , Matthew Wilcox , netfs@lists.linux.dev, linux-afs@lists.infradead.org, linux-cifs@vger.kernel.org, linux-nfs@vger.kernel.org, ceph-devel@vger.kernel.org, v9fs@lists.linux.dev, linux-erofs@lists.ozlabs.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Steve French , Paulo Alcantara Subject: [PATCH 1/2] cifs: Fix lack of credit renegotiation on read retry Date: Thu, 22 Aug 2024 23:06:48 +0100 Message-ID: <20240822220650.318774-2-dhowells@redhat.com> In-Reply-To: <20240822220650.318774-1-dhowells@redhat.com> References: <20240822220650.318774-1-dhowells@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 X-Stat-Signature: h48u6wai7k85ie5s73biz1ms7wj1pwu8 X-Rspam-User: X-Rspamd-Queue-Id: CE5F816000E X-Rspamd-Server: rspam02 X-HE-Tag: 1724364430-870287 X-HE-Meta: U2FsdGVkX1/cw0twKxogrQduKoJJRK3hOHAHHoQiIoccQQOeGOLmB/8xhnOMmumiq67Lmr18sf5/QMfY1rC2cKFBqvMF6F+N0UDx0yYYujhTS0bQ1JTbFVM6qqoz+K5XzqAfnorPkVh2u7ONIGMv6KONd23Q9tlJqY2zagzIjN4ODJTBV+aa36mstFxv/kwB8KESLtFz1oHYOpUgfVham07/TVU3An1vZmBZwewF+6VQduxZLPEKoqADpAljzM47W54Axu9JzKzPRoyeVhiM2Vuifw5LGxd+IXD6xw4+23XnCaNzz9/QHVRYmU5/SXezrczHu5m5gn4pBxj5y0cETqfHLpqPaPk9OIh3TZT1I7n2+Ahf41RY+R/0Q9QPuikV8oED5hgj6oe/AKhe9IKYaKJMI3GYcAQ6Sc5vRzbGqqoL6pn0iqNkLR68f8Dew4NMwboEWMCxXxjGSOBzKON7V2jG+797Q5PXDkQOn6ZZ3l/RSNu+QqsCU7dopGZcNtHsrpxXVZs1cusQ2By0+cc83qY6dGWxvKRw+Bt/Ci2JxJnvBeBPBmrMnQtpg/MNWoiLr0X2QBrTMamPMgMJ5kVQR2eowpqwsdi8zEqcas5/tkfew2cnNe1yarVv5oicE2Yo04rKnyiIfZStTEcZABBJSg0SdrX2+iHbtOnnMvmFmMxRXXgBbTkgI3G405xIyWF1vGRmK8aklw0HYeMobqwaPX4J+faNyxedNy6/YTPlr/Dl6jRtR1r/KEYZmT3cU5UKXKC0EXJChQFaZnf/J5DimxGljtVfTOUeRLy6PLfGern5lF3grHTO2fD/llPLKuQ25NphGEu8I7DV5yHx22rs79SsgLwvJBew/d6cbvEWXQH1hhqbHAp5PJS33B4sT1amPVgqr4krWWnhrQ+iFM1maGfs1x7rbVMySsVdCo6CuMpN7whRVY/RYQ/yzix1h4n2h58IDbShS8dmKZHqEEf F177RvmG MV4Y1pfK7bB6y/ENiuhX8xA6px5sSnysbNrSXI09lKbs++SC/XIdWoi/M94sYEh8PfqxEeC8Ftoe/j7xkZJxZYMQugtD9jzErohVkBS5x0gvf4ZRSgrd9X43hOvEWSezrtrnYGazehdUbIBgXl3i+9CAhxlynSh7hFZpw0LAOxbwcgYdovc5t3qT/zecqAHTWiLYOOZxtnvLFE6oK2iodZLKgoWeJORKIhBQj4BHiJcUu7f8Fgt14lfI9yiCT9icgQMbUkJS9yxxtXmmhAyuJVyfRgdpTF/lt4jDkAS6rswLURCGqkm67T6uyyo2GcX+XGsdquuJdRn6prRVJHAMAA0V7IgzxcUEccl11H44kKYDNRevA/0r9gGvv02TWnv6burPbUC91KBA1MRVZBYm0+FoN/FEA/Y+LG5MDhslWLcriXlclj+s4GHAe5+TrwGCkEHiFnKPfaJk4yLHDE8ws/HamtWyygj1wHGor X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: When netfslib asks cifs to issue a read operation, it prefaces this with a call to ->clamp_length() which cifs uses to negotiate credits, providing receive capacity on the server; however, in the event that a read op needs reissuing, netfslib doesn't call ->clamp_length() again as that could shorten the subrequest, leaving a gap. This causes the retried read to be done with zero credits which causes the server to reject it with STATUS_INVALID_PARAMETER. This is a problem for a DIO read that is requested that would go over the EOF. The short read will be retried, causing EINVAL to be returned to the user when it fails. Fix this by making cifs_req_issue_read() negotiate new credits if retrying (NETFS_SREQ_RETRYING now gets set in the read side as well as the write side in this instance). This isn't sufficient, however: the new credits might not be sufficient to complete the remainder of the read, so also add an additional field, rreq->actual_len, that holds the actual size of the op we want to perform without having to alter subreq->len. We then rely on repeated short reads being retried until we finish the read or reach the end of file and make a zero-length read. Also fix a couple of places where the subrequest start and length need to be altered by the amount so far transferred when being used. Fixes: 69c3c023af25 ("cifs: Implement netfslib hooks") Signed-off-by: David Howells cc: Steve French cc: Paulo Alcantara cc: Jeff Layton cc: linux-cifs@vger.kernel.org cc: netfs@lists.linux.dev cc: linux-fsdevel@vger.kernel.org --- fs/netfs/io.c | 2 ++ fs/smb/client/cifsglob.h | 1 + fs/smb/client/file.c | 28 ++++++++++++++++++++++++---- fs/smb/client/smb2pdu.c | 28 +++++++++++++++++----------- 4 files changed, 44 insertions(+), 15 deletions(-) diff --git a/fs/netfs/io.c b/fs/netfs/io.c index 5367caf3fa28..a72bc01b9609 100644 --- a/fs/netfs/io.c +++ b/fs/netfs/io.c @@ -306,6 +306,7 @@ static bool netfs_rreq_perform_resubmissions(struct netfs_io_request *rreq) break; subreq->source = NETFS_DOWNLOAD_FROM_SERVER; subreq->error = 0; + __set_bit(NETFS_SREQ_RETRYING, &subreq->flags); netfs_stat(&netfs_n_rh_download_instead); trace_netfs_sreq(subreq, netfs_sreq_trace_download_instead); netfs_get_subrequest(subreq, netfs_sreq_trace_get_resubmit); @@ -313,6 +314,7 @@ static bool netfs_rreq_perform_resubmissions(struct netfs_io_request *rreq) netfs_reset_subreq_iter(rreq, subreq); netfs_read_from_server(rreq, subreq); } else if (test_bit(NETFS_SREQ_SHORT_IO, &subreq->flags)) { + __set_bit(NETFS_SREQ_RETRYING, &subreq->flags); netfs_rreq_short_read(rreq, subreq); } } diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h index 5c9b3e6cd95f..ffd8373bb4b1 100644 --- a/fs/smb/client/cifsglob.h +++ b/fs/smb/client/cifsglob.h @@ -1486,6 +1486,7 @@ struct cifs_io_subrequest { struct cifs_io_request *req; }; ssize_t got_bytes; + size_t actual_len; unsigned int xid; int result; bool have_xid; diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c index 1fc66bcf49eb..493c16e7c4ab 100644 --- a/fs/smb/client/file.c +++ b/fs/smb/client/file.c @@ -153,7 +153,7 @@ static bool cifs_clamp_length(struct netfs_io_subrequest *subreq) struct cifs_io_request *req = container_of(subreq->rreq, struct cifs_io_request, rreq); struct TCP_Server_Info *server = req->server; struct cifs_sb_info *cifs_sb = CIFS_SB(rreq->inode->i_sb); - size_t rsize = 0; + size_t rsize; int rc; rdata->xid = get_xid(); @@ -166,8 +166,8 @@ static bool cifs_clamp_length(struct netfs_io_subrequest *subreq) cifs_sb->ctx); - rc = server->ops->wait_mtu_credits(server, cifs_sb->ctx->rsize, &rsize, - &rdata->credits); + rc = server->ops->wait_mtu_credits(server, cifs_sb->ctx->rsize, + &rsize, &rdata->credits); if (rc) { subreq->error = rc; return false; @@ -183,7 +183,8 @@ static bool cifs_clamp_length(struct netfs_io_subrequest *subreq) server->credits, server->in_flight, 0, cifs_trace_rw_credits_read_submit); - subreq->len = min_t(size_t, subreq->len, rsize); + subreq->len = umin(subreq->len, rsize); + rdata->actual_len = subreq->len; #ifdef CONFIG_CIFS_SMB_DIRECT if (server->smbd_conn) @@ -203,12 +204,31 @@ static void cifs_req_issue_read(struct netfs_io_subrequest *subreq) struct netfs_io_request *rreq = subreq->rreq; struct cifs_io_subrequest *rdata = container_of(subreq, struct cifs_io_subrequest, subreq); struct cifs_io_request *req = container_of(subreq->rreq, struct cifs_io_request, rreq); + struct TCP_Server_Info *server = req->server; + struct cifs_sb_info *cifs_sb = CIFS_SB(rreq->inode->i_sb); int rc = 0; cifs_dbg(FYI, "%s: op=%08x[%x] mapping=%p len=%zu/%zu\n", __func__, rreq->debug_id, subreq->debug_index, rreq->mapping, subreq->transferred, subreq->len); + if (test_bit(NETFS_SREQ_RETRYING, &subreq->flags)) { + /* + * As we're issuing a retry, we need to negotiate some new + * credits otherwise the server may reject the op with + * INVALID_PARAMETER. Note, however, we may get back less + * credit than we need to complete the op, in which case, we + * shorten the op and rely on additional rounds of retry. + */ + size_t rsize = umin(subreq->len - subreq->transferred, + cifs_sb->ctx->rsize); + + rc = server->ops->wait_mtu_credits(server, rsize, &rdata->actual_len, + &rdata->credits); + if (rc) + goto out; + } + if (req->cfile->invalidHandle) { do { rc = cifs_reopen_file(req->cfile, true); diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c index 83facb54276a..d80107d1ba9e 100644 --- a/fs/smb/client/smb2pdu.c +++ b/fs/smb/client/smb2pdu.c @@ -4530,9 +4530,9 @@ smb2_readv_callback(struct mid_q_entry *mid) "rdata server %p != mid server %p", rdata->server, mid->server); - cifs_dbg(FYI, "%s: mid=%llu state=%d result=%d bytes=%zu\n", + cifs_dbg(FYI, "%s: mid=%llu state=%d result=%d bytes=%zu/%zu\n", __func__, mid->mid, mid->mid_state, rdata->result, - rdata->subreq.len); + rdata->actual_len, rdata->subreq.len - rdata->subreq.transferred); switch (mid->mid_state) { case MID_RESPONSE_RECEIVED: @@ -4586,15 +4586,18 @@ smb2_readv_callback(struct mid_q_entry *mid) rdata->subreq.debug_index, rdata->xid, rdata->req->cfile->fid.persistent_fid, - tcon->tid, tcon->ses->Suid, rdata->subreq.start, - rdata->subreq.len, rdata->result); + tcon->tid, tcon->ses->Suid, + rdata->subreq.start + rdata->subreq.transferred, + rdata->actual_len, + rdata->result); } else trace_smb3_read_done(rdata->rreq->debug_id, rdata->subreq.debug_index, rdata->xid, rdata->req->cfile->fid.persistent_fid, tcon->tid, tcon->ses->Suid, - rdata->subreq.start, rdata->got_bytes); + rdata->subreq.start + rdata->subreq.transferred, + rdata->got_bytes); if (rdata->result == -ENODATA) { /* We may have got an EOF error because fallocate @@ -4622,6 +4625,7 @@ smb2_async_readv(struct cifs_io_subrequest *rdata) { int rc, flags = 0; char *buf; + struct netfs_io_subrequest *subreq = &rdata->subreq; struct smb2_hdr *shdr; struct cifs_io_parms io_parms; struct smb_rqst rqst = { .rq_iov = rdata->iov, @@ -4632,15 +4636,15 @@ smb2_async_readv(struct cifs_io_subrequest *rdata) int credit_request; cifs_dbg(FYI, "%s: offset=%llu bytes=%zu\n", - __func__, rdata->subreq.start, rdata->subreq.len); + __func__, subreq->start, subreq->len); if (!rdata->server) rdata->server = cifs_pick_channel(tcon->ses); io_parms.tcon = tlink_tcon(rdata->req->cfile->tlink); io_parms.server = server = rdata->server; - io_parms.offset = rdata->subreq.start; - io_parms.length = rdata->subreq.len; + io_parms.offset = subreq->start + subreq->transferred; + io_parms.length = rdata->actual_len; io_parms.persistent_fid = rdata->req->cfile->fid.persistent_fid; io_parms.volatile_fid = rdata->req->cfile->fid.volatile_fid; io_parms.pid = rdata->req->pid; @@ -4655,11 +4659,13 @@ smb2_async_readv(struct cifs_io_subrequest *rdata) rdata->iov[0].iov_base = buf; rdata->iov[0].iov_len = total_len; + rdata->got_bytes = 0; + rdata->result = 0; shdr = (struct smb2_hdr *)buf; if (rdata->credits.value > 0) { - shdr->CreditCharge = cpu_to_le16(DIV_ROUND_UP(rdata->subreq.len, + shdr->CreditCharge = cpu_to_le16(DIV_ROUND_UP(rdata->actual_len, SMB2_MAX_BUFFER_SIZE)); credit_request = le16_to_cpu(shdr->CreditCharge) + 8; if (server->credits >= server->max_credits) @@ -4683,11 +4689,11 @@ smb2_async_readv(struct cifs_io_subrequest *rdata) if (rc) { cifs_stats_fail_inc(io_parms.tcon, SMB2_READ_HE); trace_smb3_read_err(rdata->rreq->debug_id, - rdata->subreq.debug_index, + subreq->debug_index, rdata->xid, io_parms.persistent_fid, io_parms.tcon->tid, io_parms.tcon->ses->Suid, - io_parms.offset, io_parms.length, rc); + io_parms.offset, rdata->actual_len, rc); } async_readv_out: From patchwork Thu Aug 22 22:06:49 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Howells X-Patchwork-Id: 13774268 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8399FC3DA4A for ; Thu, 22 Aug 2024 22:07:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 111E96B01B3; Thu, 22 Aug 2024 18:07:21 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 09C0F6B0202; Thu, 22 Aug 2024 18:07:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E2F736B01FB; Thu, 22 Aug 2024 18:07:20 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id B49C76B01A6 for ; Thu, 22 Aug 2024 18:07:20 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 698681A1A21 for ; Thu, 22 Aug 2024 22:07:20 +0000 (UTC) X-FDA: 82481268240.02.DFAD4B4 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf25.hostedemail.com (Postfix) with ESMTP id A071EA0016 for ; Thu, 22 Aug 2024 22:07:18 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=W6VcskzZ; spf=pass (imf25.hostedemail.com: domain of dhowells@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=dhowells@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1724364421; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=K4cMR1cPV7JXYWx72h4PebZI2yNIfo6unToml+bPSHM=; b=edKqp8hBmVnC2E5wKV3nPvS4mj9xr/0a7IlGOtjxDJh1SNtGs2uFykHSFwwN5f7m9G4NdZ ZkUOdiqyknEotM/TFHGMA/6yv1pFTbRwRBzhvJXKMF6XGEgUIH2Gtb9C7s1FIvsxNwVay1 IJLwC9YZYntPPE98aNhUlpgjgdwYeKY= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=W6VcskzZ; spf=pass (imf25.hostedemail.com: domain of dhowells@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=dhowells@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1724364421; a=rsa-sha256; cv=none; b=MVsYE8sOrqj+dFA47h5Cpxnv0UcuxdeLQOdE3fooQ2eBFOyDHTIt2pzS22vQRnK9Dkeqfk 629Xne9Ho1bY3qy0XpPg1Y9sN/8gBrhYjICwvgiZL9vTMoAYqqh14JAYwaT7WL5ot9iBBh 3Z017v01Ttkf9JCO7coKeRsdYFGiP3k= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1724364437; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=K4cMR1cPV7JXYWx72h4PebZI2yNIfo6unToml+bPSHM=; b=W6VcskzZywatLjfQVtnUtwtKo1JnlTpIVgZyUHEcx3UQXrQg+VHq0DH6w2CpDY3N7xFsKF V7EpTCMLK5Xz2rmMlE+TFaBnMUVA5V+IXX49YBY9aEX/fcfoUj3NCY7iRQ3dR9N1XyGZ6L 4EcmMyuzUVB9BK42AeK27H2xgwP/80s= Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-517-bMIxmBy2PluYI0jC0-NnCQ-1; Thu, 22 Aug 2024 18:07:14 -0400 X-MC-Unique: bMIxmBy2PluYI0jC0-NnCQ-1 Received: from mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.17]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id C8AD61955F40; Thu, 22 Aug 2024 22:07:11 +0000 (UTC) Received: from warthog.procyon.org.uk.com (unknown [10.42.28.30]) by mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id F0C2F19560A3; Thu, 22 Aug 2024 22:07:06 +0000 (UTC) From: David Howells To: Christian Brauner Cc: David Howells , Pankaj Raghav , Jeff Layton , Matthew Wilcox , netfs@lists.linux.dev, linux-afs@lists.infradead.org, linux-cifs@vger.kernel.org, linux-nfs@vger.kernel.org, ceph-devel@vger.kernel.org, v9fs@lists.linux.dev, linux-erofs@lists.ozlabs.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Steve French , Paulo Alcantara Subject: [PATCH 2/2] netfs, cifs: Fix handling of short DIO read Date: Thu, 22 Aug 2024 23:06:49 +0100 Message-ID: <20240822220650.318774-3-dhowells@redhat.com> In-Reply-To: <20240822220650.318774-1-dhowells@redhat.com> References: <20240822220650.318774-1-dhowells@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.17 X-Rspam-User: X-Stat-Signature: zxicw6tm13dkg9zmp7y61n98f8wwdtei X-Rspamd-Queue-Id: A071EA0016 X-Rspamd-Server: rspam11 X-HE-Tag: 1724364438-413544 X-HE-Meta: U2FsdGVkX19tFrnbWjp+zFPEyU9iStYy1gs5WLSwUKg1q3yN77jhM3GZ8DzoWFior6LCwwE0x/h1oMXF3iy6kGEpWP4TNXKkCKB4QmS6hRYZbt4uYDpBOMV2uVcgdrm+P38DOSXPTjTDJEJty49/x5lZp8ILIJAWkzpN+mbLRJv+Naupn0bkOF+7xIFWbGwR5Rm5f4fwlh2N1pJO84GS4aYUe1DooInm42JKKBDVyn3ur4ppxxR1CNbuA3U+BUBJpYeVo5pC23z564FgSi37z7hl2veXUIFIcmjVsk941MfDF4zZsOOuI+Fd7WA+kbPi1lNABeTBmuRe/prjnRMNDL8cEdubSnCWV2LOdn2B+XHIewZWuVZI/bosMmNh+x4pgEFAQ2h87YtGl4FbcMPdzdjmcP1GbJEeIAM2hZasscY7HrAhoklXdzwImL5PPjr2Wc7NgTZITahq/64Zm3+DQS6BO1duefXzBx4eKZSJlEIg65G7S1AlG1h3mqh2eWnS/KGSVMZxCQ1HsE0saoukGpL9JMa+NxX0iY1foa+kao5t3GIZ3rC25/RU1We7eP4XqLg516y2MsugHolaaTuKTs0XQHXriMptMJH0QX5PV5gDXwP9pmebPOFA7bLM2UD+NcSC0Pqfa8c3cxkYhgrykQIqVBvdWLr4VncZzaaDU1is7wDho8xuj2huIw2ZRSNwrjqZdNWV+i9L4jxicXAF9+pE7elPwndkaWryuDv5sVpNUX7Dzvu5PZR4A0oVfgVZRz5OHnl3MnJVLsLRiEjN2aKodDPZu8JtzPzZU8mMBnmzqL2dS29oYCKZoXdmeIO0zEc82ofT79mf8NOtaJABJTbG+X9YXW5rw7AEYyYQpf2QuDBJuIpK9t55fuyghfJyJDc51goiTVdPwjhqXpZFaGEBEv/6neKgyoTlTKGCijC5bHkSEXWTTBggidGVRx2Pzff77GHLKTrJuR8VvxG gy6LCUbo iNPimwkS9ujoSBHRvIZt+xz5KbpVnXICioBeU+uRLa/V7psBm8SDhgSSNep82t4ZNP6aCAAk7o1xytjLI7xZaS8bZnbyH1bGmyhCNvUb/h86f82Q8sPaNywM/rdgsNwi1K51twvxRVGSwyvJ8+JHRgpb8cLX/Fdi+Bm3Z4cWv2t9vdVUlHhmoB7rhNpjLauowilzMGnJIrYF7T0QYFb9eNHIGI2Rbk9qKCOLebbxKkxDow06U2MAcqHanWQMWe1M2NO/18Fu/ousriXOzyU6AenBcx/V7YNnHoN6QWKknVW7zDnN3Phn5WVPN9/mKKUa4Zq9TSmzbsrCvWegeT6Vusa/UHSo/fRNK6kNbV0s/g83amUKtaWW34BYh5FHu0XgY3w6zcFvQ7u67jn4oKKDkNk43vEkGZe06RMX81rkwnzQSd5xVRhYCp4MTQc6ZzxotfXhgd2j14yoeE3opQZeimnpdOb8ajwtM63o2 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Short DIO reads, particularly in relation to cifs, are not being handled correctly by cifs and netfslib. This can be tested by doing a DIO read of a file where the size of read is larger than the size of the file. When it crosses the EOF, it gets a short read and this gets retried, and in the case of cifs, the retry read fails, with the failure being translated to ENODATA. Fix this by the following means: (1) Add a flag, NETFS_SREQ_HIT_EOF, for the filesystem to set when it detects that the read did hit the EOF. (2) Make the netfslib read assessment stop processing subrequests when it encounters one with that flag set. (3) Return rreq->transferred, the accumulated contiguous amount read to that point, to userspace for a DIO read. (4) Make cifs set the flag and clear the error if the read RPC returned ENODATA and the read-to file position is at or after the remote inode size. (5) Make cifs set the flag and clear the error if a short read occurred without error and the read-to file position is now at the remote inode size. Fixes: 69c3c023af25 ("cifs: Implement netfslib hooks") Signed-off-by: David Howells cc: Steve French cc: Paulo Alcantara cc: Jeff Layton cc: linux-cifs@vger.kernel.org cc: netfs@lists.linux.dev cc: linux-fsdevel@vger.kernel.org --- fs/netfs/io.c | 17 +++++++++++------ fs/smb/client/smb2pdu.c | 15 ++++++++++++++- include/linux/netfs.h | 1 + 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/fs/netfs/io.c b/fs/netfs/io.c index a72bc01b9609..d6510a9385dc 100644 --- a/fs/netfs/io.c +++ b/fs/netfs/io.c @@ -367,7 +367,8 @@ static void netfs_rreq_assess_dio(struct netfs_io_request *rreq) if (subreq->error || subreq->transferred == 0) break; transferred += subreq->transferred; - if (subreq->transferred < subreq->len) + if (subreq->transferred < subreq->len || + test_bit(NETFS_SREQ_HIT_EOF, &subreq->flags)) break; } @@ -502,7 +503,8 @@ void netfs_subreq_terminated(struct netfs_io_subrequest *subreq, subreq->error = 0; subreq->transferred += transferred_or_error; - if (subreq->transferred < subreq->len) + if (subreq->transferred < subreq->len && + !test_bit(NETFS_SREQ_HIT_EOF, &subreq->flags)) goto incomplete; complete: @@ -781,10 +783,13 @@ int netfs_begin_read(struct netfs_io_request *rreq, bool sync) TASK_UNINTERRUPTIBLE); ret = rreq->error; - if (ret == 0 && rreq->submitted < rreq->len && - rreq->origin != NETFS_DIO_READ) { - trace_netfs_failure(rreq, NULL, ret, netfs_fail_short_read); - ret = -EIO; + if (ret == 0) { + if (rreq->origin == NETFS_DIO_READ) { + ret = rreq->transferred; + } else if (rreq->submitted < rreq->len) { + trace_netfs_failure(rreq, NULL, ret, netfs_fail_short_read); + ret = -EIO; + } } } else { /* If we decrement nr_outstanding to 0, the ref belongs to us. */ diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c index d80107d1ba9e..e182fdbec887 100644 --- a/fs/smb/client/smb2pdu.c +++ b/fs/smb/client/smb2pdu.c @@ -4507,6 +4507,7 @@ static void smb2_readv_callback(struct mid_q_entry *mid) { struct cifs_io_subrequest *rdata = mid->callback_data; + struct netfs_inode *ictx = netfs_inode(rdata->rreq->inode); struct cifs_tcon *tcon = tlink_tcon(rdata->req->cfile->tlink); struct TCP_Server_Info *server = rdata->server; struct smb2_hdr *shdr = @@ -4603,8 +4604,20 @@ smb2_readv_callback(struct mid_q_entry *mid) /* We may have got an EOF error because fallocate * failed to enlarge the file. */ - if (rdata->subreq.start < rdata->subreq.rreq->i_size) + if (rdata->subreq.start + rdata->subreq.transferred < rdata->subreq.rreq->i_size) rdata->result = 0; + if (rdata->subreq.start + rdata->subreq.transferred + rdata->got_bytes >= + ictx->remote_i_size) { + __set_bit(NETFS_SREQ_HIT_EOF, &rdata->subreq.flags); + rdata->result = 0; + } + } else { + if (rdata->got_bytes < rdata->actual_len && + rdata->subreq.start + rdata->subreq.transferred + rdata->got_bytes == + ictx->remote_i_size) { + __set_bit(NETFS_SREQ_HIT_EOF, &rdata->subreq.flags); + rdata->result = 0; + } } trace_smb3_rw_credits(rreq_debug_id, subreq_debug_index, rdata->credits.value, server->credits, server->in_flight, diff --git a/include/linux/netfs.h b/include/linux/netfs.h index 983816608f15..c47443e7a97e 100644 --- a/include/linux/netfs.h +++ b/include/linux/netfs.h @@ -198,6 +198,7 @@ struct netfs_io_subrequest { #define NETFS_SREQ_NEED_RETRY 9 /* Set if the filesystem requests a retry */ #define NETFS_SREQ_RETRYING 10 /* Set if we're retrying */ #define NETFS_SREQ_FAILED 11 /* Set if the subreq failed unretryably */ +#define NETFS_SREQ_HIT_EOF 12 /* Set if we hit the EOF */ }; enum netfs_io_origin {