]> git.dujemihanovic.xyz Git - linux.git/commitdiff
netfs, cifs: Fix handling of short DIO read
authorDavid Howells <dhowells@redhat.com>
Thu, 22 Aug 2024 22:06:49 +0000 (23:06 +0100)
committerSteve French <stfrench@microsoft.com>
Wed, 28 Aug 2024 12:47:36 +0000 (07:47 -0500)
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.

 (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 <dhowells@redhat.com>
cc: Steve French <sfrench@samba.org>
cc: Paulo Alcantara <pc@manguebit.com>
cc: Jeff Layton <jlayton@kernel.org>
cc: linux-cifs@vger.kernel.org
cc: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Steve French <stfrench@microsoft.com>
fs/netfs/io.c
fs/smb/client/smb2pdu.c
include/linux/netfs.h

index 3303b515b5363563861c082872626051e89a70af..943128507af550f36a9d90a0e245e2e810ce555f 100644 (file)
@@ -368,7 +368,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;
        }
 
@@ -503,7 +504,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:
@@ -782,10 +784,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. */
index be7a1a9c691da60d3a0e7c361a4cc14510e063bf..88dc49d670371b0d9ceab9abd9fdb3aca823127e 100644 (file)
@@ -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 =
@@ -4599,11 +4600,15 @@ smb2_readv_callback(struct mid_q_entry *mid)
                                     rdata->got_bytes);
 
        if (rdata->result == -ENODATA) {
-               /* We may have got an EOF error because fallocate
-                * failed to enlarge the file.
-                */
-               if (rdata->subreq.start < rdata->subreq.rreq->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,
index 983816608f15d7b3c3dc0513bbcc0490f7c45c77..c47443e7a97efd8f7fcc8d01f0cddab187337030 100644 (file)
@@ -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 {