* [PATCH 6.10 156/375] cifs: Fix lack of credit renegotiation on read retry
[not found] <20240910092622.245959861@linuxfoundation.org>
@ 2024-09-10 9:29 ` Greg Kroah-Hartman
2024-09-10 9:29 ` [PATCH 6.10 157/375] netfs, cifs: Fix handling of short DIO read Greg Kroah-Hartman
` (4 subsequent siblings)
5 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2024-09-10 9:29 UTC (permalink / raw)
To: stable
Cc: Greg Kroah-Hartman, patches, David Howells, Steve French,
Paulo Alcantara, Jeff Layton, linux-cifs, netfs, linux-fsdevel,
Steve French, Sasha Levin
6.10-stable review patch. If anyone has any objections, please let me know.
------------------
From: David Howells <dhowells@redhat.com>
[ Upstream commit 6a5dcd487791e0c2d86622064602a5c7459941ed ]
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 <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>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
fs/netfs/io.c | 2 ++
fs/smb/client/cifsglob.h | 1 +
fs/smb/client/file.c | 37 +++++++++++++++++++++++++++++++++----
fs/smb/client/smb2ops.c | 2 +-
fs/smb/client/smb2pdu.c | 28 +++++++++++++++++-----------
fs/smb/client/trace.h | 1 +
6 files changed, 55 insertions(+), 16 deletions(-)
diff --git a/fs/netfs/io.c b/fs/netfs/io.c
index c96431d3da6d..2a5c22606fb1 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_reset_subreq_iter(rreq, subreq);
netfs_rreq_short_read(rreq, subreq);
}
diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 1e4da268de3b..552792f28122 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -1508,6 +1508,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 b202eac6584e..533f76118316 100644
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -111,6 +111,7 @@ static void cifs_issue_write(struct netfs_io_subrequest *subreq)
goto fail;
}
+ wdata->actual_len = wdata->subreq.len;
rc = adjust_credits(wdata->server, wdata, cifs_trace_rw_credits_issue_write_adjust);
if (rc)
goto fail;
@@ -153,7 +154,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 +167,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 +184,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 +205,39 @@ 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;
+
+ rdata->credits.in_flight_check = 1;
+
+ trace_smb3_rw_credits(rdata->rreq->debug_id,
+ rdata->subreq.debug_index,
+ rdata->credits.value,
+ server->credits, server->in_flight, 0,
+ cifs_trace_rw_credits_read_resubmit);
+ }
+
if (req->cfile->invalidHandle) {
do {
rc = cifs_reopen_file(req->cfile, true);
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index f44f5f249400..42352f70b01c 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -301,7 +301,7 @@ smb2_adjust_credits(struct TCP_Server_Info *server,
unsigned int /*enum smb3_rw_credits_trace*/ trace)
{
struct cifs_credits *credits = &subreq->credits;
- int new_val = DIV_ROUND_UP(subreq->subreq.len, SMB2_MAX_BUFFER_SIZE);
+ int new_val = DIV_ROUND_UP(subreq->actual_len, SMB2_MAX_BUFFER_SIZE);
int scredits, in_flight;
if (!credits->value || credits->value == new_val)
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index d262e70100c9..5f5f51bf9850 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -4523,9 +4523,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:
@@ -4579,15 +4579,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
@@ -4615,6 +4618,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,
@@ -4625,15 +4629,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;
@@ -4648,11 +4652,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)
@@ -4676,11 +4682,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:
diff --git a/fs/smb/client/trace.h b/fs/smb/client/trace.h
index 36d5295c2a6f..13adfe550b99 100644
--- a/fs/smb/client/trace.h
+++ b/fs/smb/client/trace.h
@@ -30,6 +30,7 @@
EM(cifs_trace_rw_credits_old_session, "old-session") \
EM(cifs_trace_rw_credits_read_response_add, "rd-resp-add") \
EM(cifs_trace_rw_credits_read_response_clear, "rd-resp-clr") \
+ EM(cifs_trace_rw_credits_read_resubmit, "rd-resubmit") \
EM(cifs_trace_rw_credits_read_submit, "rd-submit ") \
EM(cifs_trace_rw_credits_write_prepare, "wr-prepare ") \
EM(cifs_trace_rw_credits_write_response_add, "wr-resp-add") \
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 6.10 157/375] netfs, cifs: Fix handling of short DIO read
[not found] <20240910092622.245959861@linuxfoundation.org>
2024-09-10 9:29 ` [PATCH 6.10 156/375] cifs: Fix lack of credit renegotiation on read retry Greg Kroah-Hartman
@ 2024-09-10 9:29 ` Greg Kroah-Hartman
2024-09-10 9:29 ` [PATCH 6.10 158/375] cifs: Fix copy offload to flush destination region Greg Kroah-Hartman
` (3 subsequent siblings)
5 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2024-09-10 9:29 UTC (permalink / raw)
To: stable
Cc: Greg Kroah-Hartman, patches, David Howells, Steve French,
Paulo Alcantara, Jeff Layton, linux-cifs, netfs, linux-fsdevel,
Steve French, Sasha Levin
6.10-stable review patch. If anyone has any objections, please let me know.
------------------
From: David Howells <dhowells@redhat.com>
[ Upstream commit 1da29f2c39b67b846b74205c81bf0ccd96d34727 ]
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>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
fs/netfs/io.c | 17 +++++++++++------
fs/smb/client/smb2pdu.c | 13 +++++++++----
include/linux/netfs.h | 1 +
3 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/fs/netfs/io.c b/fs/netfs/io.c
index 2a5c22606fb1..c91e7b12bbf1 100644
--- a/fs/netfs/io.c
+++ b/fs/netfs/io.c
@@ -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:
@@ -777,10 +779,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 5f5f51bf9850..8e02e9f45e0e 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -4501,6 +4501,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 =
@@ -4593,11 +4594,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,
diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index 5d0288938cc2..d8892b1a2dd7 100644
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -200,6 +200,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 {
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 6.10 158/375] cifs: Fix copy offload to flush destination region
[not found] <20240910092622.245959861@linuxfoundation.org>
2024-09-10 9:29 ` [PATCH 6.10 156/375] cifs: Fix lack of credit renegotiation on read retry Greg Kroah-Hartman
2024-09-10 9:29 ` [PATCH 6.10 157/375] netfs, cifs: Fix handling of short DIO read Greg Kroah-Hartman
@ 2024-09-10 9:29 ` Greg Kroah-Hartman
2024-09-10 9:29 ` [PATCH 6.10 159/375] cifs: Fix FALLOC_FL_ZERO_RANGE to preflush buffered part of target region Greg Kroah-Hartman
` (2 subsequent siblings)
5 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2024-09-10 9:29 UTC (permalink / raw)
To: stable
Cc: Greg Kroah-Hartman, patches, David Howells, Steve French,
Paulo Alcantara, Shyam Prasad N, Rohith Surabattula,
Matthew Wilcox, Jeff Layton, linux-cifs, linux-mm, linux-fsdevel,
Sasha Levin
6.10-stable review patch. If anyone has any objections, please let me know.
------------------
From: David Howells <dhowells@redhat.com>
[ Upstream commit 8101d6e112e2524e967368f920c404ae445a9757 ]
Fix cifs_file_copychunk_range() to flush the destination region before
invalidating it to avoid potential loss of data should the copy fail, in
whole or in part, in some way.
Fixes: 7b2404a886f8 ("cifs: Fix flushing, invalidation and file size with copy_file_range()")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Steve French <stfrench@microsoft.com>
cc: Paulo Alcantara <pc@manguebit.com>
cc: Shyam Prasad N <nspmangalore@gmail.com>
cc: Rohith Surabattula <rohiths.msft@gmail.com>
cc: Matthew Wilcox <willy@infradead.org>
cc: Jeff Layton <jlayton@kernel.org>
cc: linux-cifs@vger.kernel.org
cc: linux-mm@kvack.org
cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
fs/smb/client/cifsfs.c | 21 ++++-----------------
1 file changed, 4 insertions(+), 17 deletions(-)
diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index 2c4b357d85e2..a1acf5bd1e3a 100644
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -1341,7 +1341,6 @@ ssize_t cifs_file_copychunk_range(unsigned int xid,
struct cifsFileInfo *smb_file_target;
struct cifs_tcon *src_tcon;
struct cifs_tcon *target_tcon;
- unsigned long long destend, fstart, fend;
ssize_t rc;
cifs_dbg(FYI, "copychunk range\n");
@@ -1391,25 +1390,13 @@ ssize_t cifs_file_copychunk_range(unsigned int xid,
goto unlock;
}
- destend = destoff + len - 1;
-
- /* Flush the folios at either end of the destination range to prevent
- * accidental loss of dirty data outside of the range.
+ /* Flush and invalidate all the folios in the destination region. If
+ * the copy was successful, then some of the flush is extra overhead,
+ * but we need to allow for the copy failing in some way (eg. ENOSPC).
*/
- fstart = destoff;
- fend = destend;
-
- rc = cifs_flush_folio(target_inode, destoff, &fstart, &fend, true);
+ rc = filemap_invalidate_inode(target_inode, true, destoff, destoff + len - 1);
if (rc)
goto unlock;
- rc = cifs_flush_folio(target_inode, destend, &fstart, &fend, false);
- if (rc)
- goto unlock;
- if (fend > target_cifsi->netfs.zero_point)
- target_cifsi->netfs.zero_point = fend + 1;
-
- /* Discard all the folios that overlap the destination region. */
- truncate_inode_pages_range(&target_inode->i_data, fstart, fend);
fscache_invalidate(cifs_inode_cookie(target_inode), NULL,
i_size_read(target_inode), 0);
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 6.10 159/375] cifs: Fix FALLOC_FL_ZERO_RANGE to preflush buffered part of target region
[not found] <20240910092622.245959861@linuxfoundation.org>
` (2 preceding siblings ...)
2024-09-10 9:29 ` [PATCH 6.10 158/375] cifs: Fix copy offload to flush destination region Greg Kroah-Hartman
@ 2024-09-10 9:29 ` Greg Kroah-Hartman
2024-09-10 9:32 ` [PATCH 6.10 363/375] cifs: Fix zero_point init on inode initialisation Greg Kroah-Hartman
2024-09-10 9:32 ` [PATCH 6.10 364/375] cifs: Fix SMB1 readv/writev callback in the same way as SMB2/3 Greg Kroah-Hartman
5 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2024-09-10 9:29 UTC (permalink / raw)
To: stable
Cc: Greg Kroah-Hartman, patches, David Howells, Steve French,
Zhang Xiaoxu, Pavel Shilovsky, Paulo Alcantara, Shyam Prasad N,
Rohith Surabattula, Jeff Layton, linux-cifs, linux-mm,
Sasha Levin
6.10-stable review patch. If anyone has any objections, please let me know.
------------------
From: David Howells <dhowells@redhat.com>
[ Upstream commit 91d1dfae464987aaf6c79ff51d8674880fb3be77 ]
Under certain conditions, the range to be cleared by FALLOC_FL_ZERO_RANGE
may only be buffered locally and not yet have been flushed to the server.
For example:
xfs_io -f -t -c "pwrite -S 0x41 0 4k" \
-c "pwrite -S 0x42 4k 4k" \
-c "fzero 0 4k" \
-c "pread -v 0 8k" /xfstest.test/foo
will write two 4KiB blocks of data, which get buffered in the pagecache,
and then fallocate() is used to clear the first 4KiB block on the server -
but we don't flush the data first, which means the EOF position on the
server is wrong, and so the FSCTL_SET_ZERO_DATA RPC fails (and xfs_io
ignores the error), but then when we try to read it, we see the old data.
Fix this by preflushing any part of the target region that above the
server's idea of the EOF position to force the server to update its EOF
position.
Note, however, that we don't want to simply expand the file by moving the
EOF before doing the FSCTL_SET_ZERO_DATA[*] because someone else might see
the zeroed region or if the RPC fails we then have to try to clean it up or
risk getting corruption.
[*] And we have to move the EOF first otherwise FSCTL_SET_ZERO_DATA won't
do what we want.
This fixes the generic/008 xfstest.
[!] Note: A better way to do this might be to split the operation into two
parts: we only do FSCTL_SET_ZERO_DATA for the part of the range below the
server's EOF and then, if that worked, invalidate the buffered pages for the
part above the range.
Fixes: 6b69040247e1 ("cifs/smb3: Fix data inconsistent when zero file range")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Steve French <stfrench@microsoft.com>
cc: Zhang Xiaoxu <zhangxiaoxu5@huawei.com>
cc: Pavel Shilovsky <pshilov@microsoft.com>
cc: Paulo Alcantara <pc@manguebit.com>
cc: Shyam Prasad N <nspmangalore@gmail.com>
cc: Rohith Surabattula <rohiths.msft@gmail.com>
cc: Jeff Layton <jlayton@kernel.org>
cc: linux-cifs@vger.kernel.org
cc: linux-mm@kvack.org
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
fs/smb/client/smb2ops.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 42352f70b01c..1d6e8eacdd74 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -3219,13 +3219,15 @@ static long smb3_zero_data(struct file *file, struct cifs_tcon *tcon,
}
static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,
- loff_t offset, loff_t len, bool keep_size)
+ unsigned long long offset, unsigned long long len,
+ bool keep_size)
{
struct cifs_ses *ses = tcon->ses;
struct inode *inode = file_inode(file);
struct cifsInodeInfo *cifsi = CIFS_I(inode);
struct cifsFileInfo *cfile = file->private_data;
- unsigned long long new_size;
+ struct netfs_inode *ictx = netfs_inode(inode);
+ unsigned long long i_size, new_size, remote_size;
long rc;
unsigned int xid;
@@ -3237,6 +3239,16 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,
inode_lock(inode);
filemap_invalidate_lock(inode->i_mapping);
+ i_size = i_size_read(inode);
+ remote_size = ictx->remote_i_size;
+ if (offset + len >= remote_size && offset < i_size) {
+ unsigned long long top = umin(offset + len, i_size);
+
+ rc = filemap_write_and_wait_range(inode->i_mapping, offset, top - 1);
+ if (rc < 0)
+ goto zero_range_exit;
+ }
+
/*
* We zero the range through ioctl, so we need remove the page caches
* first, otherwise the data may be inconsistent with the server.
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 6.10 363/375] cifs: Fix zero_point init on inode initialisation
[not found] <20240910092622.245959861@linuxfoundation.org>
` (3 preceding siblings ...)
2024-09-10 9:29 ` [PATCH 6.10 159/375] cifs: Fix FALLOC_FL_ZERO_RANGE to preflush buffered part of target region Greg Kroah-Hartman
@ 2024-09-10 9:32 ` Greg Kroah-Hartman
2024-09-10 9:32 ` [PATCH 6.10 364/375] cifs: Fix SMB1 readv/writev callback in the same way as SMB2/3 Greg Kroah-Hartman
5 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2024-09-10 9:32 UTC (permalink / raw)
To: stable
Cc: Greg Kroah-Hartman, patches, David Howells,
Paulo Alcantara (Red Hat), Jeff Layton, linux-cifs, netfs,
linux-fsdevel, linux-mm, Steve French, Sasha Levin
6.10-stable review patch. If anyone has any objections, please let me know.
------------------
From: David Howells <dhowells@redhat.com>
[ Upstream commit 517b58c1f9242a6b4ac9443d95569dee58bf6b8b ]
Fix cifs_fattr_to_inode() such that the ->zero_point tracking variable
is initialised when the inode is initialised.
Fixes: 3ee1a1fc3981 ("cifs: Cut over to using netfslib")
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Paulo Alcantara (Red Hat) <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
cc: linux-mm@kvack.org
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
fs/smb/client/inode.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index dd0afa23734c..73e2e6c230b7 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -172,6 +172,8 @@ cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr,
CIFS_I(inode)->time = 0; /* force reval */
return -ESTALE;
}
+ if (inode->i_state & I_NEW)
+ CIFS_I(inode)->netfs.zero_point = fattr->cf_eof;
cifs_revalidate_cache(inode, fattr);
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 6.10 364/375] cifs: Fix SMB1 readv/writev callback in the same way as SMB2/3
[not found] <20240910092622.245959861@linuxfoundation.org>
` (4 preceding siblings ...)
2024-09-10 9:32 ` [PATCH 6.10 363/375] cifs: Fix zero_point init on inode initialisation Greg Kroah-Hartman
@ 2024-09-10 9:32 ` Greg Kroah-Hartman
5 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2024-09-10 9:32 UTC (permalink / raw)
To: stable
Cc: Greg Kroah-Hartman, patches, David Howells, Steve French,
Paulo Alcantara, Jeff Layton, linux-cifs, netfs, linux-fsdevel,
Sasha Levin
6.10-stable review patch. If anyone has any objections, please let me know.
------------------
From: David Howells <dhowells@redhat.com>
[ Upstream commit a68c74865f517e26728735aba0ae05055eaff76c ]
Port a number of SMB2/3 async readv/writev fixes to the SMB1 transport:
commit a88d60903696c01de577558080ec4fc738a70475
cifs: Don't advance the I/O iterator before terminating subrequest
commit ce5291e56081730ec7d87bc9aa41f3de73ff3256
cifs: Defer read completion
commit 1da29f2c39b67b846b74205c81bf0ccd96d34727
netfs, cifs: Fix handling of short DIO read
Fixes: 3ee1a1fc3981 ("cifs: Cut over to using netfslib")
Signed-off-by: David Howells <dhowells@redhat.com>
Reported-by: Steve French <stfrench@microsoft.com>
Reviewed-by: 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>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
fs/smb/client/cifssmb.c | 54 +++++++++++++++++++++++++++++++++++------
1 file changed, 46 insertions(+), 8 deletions(-)
diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index 6dce70f17208..cfae2e918209 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -1261,16 +1261,32 @@ CIFS_open(const unsigned int xid, struct cifs_open_parms *oparms, int *oplock,
return rc;
}
+static void cifs_readv_worker(struct work_struct *work)
+{
+ struct cifs_io_subrequest *rdata =
+ container_of(work, struct cifs_io_subrequest, subreq.work);
+
+ netfs_subreq_terminated(&rdata->subreq,
+ (rdata->result == 0 || rdata->result == -EAGAIN) ?
+ rdata->got_bytes : rdata->result, true);
+}
+
static void
cifs_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 = tcon->ses->server;
struct smb_rqst rqst = { .rq_iov = rdata->iov,
.rq_nvec = 2,
.rq_iter = rdata->subreq.io_iter };
- struct cifs_credits credits = { .value = 1, .instance = 0 };
+ struct cifs_credits credits = {
+ .value = 1,
+ .instance = 0,
+ .rreq_debug_id = rdata->rreq->debug_id,
+ .rreq_debug_index = rdata->subreq.debug_index,
+ };
cifs_dbg(FYI, "%s: mid=%llu state=%d result=%d bytes=%zu\n",
__func__, mid->mid, mid->mid_state, rdata->result,
@@ -1282,6 +1298,7 @@ cifs_readv_callback(struct mid_q_entry *mid)
if (server->sign) {
int rc = 0;
+ iov_iter_truncate(&rqst.rq_iter, rdata->got_bytes);
rc = cifs_verify_signature(&rqst, server,
mid->sequence_number);
if (rc)
@@ -1306,13 +1323,21 @@ cifs_readv_callback(struct mid_q_entry *mid)
rdata->result = -EIO;
}
- if (rdata->result == 0 || rdata->result == -EAGAIN)
- iov_iter_advance(&rdata->subreq.io_iter, rdata->got_bytes);
+ if (rdata->result == -ENODATA) {
+ __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;
+ }
+ }
+
rdata->credits.value = 0;
- netfs_subreq_terminated(&rdata->subreq,
- (rdata->result == 0 || rdata->result == -EAGAIN) ?
- rdata->got_bytes : rdata->result,
- false);
+ INIT_WORK(&rdata->subreq.work, cifs_readv_worker);
+ queue_work(cifsiod_wq, &rdata->subreq.work);
release_mid(mid);
add_credits(server, &credits, 0);
}
@@ -1619,9 +1644,15 @@ static void
cifs_writev_callback(struct mid_q_entry *mid)
{
struct cifs_io_subrequest *wdata = mid->callback_data;
+ struct TCP_Server_Info *server = wdata->server;
struct cifs_tcon *tcon = tlink_tcon(wdata->req->cfile->tlink);
WRITE_RSP *smb = (WRITE_RSP *)mid->resp_buf;
- struct cifs_credits credits = { .value = 1, .instance = 0 };
+ struct cifs_credits credits = {
+ .value = 1,
+ .instance = 0,
+ .rreq_debug_id = wdata->rreq->debug_id,
+ .rreq_debug_index = wdata->subreq.debug_index,
+ };
ssize_t result;
size_t written;
@@ -1657,9 +1688,16 @@ cifs_writev_callback(struct mid_q_entry *mid)
break;
}
+ trace_smb3_rw_credits(credits.rreq_debug_id, credits.rreq_debug_index,
+ wdata->credits.value,
+ server->credits, server->in_flight,
+ 0, cifs_trace_rw_credits_write_response_clear);
wdata->credits.value = 0;
cifs_write_subrequest_terminated(wdata, result, true);
release_mid(mid);
+ trace_smb3_rw_credits(credits.rreq_debug_id, credits.rreq_debug_index, 0,
+ server->credits, server->in_flight,
+ credits.value, cifs_trace_rw_credits_write_response_add);
add_credits(tcon->ses->server, &credits, 0);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread