From 2a501f55cd641eb4d3c16a2eab0d678693fac663 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Fri, 15 Dec 2023 11:56:31 +1100 Subject: nfsd: call nfsd_last_thread() before final nfsd_put() If write_ports_addfd or write_ports_addxprt fail, they call nfsd_put() without calling nfsd_last_thread(). This leaves nn->nfsd_serv pointing to a structure that has been freed. So remove 'static' from nfsd_last_thread() and call it when the nfsd_serv is about to be destroyed. Fixes: ec52361df99b ("SUNRPC: stop using ->sv_nrthreads as a refcount") Signed-off-by: NeilBrown Reviewed-by: Jeff Layton Cc: Signed-off-by: Chuck Lever --- fs/nfsd/nfsctl.c | 9 +++++++-- fs/nfsd/nfsd.h | 1 + fs/nfsd/nfssvc.c | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index 739ed5bf71cd..79efb1075f38 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -705,8 +705,10 @@ static ssize_t __write_ports_addfd(char *buf, struct net *net, const struct cred err = svc_addsock(nn->nfsd_serv, net, fd, buf, SIMPLE_TRANSACTION_LIMIT, cred); - if (err >= 0 && - !nn->nfsd_serv->sv_nrthreads && !xchg(&nn->keep_active, 1)) + if (err < 0 && !nn->nfsd_serv->sv_nrthreads && !nn->keep_active) + nfsd_last_thread(net); + else if (err >= 0 && + !nn->nfsd_serv->sv_nrthreads && !xchg(&nn->keep_active, 1)) svc_get(nn->nfsd_serv); nfsd_put(net); @@ -757,6 +759,9 @@ out_close: svc_xprt_put(xprt); } out_err: + if (!nn->nfsd_serv->sv_nrthreads && !nn->keep_active) + nfsd_last_thread(net); + nfsd_put(net); return err; } diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h index f5ff42f41ee7..3286ffacbc56 100644 --- a/fs/nfsd/nfsd.h +++ b/fs/nfsd/nfsd.h @@ -155,6 +155,7 @@ int nfsd_vers(struct nfsd_net *nn, int vers, enum vers_op change); int nfsd_minorversion(struct nfsd_net *nn, u32 minorversion, enum vers_op change); void nfsd_reset_versions(struct nfsd_net *nn); int nfsd_create_serv(struct net *net); +void nfsd_last_thread(struct net *net); extern int nfsd_max_blksize; diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c index fe61d9bbcc1f..d6939e23ffcf 100644 --- a/fs/nfsd/nfssvc.c +++ b/fs/nfsd/nfssvc.c @@ -542,7 +542,7 @@ static struct notifier_block nfsd_inet6addr_notifier = { /* Only used under nfsd_mutex, so this atomic may be overkill: */ static atomic_t nfsd_notifier_refcount = ATOMIC_INIT(0); -static void nfsd_last_thread(struct net *net) +void nfsd_last_thread(struct net *net) { struct nfsd_net *nn = net_generic(net, nfsd_net_id); struct svc_serv *serv = nn->nfsd_serv; -- cgit From 1bd773b4f0c90123af19a853244be61518ae0556 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Fri, 15 Dec 2023 11:56:33 +1100 Subject: nfsd: hold nfsd_mutex across entire netlink operation Rather than using svc_get() and svc_put() to hold a stable reference to the nfsd_svc for netlink lookups, simply hold the mutex for the entire time. The "entire" time isn't very long, and the mutex is not often contented. This makes way for us to remove the refcounts of svc, which is more confusing than useful. Reported-by: Jeff Layton Closes: https://lore.kernel.org/linux-nfs/5d9bbb599569ce29f16e4e0eef6b291eda0f375b.camel@kernel.org/T/#u Fixes: bd9d6a3efa97 ("NFSD: add rpc_status netlink support") Signed-off-by: NeilBrown Reviewed-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfsd/nfsctl.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index 79efb1075f38..05b55db33424 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -1515,11 +1515,10 @@ int nfsd_nl_rpc_status_get_start(struct netlink_callback *cb) int ret = -ENODEV; mutex_lock(&nfsd_mutex); - if (nn->nfsd_serv) { - svc_get(nn->nfsd_serv); + if (nn->nfsd_serv) ret = 0; - } - mutex_unlock(&nfsd_mutex); + else + mutex_unlock(&nfsd_mutex); return ret; } @@ -1691,8 +1690,6 @@ out: */ int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb) { - mutex_lock(&nfsd_mutex); - nfsd_put(sock_net(cb->skb->sk)); mutex_unlock(&nfsd_mutex); return 0; -- cgit From 862bee84d77fa01cc8929656ae77781abf917863 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 16 Dec 2023 11:57:43 -0500 Subject: NFSD: Revert 6c41d9a9bd0298002805758216a9c44e38a8500d For some reason, the wait_on_bit() in nfsd4_deleg_getattr_conflict() is waiting forever, preventing a clean server shutdown. The requesting client might also hang waiting for a reply to the conflicting GETATTR. Invoking wait_on_bit() in an nfsd thread context is a hazard. The correct fix is to replace this wait_on_bit() call site with a mechanism that defers the conflicting GETATTR until the CB_GETATTR completes or is known to have failed. That will require some surgery and extended testing and it's late in the v6.7-rc cycle, so I'm reverting now in favor of trying again in a subsequent kernel release. This is my fault: I should have recognized the ramifications of calling wait_on_bit() in here before accepting this patch. Thanks to Dai Ngo for diagnosing the issue. Reported-by: Wolfgang Walter Closes: https://lore.kernel.org/linux-nfs/e3d43ecdad554fbdcaa7181833834f78@stwm.de/ Signed-off-by: Chuck Lever --- fs/nfsd/nfs4state.c | 114 +++++----------------------------------------------- fs/nfsd/nfs4xdr.c | 7 +--- fs/nfsd/state.h | 11 +---- 3 files changed, 14 insertions(+), 118 deletions(-) diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 3709e58f0a4a..31909f59d319 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -127,7 +127,6 @@ static void free_session(struct nfsd4_session *); static const struct nfsd4_callback_ops nfsd4_cb_recall_ops; static const struct nfsd4_callback_ops nfsd4_cb_notify_lock_ops; -static const struct nfsd4_callback_ops nfsd4_cb_getattr_ops; static struct workqueue_struct *laundry_wq; @@ -1190,10 +1189,6 @@ alloc_init_deleg(struct nfs4_client *clp, struct nfs4_file *fp, dp->dl_recalled = false; nfsd4_init_cb(&dp->dl_recall, dp->dl_stid.sc_client, &nfsd4_cb_recall_ops, NFSPROC4_CLNT_CB_RECALL); - nfsd4_init_cb(&dp->dl_cb_fattr.ncf_getattr, dp->dl_stid.sc_client, - &nfsd4_cb_getattr_ops, NFSPROC4_CLNT_CB_GETATTR); - dp->dl_cb_fattr.ncf_file_modified = false; - dp->dl_cb_fattr.ncf_cb_bmap[0] = FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE; get_nfs4_file(fp); dp->dl_stid.sc_file = fp; return dp; @@ -2901,56 +2896,11 @@ nfsd4_cb_recall_any_release(struct nfsd4_callback *cb) spin_unlock(&nn->client_lock); } -static int -nfsd4_cb_getattr_done(struct nfsd4_callback *cb, struct rpc_task *task) -{ - struct nfs4_cb_fattr *ncf = - container_of(cb, struct nfs4_cb_fattr, ncf_getattr); - - ncf->ncf_cb_status = task->tk_status; - switch (task->tk_status) { - case -NFS4ERR_DELAY: - rpc_delay(task, 2 * HZ); - return 0; - default: - return 1; - } -} - -static void -nfsd4_cb_getattr_release(struct nfsd4_callback *cb) -{ - struct nfs4_cb_fattr *ncf = - container_of(cb, struct nfs4_cb_fattr, ncf_getattr); - struct nfs4_delegation *dp = - container_of(ncf, struct nfs4_delegation, dl_cb_fattr); - - nfs4_put_stid(&dp->dl_stid); - clear_bit(CB_GETATTR_BUSY, &ncf->ncf_cb_flags); - wake_up_bit(&ncf->ncf_cb_flags, CB_GETATTR_BUSY); -} - static const struct nfsd4_callback_ops nfsd4_cb_recall_any_ops = { .done = nfsd4_cb_recall_any_done, .release = nfsd4_cb_recall_any_release, }; -static const struct nfsd4_callback_ops nfsd4_cb_getattr_ops = { - .done = nfsd4_cb_getattr_done, - .release = nfsd4_cb_getattr_release, -}; - -void nfs4_cb_getattr(struct nfs4_cb_fattr *ncf) -{ - struct nfs4_delegation *dp = - container_of(ncf, struct nfs4_delegation, dl_cb_fattr); - - if (test_and_set_bit(CB_GETATTR_BUSY, &ncf->ncf_cb_flags)) - return; - refcount_inc(&dp->dl_stid.sc_count); - nfsd4_run_cb(&ncf->ncf_getattr); -} - static struct nfs4_client *create_client(struct xdr_netobj name, struct svc_rqst *rqstp, nfs4_verifier *verf) { @@ -5686,8 +5636,6 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp, struct svc_fh *parent = NULL; int cb_up; int status = 0; - struct kstat stat; - struct path path; cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client); open->op_recall = false; @@ -5725,18 +5673,6 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp, if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) { open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE; trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid); - path.mnt = currentfh->fh_export->ex_path.mnt; - path.dentry = currentfh->fh_dentry; - if (vfs_getattr(&path, &stat, - (STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE), - AT_STATX_SYNC_AS_STAT)) { - nfs4_put_stid(&dp->dl_stid); - destroy_delegation(dp); - goto out_no_deleg; - } - dp->dl_cb_fattr.ncf_cur_fsize = stat.size; - dp->dl_cb_fattr.ncf_initial_cinfo = - nfsd4_change_attribute(&stat, d_inode(currentfh->fh_dentry)); } else { open->op_delegate_type = NFS4_OPEN_DELEGATE_READ; trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid); @@ -8489,8 +8425,6 @@ nfsd4_get_writestateid(struct nfsd4_compound_state *cstate, * nfsd4_deleg_getattr_conflict - Recall if GETATTR causes conflict * @rqstp: RPC transaction context * @inode: file to be checked for a conflict - * @modified: return true if file was modified - * @size: new size of file if modified is true * * This function is called when there is a conflict between a write * delegation and a change/size GETATTR from another client. The server @@ -8499,23 +8433,21 @@ nfsd4_get_writestateid(struct nfsd4_compound_state *cstate, * delegation before replying to the GETATTR. See RFC 8881 section * 18.7.4. * + * The current implementation does not support CB_GETATTR yet. However + * this can avoid recalling the delegation could be added in follow up + * work. + * * Returns 0 if there is no conflict; otherwise an nfs_stat * code is returned. */ __be32 -nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct inode *inode, - bool *modified, u64 *size) +nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct inode *inode) { + __be32 status; struct file_lock_context *ctx; - struct nfs4_delegation *dp; - struct nfs4_cb_fattr *ncf; struct file_lock *fl; - struct iattr attrs; - __be32 status; - - might_sleep(); + struct nfs4_delegation *dp; - *modified = false; ctx = locks_inode_context(inode); if (!ctx) return 0; @@ -8542,34 +8474,10 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct inode *inode, break_lease: spin_unlock(&ctx->flc_lock); nfsd_stats_wdeleg_getattr_inc(); - - dp = fl->fl_owner; - ncf = &dp->dl_cb_fattr; - nfs4_cb_getattr(&dp->dl_cb_fattr); - wait_on_bit(&ncf->ncf_cb_flags, CB_GETATTR_BUSY, TASK_INTERRUPTIBLE); - if (ncf->ncf_cb_status) { - status = nfserrno(nfsd_open_break_lease(inode, NFSD_MAY_READ)); - if (status != nfserr_jukebox || - !nfsd_wait_for_delegreturn(rqstp, inode)) - return status; - } - if (!ncf->ncf_file_modified && - (ncf->ncf_initial_cinfo != ncf->ncf_cb_change || - ncf->ncf_cur_fsize != ncf->ncf_cb_fsize)) - ncf->ncf_file_modified = true; - if (ncf->ncf_file_modified) { - /* - * The server would not update the file's metadata - * with the client's modified size. - */ - attrs.ia_mtime = attrs.ia_ctime = current_time(inode); - attrs.ia_valid = ATTR_MTIME | ATTR_CTIME; - setattr_copy(&nop_mnt_idmap, inode, &attrs); - mark_inode_dirty(inode); - ncf->ncf_cur_fsize = ncf->ncf_cb_fsize; - *size = ncf->ncf_cur_fsize; - *modified = true; - } + status = nfserrno(nfsd_open_break_lease(inode, NFSD_MAY_READ)); + if (status != nfserr_jukebox || + !nfsd_wait_for_delegreturn(rqstp, inode)) + return status; return 0; } break; diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c index ec4ed6206df1..b499fe9caa32 100644 --- a/fs/nfsd/nfs4xdr.c +++ b/fs/nfsd/nfs4xdr.c @@ -3505,9 +3505,7 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr, u32 attrmask[3]; unsigned long mask[2]; } u; - bool file_modified; unsigned long bit; - u64 size = 0; WARN_ON_ONCE(bmval[1] & NFSD_WRITEONLY_ATTRS_WORD1); WARN_ON_ONCE(!nfsd_attrs_supported(minorversion, bmval)); @@ -3534,8 +3532,7 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr, } args.size = 0; if (u.attrmask[0] & (FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE)) { - status = nfsd4_deleg_getattr_conflict(rqstp, d_inode(dentry), - &file_modified, &size); + status = nfsd4_deleg_getattr_conflict(rqstp, d_inode(dentry)); if (status) goto out; } @@ -3545,7 +3542,7 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr, AT_STATX_SYNC_AS_STAT); if (err) goto out_nfserr; - args.size = file_modified ? size : args.stat.size; + args.size = args.stat.size; if (!(args.stat.result_mask & STATX_BTIME)) /* underlying FS does not offer btime so we can't share it */ diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h index f96eaa8e9413..0bbbe57e027d 100644 --- a/fs/nfsd/state.h +++ b/fs/nfsd/state.h @@ -125,16 +125,8 @@ struct nfs4_cb_fattr { /* from CB_GETATTR reply */ u64 ncf_cb_change; u64 ncf_cb_fsize; - - unsigned long ncf_cb_flags; - bool ncf_file_modified; - u64 ncf_initial_cinfo; - u64 ncf_cur_fsize; }; -/* bits for ncf_cb_flags */ -#define CB_GETATTR_BUSY 0 - /* * Represents a delegation stateid. The nfs4_client holds references to these * and they are put when it is being destroyed or when the delegation is @@ -754,6 +746,5 @@ static inline bool try_to_expire_client(struct nfs4_client *clp) } extern __be32 nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, - struct inode *inode, bool *file_modified, u64 *size); -extern void nfs4_cb_getattr(struct nfs4_cb_fattr *ncf); + struct inode *inode); #endif /* NFSD4_STATE_H */ -- cgit From 1227561c2ffb81ab09ead21cce6438f59276aa6e Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 16 Dec 2023 12:12:50 -0500 Subject: NFSD: Revert 738401a9bd1ac34ccd5723d69640a4adbb1a4bc0 There's nothing wrong with this commit, but this is dead code now that nothing triggers a CB_GETATTR callback. It can be re-introduced once the issues with handling conflicting GETATTRs are resolved. Signed-off-by: Chuck Lever --- fs/nfsd/nfs4callback.c | 97 +------------------------------------------------- fs/nfsd/state.h | 14 -------- fs/nfsd/xdr4cb.h | 18 ---------- 3 files changed, 1 insertion(+), 128 deletions(-) diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c index 92bc109dabe6..4039ffcf90ba 100644 --- a/fs/nfsd/nfs4callback.c +++ b/fs/nfsd/nfs4callback.c @@ -84,21 +84,7 @@ static void encode_uint32(struct xdr_stream *xdr, u32 n) static void encode_bitmap4(struct xdr_stream *xdr, const __u32 *bitmap, size_t len) { - xdr_stream_encode_uint32_array(xdr, bitmap, len); -} - -static int decode_cb_fattr4(struct xdr_stream *xdr, uint32_t *bitmap, - struct nfs4_cb_fattr *fattr) -{ - fattr->ncf_cb_change = 0; - fattr->ncf_cb_fsize = 0; - if (bitmap[0] & FATTR4_WORD0_CHANGE) - if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_change) < 0) - return -NFSERR_BAD_XDR; - if (bitmap[0] & FATTR4_WORD0_SIZE) - if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_fsize) < 0) - return -NFSERR_BAD_XDR; - return 0; + WARN_ON_ONCE(xdr_stream_encode_uint32_array(xdr, bitmap, len) < 0); } /* @@ -371,30 +357,6 @@ encode_cb_recallany4args(struct xdr_stream *xdr, hdr->nops++; } -/* - * CB_GETATTR4args - * struct CB_GETATTR4args { - * nfs_fh4 fh; - * bitmap4 attr_request; - * }; - * - * The size and change attributes are the only one - * guaranteed to be serviced by the client. - */ -static void -encode_cb_getattr4args(struct xdr_stream *xdr, struct nfs4_cb_compound_hdr *hdr, - struct nfs4_cb_fattr *fattr) -{ - struct nfs4_delegation *dp = - container_of(fattr, struct nfs4_delegation, dl_cb_fattr); - struct knfsd_fh *fh = &dp->dl_stid.sc_file->fi_fhandle; - - encode_nfs_cb_opnum4(xdr, OP_CB_GETATTR); - encode_nfs_fh4(xdr, fh); - encode_bitmap4(xdr, fattr->ncf_cb_bmap, ARRAY_SIZE(fattr->ncf_cb_bmap)); - hdr->nops++; -} - /* * CB_SEQUENCE4args * @@ -530,26 +492,6 @@ static void nfs4_xdr_enc_cb_null(struct rpc_rqst *req, struct xdr_stream *xdr, xdr_reserve_space(xdr, 0); } -/* - * 20.1. Operation 3: CB_GETATTR - Get Attributes - */ -static void nfs4_xdr_enc_cb_getattr(struct rpc_rqst *req, - struct xdr_stream *xdr, const void *data) -{ - const struct nfsd4_callback *cb = data; - struct nfs4_cb_fattr *ncf = - container_of(cb, struct nfs4_cb_fattr, ncf_getattr); - struct nfs4_cb_compound_hdr hdr = { - .ident = cb->cb_clp->cl_cb_ident, - .minorversion = cb->cb_clp->cl_minorversion, - }; - - encode_cb_compound4args(xdr, &hdr); - encode_cb_sequence4args(xdr, cb, &hdr); - encode_cb_getattr4args(xdr, &hdr, ncf); - encode_cb_nops(&hdr); -} - /* * 20.2. Operation 4: CB_RECALL - Recall a Delegation */ @@ -605,42 +547,6 @@ static int nfs4_xdr_dec_cb_null(struct rpc_rqst *req, struct xdr_stream *xdr, return 0; } -/* - * 20.1. Operation 3: CB_GETATTR - Get Attributes - */ -static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp, - struct xdr_stream *xdr, - void *data) -{ - struct nfsd4_callback *cb = data; - struct nfs4_cb_compound_hdr hdr; - int status; - u32 bitmap[3] = {0}; - u32 attrlen; - struct nfs4_cb_fattr *ncf = - container_of(cb, struct nfs4_cb_fattr, ncf_getattr); - - status = decode_cb_compound4res(xdr, &hdr); - if (unlikely(status)) - return status; - - status = decode_cb_sequence4res(xdr, cb); - if (unlikely(status || cb->cb_seq_status)) - return status; - - status = decode_cb_op_status(xdr, OP_CB_GETATTR, &cb->cb_status); - if (status) - return status; - if (xdr_stream_decode_uint32_array(xdr, bitmap, 3) < 0) - return -NFSERR_BAD_XDR; - if (xdr_stream_decode_u32(xdr, &attrlen) < 0) - return -NFSERR_BAD_XDR; - if (attrlen > (sizeof(ncf->ncf_cb_change) + sizeof(ncf->ncf_cb_fsize))) - return -NFSERR_BAD_XDR; - status = decode_cb_fattr4(xdr, bitmap, ncf); - return status; -} - /* * 20.2. Operation 4: CB_RECALL - Recall a Delegation */ @@ -949,7 +855,6 @@ static const struct rpc_procinfo nfs4_cb_procedures[] = { PROC(CB_NOTIFY_LOCK, COMPOUND, cb_notify_lock, cb_notify_lock), PROC(CB_OFFLOAD, COMPOUND, cb_offload, cb_offload), PROC(CB_RECALL_ANY, COMPOUND, cb_recall_any, cb_recall_any), - PROC(CB_GETATTR, COMPOUND, cb_getattr, cb_getattr), }; static unsigned int nfs4_cb_counts[ARRAY_SIZE(nfs4_cb_procedures)]; diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h index 0bbbe57e027d..41bdc913fa71 100644 --- a/fs/nfsd/state.h +++ b/fs/nfsd/state.h @@ -117,16 +117,6 @@ struct nfs4_cpntf_state { time64_t cpntf_time; /* last time stateid used */ }; -struct nfs4_cb_fattr { - struct nfsd4_callback ncf_getattr; - u32 ncf_cb_status; - u32 ncf_cb_bmap[1]; - - /* from CB_GETATTR reply */ - u64 ncf_cb_change; - u64 ncf_cb_fsize; -}; - /* * Represents a delegation stateid. The nfs4_client holds references to these * and they are put when it is being destroyed or when the delegation is @@ -160,9 +150,6 @@ struct nfs4_delegation { int dl_retries; struct nfsd4_callback dl_recall; bool dl_recalled; - - /* for CB_GETATTR */ - struct nfs4_cb_fattr dl_cb_fattr; }; #define cb_to_delegation(cb) \ @@ -653,7 +640,6 @@ enum nfsd4_cb_op { NFSPROC4_CLNT_CB_SEQUENCE, NFSPROC4_CLNT_CB_NOTIFY_LOCK, NFSPROC4_CLNT_CB_RECALL_ANY, - NFSPROC4_CLNT_CB_GETATTR, }; /* Returns true iff a is later than b: */ diff --git a/fs/nfsd/xdr4cb.h b/fs/nfsd/xdr4cb.h index e8b00309c449..0d39af1b00a0 100644 --- a/fs/nfsd/xdr4cb.h +++ b/fs/nfsd/xdr4cb.h @@ -54,21 +54,3 @@ #define NFS4_dec_cb_recall_any_sz (cb_compound_dec_hdr_sz + \ cb_sequence_dec_sz + \ op_dec_sz) - -/* - * 1: CB_GETATTR opcode (32-bit) - * N: file_handle - * 1: number of entry in attribute array (32-bit) - * 1: entry 0 in attribute array (32-bit) - */ -#define NFS4_enc_cb_getattr_sz (cb_compound_enc_hdr_sz + \ - cb_sequence_enc_sz + \ - 1 + enc_nfs4_fh_sz + 1 + 1) -/* - * 4: fattr_bitmap_maxsz - * 1: attribute array len - * 2: change attr (64-bit) - * 2: size (64-bit) - */ -#define NFS4_dec_cb_getattr_sz (cb_compound_dec_hdr_sz + \ - cb_sequence_dec_sz + 4 + 1 + 2 + 2 + op_dec_sz) -- cgit From bd018b98ba84ca0c80abac1ef23ce726a809e58c Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Mon, 18 Dec 2023 17:05:40 -0500 Subject: SUNRPC: Revert 5f7fc5d69f6e92ec0b38774c387f5cf7812c5806 Guillaume says: > I believe commit 5f7fc5d69f6e ("SUNRPC: Resupply rq_pages from > node-local memory") in Linux 6.5+ is incorrect. It passes > unconditionally rq_pool->sp_id as the NUMA node. > > While the comment in the svc_pool declaration in sunrpc/svc.h says > that sp_id is also the NUMA node id, it might not be the case if > the svc is created using svc_create_pooled(). svc_created_pooled() > can use the per-cpu pool mode therefore in this case sp_id would > be the cpu id. Fix this by reverting now. At a later point this minor optimization, and the deceptive labeling of the sp_id field, can be revisited. Reported-by: Guillaume Morin Closes: https://lore.kernel.org/linux-nfs/ZYC9rsno8qYggVt9@bender.morinfr.org/T/#u Fixes: 5f7fc5d69f6e ("SUNRPC: Resupply rq_pages from node-local memory") Signed-off-by: Chuck Lever --- net/sunrpc/svc_xprt.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c index fee83d1024bc..1b71055fc391 100644 --- a/net/sunrpc/svc_xprt.c +++ b/net/sunrpc/svc_xprt.c @@ -654,9 +654,8 @@ static bool svc_alloc_arg(struct svc_rqst *rqstp) } for (filled = 0; filled < pages; filled = ret) { - ret = alloc_pages_bulk_array_node(GFP_KERNEL, - rqstp->rq_pool->sp_id, - pages, rqstp->rq_pages); + ret = alloc_pages_bulk_array(GFP_KERNEL, pages, + rqstp->rq_pages); if (ret > filled) /* Made progress, don't sleep yet */ continue; -- cgit