* [PATCH 1/8] netfs: Fix mtime/ctime update for mmapped writes
2024-09-23 15:07 [PATCH 0/8] netfs, afs, cifs: Miscellaneous fixes/changes David Howells
@ 2024-09-23 15:07 ` David Howells
2024-09-23 23:17 ` Steve French
2024-09-23 15:07 ` [PATCH 2/8] netfs: Drop the was_async arg from netfs_read_subreq_terminated() David Howells
` (6 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: David Howells @ 2024-09-23 15:07 UTC (permalink / raw)
To: Christian Brauner, Steve French, Marc Dionne
Cc: David Howells, Paulo Alcantara, Jeff Layton, Matthew Wilcox,
netfs, linux-afs, linux-cifs, linux-nfs, ceph-devel, v9fs,
linux-erofs, linux-fsdevel, linux-mm, linux-kernel,
kernel test robot
The cifs flag CIFS_INO_MODIFIED_ATTR, which indicates that the mtime and
ctime need to be written back on close, got taken over by netfs as
NETFS_ICTX_MODIFIED_ATTR to avoid the need to call a function pointer to
set it.
The flag gets set correctly on buffered writes, but doesn't get set by
netfs_page_mkwrite(), leading to occasional failures in generic/080 and
generic/215.
Fix this by setting the flag in netfs_page_mkwrite().
Fixes: 73425800ac94 ("netfs, cifs: Move CIFS_INO_MODIFIED_ATTR to netfs_inode")
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202409161629.98887b2-oliver.sang@intel.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Jeff Layton <jlayton@kernel.org>
cc: Steve French <sfrench@samba.org>
cc: Paulo Alcantara <pc@manguebit.com>
cc: linux-cifs@vger.kernel.org
cc: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
---
fs/netfs/buffered_write.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/netfs/buffered_write.c b/fs/netfs/buffered_write.c
index d7eae597e54d..b3910dfcb56d 100644
--- a/fs/netfs/buffered_write.c
+++ b/fs/netfs/buffered_write.c
@@ -552,6 +552,7 @@ vm_fault_t netfs_page_mkwrite(struct vm_fault *vmf, struct netfs_group *netfs_gr
trace_netfs_folio(folio, netfs_folio_trace_mkwrite);
netfs_set_group(folio, netfs_group);
file_update_time(file);
+ set_bit(NETFS_ICTX_MODIFIED_ATTR, &ictx->flags);
if (ictx->ops->post_modify)
ictx->ops->post_modify(inode);
ret = VM_FAULT_LOCKED;
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/8] netfs: Fix mtime/ctime update for mmapped writes
2024-09-23 15:07 ` [PATCH 1/8] netfs: Fix mtime/ctime update for mmapped writes David Howells
@ 2024-09-23 23:17 ` Steve French
0 siblings, 0 replies; 13+ messages in thread
From: Steve French @ 2024-09-23 23:17 UTC (permalink / raw)
To: David Howells
Cc: Christian Brauner, Steve French, Marc Dionne, Paulo Alcantara,
Jeff Layton, Matthew Wilcox, netfs, linux-afs, linux-cifs,
linux-nfs, ceph-devel, v9fs, linux-erofs, linux-fsdevel, linux-mm,
linux-kernel, kernel test robot
added to cifs-2.6.git for-next since it is important as it fixes a
regression affecting cifs.ko
On Mon, Sep 23, 2024 at 10:08 AM David Howells <dhowells@redhat.com> wrote:
>
> The cifs flag CIFS_INO_MODIFIED_ATTR, which indicates that the mtime and
> ctime need to be written back on close, got taken over by netfs as
> NETFS_ICTX_MODIFIED_ATTR to avoid the need to call a function pointer to
> set it.
>
> The flag gets set correctly on buffered writes, but doesn't get set by
> netfs_page_mkwrite(), leading to occasional failures in generic/080 and
> generic/215.
>
> Fix this by setting the flag in netfs_page_mkwrite().
>
> Fixes: 73425800ac94 ("netfs, cifs: Move CIFS_INO_MODIFIED_ATTR to netfs_inode")
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202409161629.98887b2-oliver.sang@intel.com
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Jeff Layton <jlayton@kernel.org>
> cc: Steve French <sfrench@samba.org>
> cc: Paulo Alcantara <pc@manguebit.com>
> cc: linux-cifs@vger.kernel.org
> cc: netfs@lists.linux.dev
> cc: linux-fsdevel@vger.kernel.org
> ---
> fs/netfs/buffered_write.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/netfs/buffered_write.c b/fs/netfs/buffered_write.c
> index d7eae597e54d..b3910dfcb56d 100644
> --- a/fs/netfs/buffered_write.c
> +++ b/fs/netfs/buffered_write.c
> @@ -552,6 +552,7 @@ vm_fault_t netfs_page_mkwrite(struct vm_fault *vmf, struct netfs_group *netfs_gr
> trace_netfs_folio(folio, netfs_folio_trace_mkwrite);
> netfs_set_group(folio, netfs_group);
> file_update_time(file);
> + set_bit(NETFS_ICTX_MODIFIED_ATTR, &ictx->flags);
> if (ictx->ops->post_modify)
> ictx->ops->post_modify(inode);
> ret = VM_FAULT_LOCKED;
>
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/8] netfs: Drop the was_async arg from netfs_read_subreq_terminated()
2024-09-23 15:07 [PATCH 0/8] netfs, afs, cifs: Miscellaneous fixes/changes David Howells
2024-09-23 15:07 ` [PATCH 1/8] netfs: Fix mtime/ctime update for mmapped writes David Howells
@ 2024-09-23 15:07 ` David Howells
2024-09-23 15:07 ` [PATCH 3/8] afs: Fix missing wire-up of afs_retry_request() David Howells
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2024-09-23 15:07 UTC (permalink / raw)
To: Christian Brauner, Steve French, Marc Dionne
Cc: David Howells, Paulo Alcantara, Jeff Layton, Matthew Wilcox,
netfs, linux-afs, linux-cifs, linux-nfs, ceph-devel, v9fs,
linux-erofs, linux-fsdevel, linux-mm, linux-kernel,
Linus Torvalds
Drop the was_async argument from netfs_read_subreq_terminated(). Almost
every caller is either in process context and passes false. Some
filesystems delegate the call to a workqueue to avoid doing the work in
their network message queue parsing thread.
The only exception is netfs_cache_read_terminated() which handles
completion in the cache - which is usually a callback from the backing
filesystem in softirq context, though it can be from process context if an
error occurred. In this case, delegate to a workqueue.
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Link: https://lore.kernel.org/r/CAHk-=wiVC5Cgyz6QKXFu6fTaA6h4CjexDR-OV9kL6Vo5x9v8=A@mail.gmail.com/
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Jeff Layton <jlayton@kernel.org>
cc: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
---
fs/9p/vfs_addr.c | 3 +-
fs/afs/file.c | 15 ++++---
fs/afs/fsclient.c | 2 +-
fs/afs/yfsclient.c | 2 +-
fs/ceph/addr.c | 13 ++++--
fs/netfs/buffered_read.c | 16 +++----
fs/netfs/direct_read.c | 2 +-
fs/netfs/internal.h | 2 +-
fs/netfs/objects.c | 17 ++++++-
fs/netfs/read_collect.c | 95 +++++++++++++++++-----------------------
fs/netfs/read_retry.c | 2 +-
fs/nfs/fscache.c | 6 ++-
fs/nfs/fscache.h | 3 +-
fs/smb/client/cifssmb.c | 10 +----
fs/smb/client/file.c | 3 +-
fs/smb/client/smb2pdu.c | 10 +----
include/linux/netfs.h | 7 ++-
17 files changed, 101 insertions(+), 107 deletions(-)
diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c
index 819c75233235..e4144e1a10a9 100644
--- a/fs/9p/vfs_addr.c
+++ b/fs/9p/vfs_addr.c
@@ -83,7 +83,8 @@ static void v9fs_issue_read(struct netfs_io_subrequest *subreq)
if (!err)
subreq->transferred += total;
- netfs_read_subreq_terminated(subreq, err, false);
+ subreq->error = err;
+ netfs_read_subreq_terminated(subreq);
}
/**
diff --git a/fs/afs/file.c b/fs/afs/file.c
index 492d857a3fa0..1d30924cec5b 100644
--- a/fs/afs/file.c
+++ b/fs/afs/file.c
@@ -246,7 +246,8 @@ static void afs_fetch_data_notify(struct afs_operation *op)
subreq->rreq->i_size = req->file_size;
if (req->pos + req->actual_len >= req->file_size)
__set_bit(NETFS_SREQ_HIT_EOF, &subreq->flags);
- netfs_read_subreq_terminated(subreq, error, false);
+ subreq->error = error;
+ netfs_read_subreq_terminated(subreq);
req->subreq = NULL;
} else if (req->done) {
req->done(req);
@@ -301,8 +302,10 @@ int afs_fetch_data(struct afs_vnode *vnode, struct afs_read *req)
op = afs_alloc_operation(req->key, vnode->volume);
if (IS_ERR(op)) {
- if (req->subreq)
- netfs_read_subreq_terminated(req->subreq, PTR_ERR(op), false);
+ if (req->subreq) {
+ req->subreq->error = PTR_ERR(op);
+ netfs_read_subreq_terminated(req->subreq);
+ }
return PTR_ERR(op);
}
@@ -320,8 +323,10 @@ static void afs_read_worker(struct work_struct *work)
struct afs_read *fsreq;
fsreq = afs_alloc_read(GFP_NOFS);
- if (!fsreq)
- return netfs_read_subreq_terminated(subreq, -ENOMEM, false);
+ if (!fsreq) {
+ subreq->error = -ENOMEM;
+ return netfs_read_subreq_terminated(subreq);
+ }
fsreq->subreq = subreq;
fsreq->pos = subreq->start + subreq->transferred;
diff --git a/fs/afs/fsclient.c b/fs/afs/fsclient.c
index 098fa034a1cc..784f7daab112 100644
--- a/fs/afs/fsclient.c
+++ b/fs/afs/fsclient.c
@@ -352,7 +352,7 @@ static int afs_deliver_fs_fetch_data(struct afs_call *call)
ret = afs_extract_data(call, true);
if (req->subreq) {
req->subreq->transferred += count_before - call->iov_len;
- netfs_read_subreq_progress(req->subreq, false);
+ netfs_read_subreq_progress(req->subreq);
}
if (ret < 0)
return ret;
diff --git a/fs/afs/yfsclient.c b/fs/afs/yfsclient.c
index 024227aba4cd..368cf277d801 100644
--- a/fs/afs/yfsclient.c
+++ b/fs/afs/yfsclient.c
@@ -398,7 +398,7 @@ static int yfs_deliver_fs_fetch_data64(struct afs_call *call)
ret = afs_extract_data(call, true);
if (req->subreq) {
req->subreq->transferred += count_before - call->iov_len;
- netfs_read_subreq_progress(req->subreq, false);
+ netfs_read_subreq_progress(req->subreq);
}
if (ret < 0)
return ret;
diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index 5d9ccda098cc..0d131101db3d 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -254,8 +254,9 @@ static void finish_netfs_read(struct ceph_osd_request *req)
subreq->transferred = err;
err = 0;
}
+ subreq->error = err;
trace_netfs_sreq(subreq, netfs_sreq_trace_io_progress);
- netfs_read_subreq_terminated(subreq, err, false);
+ netfs_read_subreq_terminated(subreq);
iput(req->r_inode);
ceph_dec_osd_stopping_blocker(fsc->mdsc);
}
@@ -315,7 +316,9 @@ static bool ceph_netfs_issue_op_inline(struct netfs_io_subrequest *subreq)
ceph_mdsc_put_request(req);
out:
- netfs_read_subreq_terminated(subreq, err, false);
+ subreq->error = err;
+ trace_netfs_sreq(subreq, netfs_sreq_trace_io_progress);
+ netfs_read_subreq_terminated(subreq);
return true;
}
@@ -427,8 +430,10 @@ static void ceph_netfs_issue_read(struct netfs_io_subrequest *subreq)
ceph_osdc_start_request(req->r_osdc, req);
out:
ceph_osdc_put_request(req);
- if (err)
- netfs_read_subreq_terminated(subreq, err, false);
+ if (err) {
+ subreq->error = err;
+ netfs_read_subreq_terminated(subreq);
+ }
doutc(cl, "%llx.%llx result %d\n", ceph_vinop(inode), err);
}
diff --git a/fs/netfs/buffered_read.c b/fs/netfs/buffered_read.c
index c40e226053cc..518799894990 100644
--- a/fs/netfs/buffered_read.c
+++ b/fs/netfs/buffered_read.c
@@ -183,13 +183,12 @@ static void netfs_cache_read_terminated(void *priv, ssize_t transferred_or_error
struct netfs_io_subrequest *subreq = priv;
if (transferred_or_error < 0) {
- netfs_read_subreq_terminated(subreq, transferred_or_error, was_async);
- return;
- }
-
- if (transferred_or_error > 0)
+ subreq->error = transferred_or_error;
+ } else {
+ subreq->error = 0;
subreq->transferred += transferred_or_error;
- netfs_read_subreq_terminated(subreq, 0, was_async);
+ }
+ schedule_work(&subreq->work);
}
/*
@@ -295,7 +294,8 @@ static void netfs_read_to_pagecache(struct netfs_io_request *rreq)
netfs_stat(&netfs_n_rh_zero);
slice = netfs_prepare_read_iterator(subreq);
__set_bit(NETFS_SREQ_CLEAR_TAIL, &subreq->flags);
- netfs_read_subreq_terminated(subreq, 0, false);
+ subreq->error = 0;
+ netfs_read_subreq_terminated(subreq);
goto done;
}
@@ -317,7 +317,7 @@ static void netfs_read_to_pagecache(struct netfs_io_request *rreq)
} while (size > 0);
if (atomic_dec_and_test(&rreq->nr_outstanding))
- netfs_rreq_terminated(rreq, false);
+ netfs_rreq_terminated(rreq);
/* Defer error return as we may need to wait for outstanding I/O. */
cmpxchg(&rreq->error, 0, ret);
diff --git a/fs/netfs/direct_read.c b/fs/netfs/direct_read.c
index b1a66a6e6bc2..bde99fe4221b 100644
--- a/fs/netfs/direct_read.c
+++ b/fs/netfs/direct_read.c
@@ -100,7 +100,7 @@ static int netfs_dispatch_unbuffered_reads(struct netfs_io_request *rreq)
} while (size > 0);
if (atomic_dec_and_test(&rreq->nr_outstanding))
- netfs_rreq_terminated(rreq, false);
+ netfs_rreq_terminated(rreq);
return ret;
}
diff --git a/fs/netfs/internal.h b/fs/netfs/internal.h
index c9f0ed24cb7b..c7f23dd3556a 100644
--- a/fs/netfs/internal.h
+++ b/fs/netfs/internal.h
@@ -87,7 +87,7 @@ static inline void netfs_see_request(struct netfs_io_request *rreq,
* read_collect.c
*/
void netfs_read_termination_worker(struct work_struct *work);
-void netfs_rreq_terminated(struct netfs_io_request *rreq, bool was_async);
+void netfs_rreq_terminated(struct netfs_io_request *rreq);
/*
* read_pgpriv2.c
diff --git a/fs/netfs/objects.c b/fs/netfs/objects.c
index 31e388ec6e48..d32964e8ca5d 100644
--- a/fs/netfs/objects.c
+++ b/fs/netfs/objects.c
@@ -56,7 +56,7 @@ struct netfs_io_request *netfs_alloc_request(struct address_space *mapping,
origin == NETFS_READ_GAPS ||
origin == NETFS_READ_FOR_WRITE ||
origin == NETFS_DIO_READ)
- INIT_WORK(&rreq->work, netfs_read_termination_worker);
+ INIT_WORK(&rreq->work, NULL);
else
INIT_WORK(&rreq->work, netfs_write_collection_worker);
@@ -191,7 +191,20 @@ struct netfs_io_subrequest *netfs_alloc_subrequest(struct netfs_io_request *rreq
}
memset(subreq, 0, kmem_cache_size(cache));
- INIT_WORK(&subreq->work, NULL);
+
+ switch (rreq->origin) {
+ case NETFS_READAHEAD:
+ case NETFS_READPAGE:
+ case NETFS_READ_GAPS:
+ case NETFS_READ_FOR_WRITE:
+ case NETFS_DIO_READ:
+ INIT_WORK(&subreq->work, netfs_read_subreq_termination_worker);
+ break;
+ default:
+ INIT_WORK(&subreq->work, NULL);
+ break;
+ }
+
INIT_LIST_HEAD(&subreq->rreq_link);
refcount_set(&subreq->ref, 2);
subreq->rreq = rreq;
diff --git a/fs/netfs/read_collect.c b/fs/netfs/read_collect.c
index b18c65ba5580..4ff4e520fc95 100644
--- a/fs/netfs/read_collect.c
+++ b/fs/netfs/read_collect.c
@@ -83,7 +83,7 @@ static void netfs_unlock_read_folio(struct netfs_io_subrequest *subreq,
* Unlock any folios that are now completely read. Returns true if the
* subrequest is removed from the list.
*/
-static bool netfs_consume_read_data(struct netfs_io_subrequest *subreq, bool was_async)
+static bool netfs_consume_read_data(struct netfs_io_subrequest *subreq)
{
struct netfs_io_subrequest *prev, *next;
struct netfs_io_request *rreq = subreq->rreq;
@@ -222,8 +222,7 @@ static bool netfs_consume_read_data(struct netfs_io_subrequest *subreq, bool was
subreq->curr_folioq_slot = slot;
if (folioq && folioq_folio(folioq, slot))
subreq->curr_folio_order = folioq->orders[slot];
- if (!was_async)
- cond_resched();
+ cond_resched();
goto next_folio;
}
@@ -359,7 +358,7 @@ static void netfs_rreq_assess_dio(struct netfs_io_request *rreq)
* Note that we're in normal kernel thread context at this point, possibly
* running on a workqueue.
*/
-static void netfs_rreq_assess(struct netfs_io_request *rreq)
+void netfs_rreq_terminated(struct netfs_io_request *rreq)
{
trace_netfs_rreq(rreq, netfs_rreq_trace_assess);
@@ -386,56 +385,28 @@ static void netfs_rreq_assess(struct netfs_io_request *rreq)
netfs_pgpriv2_write_to_the_cache(rreq);
}
-void netfs_read_termination_worker(struct work_struct *work)
-{
- struct netfs_io_request *rreq =
- container_of(work, struct netfs_io_request, work);
- netfs_see_request(rreq, netfs_rreq_trace_see_work);
- netfs_rreq_assess(rreq);
- netfs_put_request(rreq, false, netfs_rreq_trace_put_work_complete);
-}
-
-/*
- * Handle the completion of all outstanding I/O operations on a read request.
- * We inherit a ref from the caller.
- */
-void netfs_rreq_terminated(struct netfs_io_request *rreq, bool was_async)
-{
- if (!was_async)
- return netfs_rreq_assess(rreq);
- if (!work_pending(&rreq->work)) {
- netfs_get_request(rreq, netfs_rreq_trace_get_work);
- if (!queue_work(system_unbound_wq, &rreq->work))
- netfs_put_request(rreq, was_async, netfs_rreq_trace_put_work_nq);
- }
-}
-
/**
* netfs_read_subreq_progress - Note progress of a read operation.
- * @subreq: The read request that has terminated.
- * @was_async: True if we're in an asynchronous context.
*
* This tells the read side of netfs lib that a contributory I/O operation has
* made some progress and that it may be possible to unlock some folios.
*
* Before calling, the filesystem should update subreq->transferred to track
* the amount of data copied into the output buffer.
- *
- * If @was_async is true, the caller might be running in softirq or interrupt
- * context and we can't sleep.
*/
-void netfs_read_subreq_progress(struct netfs_io_subrequest *subreq,
- bool was_async)
+void netfs_read_subreq_progress(struct netfs_io_subrequest *subreq)
{
struct netfs_io_request *rreq = subreq->rreq;
+ might_sleep();
+
trace_netfs_sreq(subreq, netfs_sreq_trace_progress);
if (subreq->transferred > subreq->consumed &&
(rreq->origin == NETFS_READAHEAD ||
rreq->origin == NETFS_READPAGE ||
rreq->origin == NETFS_READ_FOR_WRITE)) {
- netfs_consume_read_data(subreq, was_async);
+ netfs_consume_read_data(subreq);
__clear_bit(NETFS_SREQ_NO_PROGRESS, &subreq->flags);
}
}
@@ -444,28 +415,25 @@ EXPORT_SYMBOL(netfs_read_subreq_progress);
/**
* netfs_read_subreq_terminated - Note the termination of an I/O operation.
* @subreq: The I/O request that has terminated.
- * @error: Error code indicating type of completion.
- * @was_async: The termination was asynchronous
*
* This tells the read helper that a contributory I/O operation has terminated,
* one way or another, and that it should integrate the results.
*
- * The caller indicates the outcome of the operation through @error, supplying
- * 0 to indicate a successful or retryable transfer (if NETFS_SREQ_NEED_RETRY
- * is set) or a negative error code. The helper will look after reissuing I/O
- * operations as appropriate and writing downloaded data to the cache.
+ * The caller indicates the outcome of the operation through @subreq->error,
+ * supplying 0 to indicate a successful or retryable transfer (if
+ * NETFS_SREQ_NEED_RETRY is set) or a negative error code. The helper will
+ * look after reissuing I/O operations as appropriate and writing downloaded
+ * data to the cache.
*
* Before calling, the filesystem should update subreq->transferred to track
* the amount of data copied into the output buffer.
- *
- * If @was_async is true, the caller might be running in softirq or interrupt
- * context and we can't sleep.
*/
-void netfs_read_subreq_terminated(struct netfs_io_subrequest *subreq,
- int error, bool was_async)
+void netfs_read_subreq_terminated(struct netfs_io_subrequest *subreq)
{
struct netfs_io_request *rreq = subreq->rreq;
+ might_sleep();
+
switch (subreq->source) {
case NETFS_READ_FROM_CACHE:
netfs_stat(&netfs_n_rh_read_done);
@@ -483,7 +451,7 @@ void netfs_read_subreq_terminated(struct netfs_io_subrequest *subreq,
* If the read completed validly short, then we can clear the
* tail before going on to unlock the folios.
*/
- if (error == 0 && subreq->transferred < subreq->len &&
+ if (subreq->error == 0 && subreq->transferred < subreq->len &&
(test_bit(NETFS_SREQ_HIT_EOF, &subreq->flags) ||
test_bit(NETFS_SREQ_CLEAR_TAIL, &subreq->flags))) {
netfs_clear_unread(subreq);
@@ -494,7 +462,7 @@ void netfs_read_subreq_terminated(struct netfs_io_subrequest *subreq,
(rreq->origin == NETFS_READAHEAD ||
rreq->origin == NETFS_READPAGE ||
rreq->origin == NETFS_READ_FOR_WRITE)) {
- netfs_consume_read_data(subreq, was_async);
+ netfs_consume_read_data(subreq);
__clear_bit(NETFS_SREQ_NO_PROGRESS, &subreq->flags);
}
rreq->transferred += subreq->transferred;
@@ -503,7 +471,7 @@ void netfs_read_subreq_terminated(struct netfs_io_subrequest *subreq,
/* Deal with retry requests, short reads and errors. If we retry
* but don't make progress, we abandon the attempt.
*/
- if (!error && subreq->transferred < subreq->len) {
+ if (!subreq->error && subreq->transferred < subreq->len) {
if (test_bit(NETFS_SREQ_HIT_EOF, &subreq->flags)) {
trace_netfs_sreq(subreq, netfs_sreq_trace_hit_eof);
} else {
@@ -517,16 +485,15 @@ void netfs_read_subreq_terminated(struct netfs_io_subrequest *subreq,
set_bit(NETFS_RREQ_NEED_RETRY, &rreq->flags);
} else {
__set_bit(NETFS_SREQ_FAILED, &subreq->flags);
- error = -ENODATA;
+ subreq->error = -ENODATA;
}
}
}
- subreq->error = error;
trace_netfs_sreq(subreq, netfs_sreq_trace_terminated);
- if (unlikely(error < 0)) {
- trace_netfs_failure(rreq, subreq, error, netfs_fail_read);
+ if (unlikely(subreq->error < 0)) {
+ trace_netfs_failure(rreq, subreq, subreq->error, netfs_fail_read);
if (subreq->source == NETFS_READ_FROM_CACHE) {
netfs_stat(&netfs_n_rh_read_failed);
} else {
@@ -537,8 +504,24 @@ void netfs_read_subreq_terminated(struct netfs_io_subrequest *subreq,
}
if (atomic_dec_and_test(&rreq->nr_outstanding))
- netfs_rreq_terminated(rreq, was_async);
+ netfs_rreq_terminated(rreq);
- netfs_put_subrequest(subreq, was_async, netfs_sreq_trace_put_terminated);
+ netfs_put_subrequest(subreq, false, netfs_sreq_trace_put_terminated);
}
EXPORT_SYMBOL(netfs_read_subreq_terminated);
+
+/**
+ * netfs_read_subreq_termination_worker - Workqueue helper for read termination
+ * @work: The subreq->work in the I/O request that has been terminated.
+ *
+ * Helper function to jump to netfs_read_subreq_terminated() from the
+ * subrequest work item.
+ */
+void netfs_read_subreq_termination_worker(struct work_struct *work)
+{
+ struct netfs_io_subrequest *subreq =
+ container_of(work, struct netfs_io_subrequest, work);
+
+ netfs_read_subreq_terminated(subreq);
+}
+EXPORT_SYMBOL(netfs_read_subreq_termination_worker);
diff --git a/fs/netfs/read_retry.c b/fs/netfs/read_retry.c
index 0350592ea804..3f29e823d379 100644
--- a/fs/netfs/read_retry.c
+++ b/fs/netfs/read_retry.c
@@ -232,7 +232,7 @@ void netfs_retry_reads(struct netfs_io_request *rreq)
netfs_retry_read_subrequests(rreq);
if (atomic_dec_and_test(&rreq->nr_outstanding))
- netfs_rreq_terminated(rreq, false);
+ netfs_rreq_terminated(rreq);
}
/*
diff --git a/fs/nfs/fscache.c b/fs/nfs/fscache.c
index 810269ee0a50..2f3c4f773d73 100644
--- a/fs/nfs/fscache.c
+++ b/fs/nfs/fscache.c
@@ -307,8 +307,10 @@ static void nfs_netfs_issue_read(struct netfs_io_subrequest *sreq)
&nfs_async_read_completion_ops);
netfs = nfs_netfs_alloc(sreq);
- if (!netfs)
- return netfs_read_subreq_terminated(sreq, -ENOMEM, false);
+ if (!netfs) {
+ sreq->error = -ENOMEM;
+ return netfs_read_subreq_terminated(sreq);
+ }
pgio.pg_netfs = netfs; /* used in completion */
diff --git a/fs/nfs/fscache.h b/fs/nfs/fscache.h
index 772d485e96d3..9d86868f4998 100644
--- a/fs/nfs/fscache.h
+++ b/fs/nfs/fscache.h
@@ -74,7 +74,8 @@ static inline void nfs_netfs_put(struct nfs_netfs_io_data *netfs)
*/
netfs->sreq->transferred = min_t(s64, netfs->sreq->len,
atomic64_read(&netfs->transferred));
- netfs_read_subreq_terminated(netfs->sreq, netfs->error, false);
+ netfs->sreq->error = netfs->error;
+ netfs_read_subreq_terminated(netfs->sreq);
kfree(netfs);
}
static inline void nfs_netfs_inode_init(struct nfs_inode *nfsi)
diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index 131f20b91c3e..1f27847350cf 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -1261,14 +1261,6 @@ 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_read_subreq_terminated(&rdata->subreq, rdata->result, false);
-}
-
static void
cifs_readv_callback(struct mid_q_entry *mid)
{
@@ -1334,8 +1326,8 @@ cifs_readv_callback(struct mid_q_entry *mid)
}
rdata->credits.value = 0;
+ rdata->subreq.error = rdata->result;
rdata->subreq.transferred += rdata->got_bytes;
- INIT_WORK(&rdata->subreq.work, cifs_readv_worker);
queue_work(cifsiod_wq, &rdata->subreq.work);
release_mid(mid);
add_credits(server, &credits, 0);
diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index 78b59c4ef3ce..fb8e12c3c37c 100644
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -227,7 +227,8 @@ static void cifs_issue_read(struct netfs_io_subrequest *subreq)
return;
failed:
- netfs_read_subreq_terminated(subreq, rc, false);
+ subreq->error = rc;
+ netfs_read_subreq_terminated(subreq);
}
/*
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 2cb1bf65a172..0b63608aeecb 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -4494,14 +4494,6 @@ smb2_new_read_req(void **buf, unsigned int *total_len,
return rc;
}
-static void smb2_readv_worker(struct work_struct *work)
-{
- struct cifs_io_subrequest *rdata =
- container_of(work, struct cifs_io_subrequest, subreq.work);
-
- netfs_read_subreq_terminated(&rdata->subreq, rdata->result, false);
-}
-
static void
smb2_readv_callback(struct mid_q_entry *mid)
{
@@ -4614,9 +4606,9 @@ smb2_readv_callback(struct mid_q_entry *mid)
server->credits, server->in_flight,
0, cifs_trace_rw_credits_read_response_clear);
rdata->credits.value = 0;
+ rdata->subreq.error = rdata->result;
rdata->subreq.transferred += rdata->got_bytes;
trace_netfs_sreq(&rdata->subreq, netfs_sreq_trace_io_progress);
- INIT_WORK(&rdata->subreq.work, smb2_readv_worker);
queue_work(cifsiod_wq, &rdata->subreq.work);
release_mid(mid);
trace_smb3_rw_credits(rreq_debug_id, subreq_debug_index, 0,
diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index 5eaceef41e6c..d1f96b057b8f 100644
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -429,10 +429,9 @@ bool netfs_release_folio(struct folio *folio, gfp_t gfp);
vm_fault_t netfs_page_mkwrite(struct vm_fault *vmf, struct netfs_group *netfs_group);
/* (Sub)request management API. */
-void netfs_read_subreq_progress(struct netfs_io_subrequest *subreq,
- bool was_async);
-void netfs_read_subreq_terminated(struct netfs_io_subrequest *subreq,
- int error, bool was_async);
+void netfs_read_subreq_progress(struct netfs_io_subrequest *subreq);
+void netfs_read_subreq_terminated(struct netfs_io_subrequest *subreq);
+void netfs_read_subreq_termination_worker(struct work_struct *work);
void netfs_get_subrequest(struct netfs_io_subrequest *subreq,
enum netfs_sreq_ref_trace what);
void netfs_put_subrequest(struct netfs_io_subrequest *subreq,
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/8] afs: Fix missing wire-up of afs_retry_request()
2024-09-23 15:07 [PATCH 0/8] netfs, afs, cifs: Miscellaneous fixes/changes David Howells
2024-09-23 15:07 ` [PATCH 1/8] netfs: Fix mtime/ctime update for mmapped writes David Howells
2024-09-23 15:07 ` [PATCH 2/8] netfs: Drop the was_async arg from netfs_read_subreq_terminated() David Howells
@ 2024-09-23 15:07 ` David Howells
2024-09-23 15:07 ` [PATCH 4/8] afs: Remove unused struct and function prototype David Howells
` (4 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2024-09-23 15:07 UTC (permalink / raw)
To: Christian Brauner, Steve French, Marc Dionne
Cc: David Howells, Paulo Alcantara, Jeff Layton, Matthew Wilcox,
netfs, linux-afs, linux-cifs, linux-nfs, ceph-devel, v9fs,
linux-erofs, linux-fsdevel, linux-mm, linux-kernel,
Dr. David Alan Gilbert
afs_retry_request() is supposed to be pointed to by the afs_req_ops netfs
operations table, but the pointer got lost somewhere. The function is used
during writeback to rotate through the authentication keys that were in
force when the file was modified locally.
Fix this by adding the pointer to the function.
Fixes: 1ecb146f7cd8 ("netfs, afs: Use writeback retry to deal with alternate keys")
Reported-by: "Dr. David Alan Gilbert" <linux@treblig.org>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Jeff Layton <jlayton@kernel.org>
cc: linux-afs@lists.infradead.org
cc: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
---
fs/afs/file.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/afs/file.c b/fs/afs/file.c
index 1d30924cec5b..f717168da4ab 100644
--- a/fs/afs/file.c
+++ b/fs/afs/file.c
@@ -425,6 +425,7 @@ const struct netfs_request_ops afs_req_ops = {
.begin_writeback = afs_begin_writeback,
.prepare_write = afs_prepare_write,
.issue_write = afs_issue_write,
+ .retry_request = afs_retry_request,
};
static void afs_add_open_mmap(struct afs_vnode *vnode)
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/8] afs: Remove unused struct and function prototype
2024-09-23 15:07 [PATCH 0/8] netfs, afs, cifs: Miscellaneous fixes/changes David Howells
` (2 preceding siblings ...)
2024-09-23 15:07 ` [PATCH 3/8] afs: Fix missing wire-up of afs_retry_request() David Howells
@ 2024-09-23 15:07 ` David Howells
2024-09-27 8:07 ` (subset) " Christian Brauner
2024-09-23 15:07 ` [PATCH 5/8] afs: Fix possible infinite loop with unresponsive servers David Howells
` (3 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: David Howells @ 2024-09-23 15:07 UTC (permalink / raw)
To: Christian Brauner, Steve French, Marc Dionne
Cc: David Howells, Paulo Alcantara, Jeff Layton, Matthew Wilcox,
netfs, linux-afs, linux-cifs, linux-nfs, ceph-devel, v9fs,
linux-erofs, linux-fsdevel, linux-mm, linux-kernel, Thorsten Blum
From: Thorsten Blum <thorsten.blum@toblux.com>
The struct afs_address_list and the function prototype
afs_put_address_list() are not used anymore and can be removed. Remove
them.
Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
cc: linux-fsdevel@vger.kernel.org
Link: https://lore.kernel.org/r/20240911095046.3749-2-thorsten.blum@toblux.com/
---
fs/afs/afs_vl.h | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/fs/afs/afs_vl.h b/fs/afs/afs_vl.h
index 9c65ffb8a523..a06296c8827d 100644
--- a/fs/afs/afs_vl.h
+++ b/fs/afs/afs_vl.h
@@ -134,13 +134,4 @@ struct afs_uvldbentry__xdr {
__be32 spares9;
};
-struct afs_address_list {
- refcount_t usage;
- unsigned int version;
- unsigned int nr_addrs;
- struct sockaddr_rxrpc addrs[];
-};
-
-extern void afs_put_address_list(struct afs_address_list *alist);
-
#endif /* AFS_VL_H */
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: (subset) [PATCH 4/8] afs: Remove unused struct and function prototype
2024-09-23 15:07 ` [PATCH 4/8] afs: Remove unused struct and function prototype David Howells
@ 2024-09-27 8:07 ` Christian Brauner
0 siblings, 0 replies; 13+ messages in thread
From: Christian Brauner @ 2024-09-27 8:07 UTC (permalink / raw)
To: David Howells
Cc: Christian Brauner, Paulo Alcantara, Jeff Layton, Matthew Wilcox,
netfs, linux-afs, linux-cifs, linux-nfs, ceph-devel, v9fs,
linux-erofs, linux-fsdevel, linux-mm, linux-kernel, Thorsten Blum,
Steve French, Marc Dionne
On Mon, 23 Sep 2024 16:07:48 +0100, David Howells wrote:
> The struct afs_address_list and the function prototype
> afs_put_address_list() are not used anymore and can be removed. Remove
> them.
>
>
Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.
Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.
It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes
[4/8] afs: Remove unused struct and function prototype
https://git.kernel.org/vfs/vfs/c/45635ccabac2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 5/8] afs: Fix possible infinite loop with unresponsive servers
2024-09-23 15:07 [PATCH 0/8] netfs, afs, cifs: Miscellaneous fixes/changes David Howells
` (3 preceding siblings ...)
2024-09-23 15:07 ` [PATCH 4/8] afs: Remove unused struct and function prototype David Howells
@ 2024-09-23 15:07 ` David Howells
2024-09-27 8:05 ` (subset) " Christian Brauner
2024-09-23 15:07 ` [PATCH 6/8] afs: Fix the setting of the server responding flag David Howells
` (2 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: David Howells @ 2024-09-23 15:07 UTC (permalink / raw)
To: Christian Brauner, Steve French, Marc Dionne
Cc: David Howells, Paulo Alcantara, Jeff Layton, Matthew Wilcox,
netfs, linux-afs, linux-cifs, linux-nfs, ceph-devel, v9fs,
linux-erofs, linux-fsdevel, linux-mm, linux-kernel,
Markus Suvanto
From: Marc Dionne <marc.dionne@auristor.com>
A return code of 0 from afs_wait_for_one_fs_probe is an indication
that the endpoint state attached to the operation is stale and has
been superseded. In that case the iteration needs to be restarted
so that the newer probe result state gets used.
Failure to do so can result in an tight infinite loop around the
iterate_address label, where all addresses are thought to be responsive
and have been tried, with nothing to refresh the endpoint state.
Fixes: 495f2ae9e355 ("afs: Fix fileserver rotation")
Reported-by: Markus Suvanto <markus.suvanto@gmail.com>
Link: https://lists.infradead.org/pipermail/linux-afs/2024-July/008628.html
cc: linux-afs@lists.infradead.org
Signed-off-by: Marc Dionne <marc.dionne@auristor.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Link: https://lore.kernel.org/r/20240906134019.131553-1-marc.dionne@auristor.com/
---
fs/afs/fs_probe.c | 4 ++--
fs/afs/rotate.c | 11 ++++++++---
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/fs/afs/fs_probe.c b/fs/afs/fs_probe.c
index 580de4adaaf6..b516d05b0fef 100644
--- a/fs/afs/fs_probe.c
+++ b/fs/afs/fs_probe.c
@@ -506,10 +506,10 @@ int afs_wait_for_one_fs_probe(struct afs_server *server, struct afs_endpoint_sta
finish_wait(&server->probe_wq, &wait);
dont_wait:
- if (estate->responsive_set & ~exclude)
- return 1;
if (test_bit(AFS_ESTATE_SUPERSEDED, &estate->flags))
return 0;
+ if (estate->responsive_set & ~exclude)
+ return 1;
if (is_intr && signal_pending(current))
return -ERESTARTSYS;
if (timo == 0)
diff --git a/fs/afs/rotate.c b/fs/afs/rotate.c
index ed09d4d4c211..d612983d6f38 100644
--- a/fs/afs/rotate.c
+++ b/fs/afs/rotate.c
@@ -632,8 +632,10 @@ bool afs_select_fileserver(struct afs_operation *op)
wait_for_more_probe_results:
error = afs_wait_for_one_fs_probe(op->server, op->estate, op->addr_tried,
!(op->flags & AFS_OPERATION_UNINTR));
- if (!error)
+ if (error == 1)
goto iterate_address;
+ if (!error)
+ goto restart_from_beginning;
/* We've now had a failure to respond on all of a server's addresses -
* immediately probe them again and consider retrying the server.
@@ -644,10 +646,13 @@ bool afs_select_fileserver(struct afs_operation *op)
error = afs_wait_for_one_fs_probe(op->server, op->estate, op->addr_tried,
!(op->flags & AFS_OPERATION_UNINTR));
switch (error) {
- case 0:
+ case 1:
op->flags &= ~AFS_OPERATION_RETRY_SERVER;
- trace_afs_rotate(op, afs_rotate_trace_retry_server, 0);
+ trace_afs_rotate(op, afs_rotate_trace_retry_server, 1);
goto retry_server;
+ case 0:
+ trace_afs_rotate(op, afs_rotate_trace_retry_server, 0);
+ goto restart_from_beginning;
case -ERESTARTSYS:
afs_op_set_error(op, error);
goto failed;
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: (subset) [PATCH 5/8] afs: Fix possible infinite loop with unresponsive servers
2024-09-23 15:07 ` [PATCH 5/8] afs: Fix possible infinite loop with unresponsive servers David Howells
@ 2024-09-27 8:05 ` Christian Brauner
0 siblings, 0 replies; 13+ messages in thread
From: Christian Brauner @ 2024-09-27 8:05 UTC (permalink / raw)
To: David Howells
Cc: Christian Brauner, Paulo Alcantara, Jeff Layton, Matthew Wilcox,
netfs, linux-afs, linux-cifs, linux-nfs, ceph-devel, v9fs,
linux-erofs, linux-fsdevel, linux-mm, linux-kernel,
Markus Suvanto, Steve French, Marc Dionne
On Mon, 23 Sep 2024 16:07:49 +0100, David Howells wrote:
> A return code of 0 from afs_wait_for_one_fs_probe is an indication
> that the endpoint state attached to the operation is stale and has
> been superseded. In that case the iteration needs to be restarted
> so that the newer probe result state gets used.
>
> Failure to do so can result in an tight infinite loop around the
> iterate_address label, where all addresses are thought to be responsive
> and have been tried, with nothing to refresh the endpoint state.
>
> [...]
Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.
Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.
It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes
[5/8] afs: Fix possible infinite loop with unresponsive servers
https://git.kernel.org/vfs/vfs/c/6e45740867ee
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 6/8] afs: Fix the setting of the server responding flag
2024-09-23 15:07 [PATCH 0/8] netfs, afs, cifs: Miscellaneous fixes/changes David Howells
` (4 preceding siblings ...)
2024-09-23 15:07 ` [PATCH 5/8] afs: Fix possible infinite loop with unresponsive servers David Howells
@ 2024-09-23 15:07 ` David Howells
2024-09-27 8:07 ` (subset) " Christian Brauner
2024-09-23 15:07 ` [PATCH 7/8] cifs: Fix reversion of the iter in cifs_readv_receive() David Howells
2024-09-23 15:07 ` [PATCH 8/8] cifs: Make the write_{enter,done,err} tracepoints display netfs info David Howells
7 siblings, 1 reply; 13+ messages in thread
From: David Howells @ 2024-09-23 15:07 UTC (permalink / raw)
To: Christian Brauner, Steve French, Marc Dionne
Cc: David Howells, Paulo Alcantara, Jeff Layton, Matthew Wilcox,
netfs, linux-afs, linux-cifs, linux-nfs, ceph-devel, v9fs,
linux-erofs, linux-fsdevel, linux-mm, linux-kernel
In afs_wait_for_operation(), we set transcribe the call responded flag to
the server record that we used after doing the fileserver iteration loop -
but it's possible to exit the loop having had a response from the server
that we've discarded (e.g. it returned an abort or we started receiving
data, but the call didn't complete).
This means that op->server might be NULL, but we don't check that before
attempting to set the server flag.
Fixes: 98f9fda2057b ("afs: Fold the afs_addr_cursor struct in")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
---
fs/afs/fs_operation.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/afs/fs_operation.c b/fs/afs/fs_operation.c
index 3546b087e791..428721bbe4f6 100644
--- a/fs/afs/fs_operation.c
+++ b/fs/afs/fs_operation.c
@@ -201,7 +201,7 @@ void afs_wait_for_operation(struct afs_operation *op)
}
}
- if (op->call_responded)
+ if (op->call_responded && op->server)
set_bit(AFS_SERVER_FL_RESPONDING, &op->server->flags);
if (!afs_op_error(op)) {
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: (subset) [PATCH 6/8] afs: Fix the setting of the server responding flag
2024-09-23 15:07 ` [PATCH 6/8] afs: Fix the setting of the server responding flag David Howells
@ 2024-09-27 8:07 ` Christian Brauner
0 siblings, 0 replies; 13+ messages in thread
From: Christian Brauner @ 2024-09-27 8:07 UTC (permalink / raw)
To: David Howells
Cc: Christian Brauner, Paulo Alcantara, Jeff Layton, Matthew Wilcox,
netfs, linux-afs, linux-cifs, linux-nfs, ceph-devel, v9fs,
linux-erofs, linux-fsdevel, linux-mm, linux-kernel, Steve French,
Marc Dionne
On Mon, 23 Sep 2024 16:07:50 +0100, David Howells wrote:
> In afs_wait_for_operation(), we set transcribe the call responded flag to
> the server record that we used after doing the fileserver iteration loop -
> but it's possible to exit the loop having had a response from the server
> that we've discarded (e.g. it returned an abort or we started receiving
> data, but the call didn't complete).
>
> This means that op->server might be NULL, but we don't check that before
> attempting to set the server flag.
>
> [...]
Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.
Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.
It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes
[6/8] afs: Fix the setting of the server responding flag
https://git.kernel.org/vfs/vfs/c/830c1b2c1c28
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 7/8] cifs: Fix reversion of the iter in cifs_readv_receive().
2024-09-23 15:07 [PATCH 0/8] netfs, afs, cifs: Miscellaneous fixes/changes David Howells
` (5 preceding siblings ...)
2024-09-23 15:07 ` [PATCH 6/8] afs: Fix the setting of the server responding flag David Howells
@ 2024-09-23 15:07 ` David Howells
2024-09-23 15:07 ` [PATCH 8/8] cifs: Make the write_{enter,done,err} tracepoints display netfs info David Howells
7 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2024-09-23 15:07 UTC (permalink / raw)
To: Christian Brauner, Steve French, Marc Dionne
Cc: David Howells, Paulo Alcantara, Jeff Layton, Matthew Wilcox,
netfs, linux-afs, linux-cifs, linux-nfs, ceph-devel, v9fs,
linux-erofs, linux-fsdevel, linux-mm, linux-kernel,
Shyam Prasad N, Rohith Surabattula
cifs_read_iter_from_socket() copies the iterator that's passed in for the
socket to modify as and if it will, and then advances the original iterator
by the amount sent. However, both callers revert the advancement (although
receive_encrypted_read() zeros beyond the iterator first). The problem is,
though, that cifs_readv_receive() reverts by the original length, not the
amount transmitted which can cause an oops in iov_iter_revert().
Fix this by:
(1) Remove the iov_iter_advance() from cifs_read_iter_from_socket().
(2) Remove the iov_iter_revert() from both callers. This fixes the bug in
cifs_readv_receive().
(3) In receive_encrypted_read(), if we didn't get back as much data as the
buffer will hold, copy the iterator, advance the copy and use the copy
to drive iov_iter_zero().
As a bonus, this gets rid of some unnecessary work.
This was triggered by generic/074 with the "-o sign" mount option.
Fixes: 3ee1a1fc3981 ("cifs: Cut over to using netfslib")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Steve French <sfrench@samba.org>
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: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
---
fs/smb/client/connect.c | 6 +-----
fs/smb/client/smb2ops.c | 9 ++++++---
fs/smb/client/transport.c | 3 ---
3 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index 08a41c7aaf72..be6e632388f8 100644
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c
@@ -811,13 +811,9 @@ cifs_read_iter_from_socket(struct TCP_Server_Info *server, struct iov_iter *iter
unsigned int to_read)
{
struct msghdr smb_msg = { .msg_iter = *iter };
- int ret;
iov_iter_truncate(&smb_msg.msg_iter, to_read);
- ret = cifs_readv_from_socket(server, &smb_msg);
- if (ret > 0)
- iov_iter_advance(iter, ret);
- return ret;
+ return cifs_readv_from_socket(server, &smb_msg);
}
static bool
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 7381ec333c6d..1ee2dd4a1cae 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -4869,9 +4869,12 @@ receive_encrypted_read(struct TCP_Server_Info *server, struct mid_q_entry **mid,
goto discard_data;
server->total_read += rc;
- if (rc < len)
- iov_iter_zero(len - rc, &iter);
- iov_iter_revert(&iter, len);
+ if (rc < len) {
+ struct iov_iter tmp = iter;
+
+ iov_iter_advance(&tmp, rc);
+ iov_iter_zero(len - rc, &tmp);
+ }
iov_iter_truncate(&iter, dw->len);
rc = cifs_discard_remaining_data(server);
diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
index fd5a85d43759..91812150186c 100644
--- a/fs/smb/client/transport.c
+++ b/fs/smb/client/transport.c
@@ -1817,11 +1817,8 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
length = data_len; /* An RDMA read is already done. */
else
#endif
- {
length = cifs_read_iter_from_socket(server, &rdata->subreq.io_iter,
data_len);
- iov_iter_revert(&rdata->subreq.io_iter, data_len);
- }
if (length > 0)
rdata->got_bytes += length;
server->total_read += length;
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 8/8] cifs: Make the write_{enter,done,err} tracepoints display netfs info
2024-09-23 15:07 [PATCH 0/8] netfs, afs, cifs: Miscellaneous fixes/changes David Howells
` (6 preceding siblings ...)
2024-09-23 15:07 ` [PATCH 7/8] cifs: Fix reversion of the iter in cifs_readv_receive() David Howells
@ 2024-09-23 15:07 ` David Howells
7 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2024-09-23 15:07 UTC (permalink / raw)
To: Christian Brauner, Steve French, Marc Dionne
Cc: David Howells, Paulo Alcantara, Jeff Layton, Matthew Wilcox,
netfs, linux-afs, linux-cifs, linux-nfs, ceph-devel, v9fs,
linux-erofs, linux-fsdevel, linux-mm, linux-kernel, Steve French
Make the write RPC tracepoints use the same trace macro complexes as the
read tracepoints and display the netfs request and subrequest IDs where
available (see commit 519be989717c ("cifs: Add a tracepoint to track
credits involved in R/W requests")).
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Steve French <stfrench@microsoft.com>
cc: 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
---
fs/smb/client/smb2pdu.c | 22 +++++++++++++++-------
fs/smb/client/trace.h | 6 +++---
2 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 0b63608aeecb..9afc3baba27b 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -4858,7 +4858,9 @@ smb2_writev_callback(struct mid_q_entry *mid)
#endif
if (result) {
cifs_stats_fail_inc(tcon, SMB2_WRITE_HE);
- trace_smb3_write_err(wdata->xid,
+ trace_smb3_write_err(wdata->rreq->debug_id,
+ wdata->subreq.debug_index,
+ wdata->xid,
wdata->req->cfile->fid.persistent_fid,
tcon->tid, tcon->ses->Suid, wdata->subreq.start,
wdata->subreq.len, wdata->result);
@@ -4866,7 +4868,9 @@ smb2_writev_callback(struct mid_q_entry *mid)
pr_warn_once("Out of space writing to %s\n",
tcon->tree_name);
} else
- trace_smb3_write_done(0 /* no xid */,
+ trace_smb3_write_done(wdata->rreq->debug_id,
+ wdata->subreq.debug_index,
+ wdata->xid,
wdata->req->cfile->fid.persistent_fid,
tcon->tid, tcon->ses->Suid,
wdata->subreq.start, wdata->subreq.len);
@@ -4944,7 +4948,9 @@ smb2_async_writev(struct cifs_io_subrequest *wdata)
offsetof(struct smb2_write_req, Buffer));
req->RemainingBytes = 0;
- trace_smb3_write_enter(wdata->xid,
+ trace_smb3_write_enter(wdata->rreq->debug_id,
+ wdata->subreq.debug_index,
+ wdata->xid,
io_parms->persistent_fid,
io_parms->tcon->tid,
io_parms->tcon->ses->Suid,
@@ -5024,7 +5030,9 @@ smb2_async_writev(struct cifs_io_subrequest *wdata)
wdata, flags, &wdata->credits);
/* Can't touch wdata if rc == 0 */
if (rc) {
- trace_smb3_write_err(xid,
+ trace_smb3_write_err(wdata->rreq->debug_id,
+ wdata->subreq.debug_index,
+ xid,
io_parms->persistent_fid,
io_parms->tcon->tid,
io_parms->tcon->ses->Suid,
@@ -5104,7 +5112,7 @@ SMB2_write(const unsigned int xid, struct cifs_io_parms *io_parms,
offsetof(struct smb2_write_req, Buffer));
req->RemainingBytes = 0;
- trace_smb3_write_enter(xid, io_parms->persistent_fid,
+ trace_smb3_write_enter(0, 0, xid, io_parms->persistent_fid,
io_parms->tcon->tid, io_parms->tcon->ses->Suid,
io_parms->offset, io_parms->length);
@@ -5125,7 +5133,7 @@ SMB2_write(const unsigned int xid, struct cifs_io_parms *io_parms,
rsp = (struct smb2_write_rsp *)rsp_iov.iov_base;
if (rc) {
- trace_smb3_write_err(xid,
+ trace_smb3_write_err(0, 0, xid,
req->PersistentFileId,
io_parms->tcon->tid,
io_parms->tcon->ses->Suid,
@@ -5134,7 +5142,7 @@ SMB2_write(const unsigned int xid, struct cifs_io_parms *io_parms,
cifs_dbg(VFS, "Send error in write = %d\n", rc);
} else {
*nbytes = le32_to_cpu(rsp->DataLength);
- trace_smb3_write_done(xid,
+ trace_smb3_write_done(0, 0, xid,
req->PersistentFileId,
io_parms->tcon->tid,
io_parms->tcon->ses->Suid,
diff --git a/fs/smb/client/trace.h b/fs/smb/client/trace.h
index 8e9964001e2a..0b52d22a91a0 100644
--- a/fs/smb/client/trace.h
+++ b/fs/smb/client/trace.h
@@ -157,6 +157,7 @@ DEFINE_EVENT(smb3_rw_err_class, smb3_##name, \
TP_ARGS(rreq_debug_id, rreq_debug_index, xid, fid, tid, sesid, offset, len, rc))
DEFINE_SMB3_RW_ERR_EVENT(read_err);
+DEFINE_SMB3_RW_ERR_EVENT(write_err);
/* For logging errors in other file I/O ops */
DECLARE_EVENT_CLASS(smb3_other_err_class,
@@ -202,7 +203,6 @@ DEFINE_EVENT(smb3_other_err_class, smb3_##name, \
int rc), \
TP_ARGS(xid, fid, tid, sesid, offset, len, rc))
-DEFINE_SMB3_OTHER_ERR_EVENT(write_err);
DEFINE_SMB3_OTHER_ERR_EVENT(query_dir_err);
DEFINE_SMB3_OTHER_ERR_EVENT(zero_err);
DEFINE_SMB3_OTHER_ERR_EVENT(falloc_err);
@@ -370,6 +370,8 @@ DEFINE_EVENT(smb3_rw_done_class, smb3_##name, \
DEFINE_SMB3_RW_DONE_EVENT(read_enter);
DEFINE_SMB3_RW_DONE_EVENT(read_done);
+DEFINE_SMB3_RW_DONE_EVENT(write_enter);
+DEFINE_SMB3_RW_DONE_EVENT(write_done);
/* For logging successful other op */
DECLARE_EVENT_CLASS(smb3_other_done_class,
@@ -411,11 +413,9 @@ DEFINE_EVENT(smb3_other_done_class, smb3_##name, \
__u32 len), \
TP_ARGS(xid, fid, tid, sesid, offset, len))
-DEFINE_SMB3_OTHER_DONE_EVENT(write_enter);
DEFINE_SMB3_OTHER_DONE_EVENT(query_dir_enter);
DEFINE_SMB3_OTHER_DONE_EVENT(zero_enter);
DEFINE_SMB3_OTHER_DONE_EVENT(falloc_enter);
-DEFINE_SMB3_OTHER_DONE_EVENT(write_done);
DEFINE_SMB3_OTHER_DONE_EVENT(query_dir_done);
DEFINE_SMB3_OTHER_DONE_EVENT(zero_done);
DEFINE_SMB3_OTHER_DONE_EVENT(falloc_done);
^ permalink raw reply related [flat|nested] 13+ messages in thread