netfs, afs, cifs: Miscellaneous fixes/changes
Subsystems (10)
David Howells
maintainercover netfs, afs, cifs: Miscellaneous fixes/changes
Hi Christian, Steve, Marc,
Here are some miscellaneous fixes and changes for netfslib and the afs and cifs filesystems, some of which are already in the vfs or cifs trees, but I thought I'd repost them all for completeness, starting with netfs:
(1) Fix the update of mtime and ctime for mmapped files.
(2) Drop the was_async argument from netfs_read_subreq_terminated().
then afs:
(3) Wire up afs_retry_request() so that writeback rotates through the available keys.
(4) Remove some unused defs.
(5) Fix a potential infinite loop in the server rotation code.
(6) Fix an oops that can occur when a server responds, but we decide the operation failed (e.g. an abort).
and then cifs:
(7) Fix reversion of the I/O iterator causing cryptographically signed transport reception to fail.
(8) Alter the write tracepoints to display netfs request info.
The patches can also be found here:
https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=netfs-fixes
Thanks,
David
David Howells (6):
netfs: Fix mtime/ctime update for mmapped writes
netfs: Drop the was_async arg from netfs_read_subreq_terminated()
afs: Fix missing wire-up of afs_retry_request()
afs: Fix the setting of the server responding flag
cifs: Fix reversion of the iter in cifs_readv_receive().
cifs: Make the write_{enter,done,err} tracepoints display netfs info
Marc Dionne (1):
afs: Fix possible infinite loop with unresponsive servers
Thorsten Blum (1):
afs: Remove unused struct and function prototype
fs/9p/vfs_addr.c | 3 +- fs/afs/afs_vl.h | 9 ---- fs/afs/file.c | 16 ++++--- fs/afs/fs_operation.c | 2 +- fs/afs/fs_probe.c | 4 +- fs/afs/fsclient.c | 2 +- fs/afs/rotate.c | 11 +++-- fs/afs/yfsclient.c | 2 +- fs/ceph/addr.c | 13 ++++-- fs/netfs/buffered_read.c | 16 +++---- fs/netfs/buffered_write.c | 1 + 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/connect.c | 6 +-- fs/smb/client/file.c | 3 +- fs/smb/client/smb2ops.c | 9 ++-- fs/smb/client/smb2pdu.c | 32 ++++++------- fs/smb/client/trace.h | 6 +-- fs/smb/client/transport.c | 3 -- include/linux/netfs.h | 7 ++- 26 files changed, 139 insertions(+), 143 deletions(-)
David Howells
maintainerPATCH 1/8 netfs: Fix mtime/ctime update for mmapped writes
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().
--- fs/netfs/buffered_write.c | 1 + 1 file changed, 1 insertion(+)
fs/netfs/buffered_write.c +1 -0
@@ -552,6 +552,7 @@ vm_fault_t netfs_page_mkwrite(struct vm_fault *vmf, struct netfs_group *netfs_grtrace_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 ;
Re: [PATCH 1/8] netfs: Fix mtime/ctime update for mmapped writes
added to cifs-2.6.git for-next since it is important as it fixes a regression affecting cifs.ko
Quoted 40 lines · David Howells
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 asNETFS_ICTX_MODIFIED_ATTRto 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 <[email protected]>Signed-off-by David Howells <[email protected]>cc Jeff Layton <[email protected]>cc Steve French <[email protected]>cc Paulo Alcantara <[email protected]>--- fs/netfs/buffered_write.c | 1 + 1 file changed, 1 insertion(+)fs/netfs/buffered_write.c +1
-0@@ -552,6 +552,7 @@ vm_fault_t netfs_page_mkwrite(struct vm_fault *vmf, struct netfs_group *netfs_grtrace_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 ;
Signature · 3 lines
Thanks, Steve
David Howells
maintainerPATCH 2/8 netfs: Drop the was_async arg from netfs_read_subreq_terminated()
In reply to David HowellsDrop 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.
--- 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(-)
fs/9p/vfs_addr.c +2 -1
@@ -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); }/**
fs/afs/file.c +10 -5
@@ -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;
fs/afs/fsclient.c +1 -1
@@ -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;
fs/afs/yfsclient.c +1 -1
@@ -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;
fs/ceph/addr.c +9 -4
@@ -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); }
fs/netfs/buffered_read.c +8 -8
@@ -183,13 +183,12 @@ static void netfs_cache_read_terminated(void *priv, ssize_t transferred_or_errorstruct 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);
fs/netfs/direct_read.c +1 -1
@@ -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; }
fs/netfs/internal.h +1 -1
@@ -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
fs/netfs/objects.c +15 -2
@@ -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;
fs/netfs/read_collect.c +39 -56
190 lines not on this page · raw message
fs/netfs/read_retry.c +1 -1
@@ -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); }/*
fs/nfs/fscache.c +4 -2
@@ -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 */
fs/nfs/fscache.h +2 -1
@@ -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)
fs/smb/client/cifssmb.c +1 -9
@@ -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 );
fs/smb/client/file.c +2 -1
@@ -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); }/*
fs/smb/client/smb2pdu.c +1 -9
@@ -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,
include/linux/netfs.h +3 -4
@@ -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,
David Howells
maintainerPATCH 3/8 afs: Fix missing wire-up of afs_retry_request()
In reply to David Howellsafs_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.
--- fs/afs/file.c | 1 + 1 file changed, 1 insertion(+)
fs/afs/file.c +1 -0
@@ -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)
David Howells
maintainerPATCH 4/8 afs: Remove unused struct and function prototype
In reply to David HowellsFrom: Thorsten Blum <[email protected]>
The struct afs_address_list and the function prototype afs_put_address_list() are not used anymore and can be removed. Remove them.
--- fs/afs/afs_vl.h | 9 --------- 1 file changed, 9 deletions(-)
fs/afs/afs_vl.h +0 -9
@@ -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 */
Re: (subset) [PATCH 4/8] afs: Remove unused struct and function prototype
Quoted 5 lines · David Howells
The struct
afs_address_listand the function prototypeafs_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
David Howells
maintainerPATCH 5/8 afs: Fix possible infinite loop with unresponsive servers
In reply to David HowellsFrom: Marc Dionne <[email protected]>
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.
--- fs/afs/fs_probe.c | 4 ++-- fs/afs/rotate.c | 11 ++++++++--- 2 files changed, 10 insertions(+), 5 deletions(-)
fs/afs/fs_probe.c +2 -2
@@ -506,10 +506,10 @@ int afs_wait_for_one_fs_probe(struct afs_server *server, struct afs_endpoint_stafinish_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 )
fs/afs/rotate.c +8 -3
@@ -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;
Re: (subset) [PATCH 5/8] afs: Fix possible infinite loop with unresponsive servers
Quoted 10 lines · David Howells
A return code of 0 from
afs_wait_for_one_fs_probeis 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_addresslabel, 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
David Howells
maintainerPATCH 6/8 afs: Fix the setting of the server responding flag
In reply to David HowellsIn 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.
--- fs/afs/fs_operation.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/afs/fs_operation.c +1 -1
@@ -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)) {
Re: (subset) [PATCH 6/8] afs: Fix the setting of the server responding flag
Quoted 10 lines · David Howells
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->servermight 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
David Howells
maintainerPATCH 7/8 cifs: Fix reversion of the iter in cifs_readv_receive().
In reply to David Howellscifs_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.
--- 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(-)
fs/smb/client/connect.c +1 -5
@@ -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
fs/smb/client/smb2ops.c +6 -3
@@ -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);
fs/smb/client/transport.c +0 -3
@@ -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;
David Howells
maintainerPATCH 8/8 cifs: Make the write_{enter,done,err} tracepoints display netfs info
In reply to David HowellsMake 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")).
--- fs/smb/client/smb2pdu.c | 22 +++++++++++++++------- fs/smb/client/trace.h | 6 +++--- 2 files changed, 18 insertions(+), 10 deletions(-)
fs/smb/client/smb2pdu.c +15 -7
@@ -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,
fs/smb/client/trace.h +3 -3
@@ -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);