sunrpc: fixes and cleanups for svc creation and thread handling
Subsystems (4)
cover sunrpc: fixes and cleanups for svc creation and thread handling
Hi Bruce!
Patch #1 in this series is a bugfix, but probably isn't worth sending to stable. The rest are just cleanups in preparation for some other patches that I have queued up. Can you consider these for 3.19?
The main one that I'm hoping to send soon for v3.19 is the one to help reduce the pool->sp_lock contention on busy servers. I have some preliminary numbers that look pretty good, but one of our QA folks is working on getting some better ones from a more rigorous test. I'll send those along in a few days once I have those numbers.
Jeff Layton (10):
sunrpc: release svc_pool_map reference when serv allocation fails
sunrpc: add a generic rq_flags field to svc_rqst and move rq_secure to
it
sunrpc: move rq_local field to rq_flags
sunrpc: move rq_usedeferral flag to rq_flags
sunrpc: move rq_dropme flag into rq_flags
sunrpc: move rq_splice_ok flag into rq_flags
sunrpc: move rq_cachetype field to better optimize space
sunrpc: convert sp_task_pending flag to use atomic bitops
sunrpc: have svc_wake_up only deal with pool 0
sunrpc: require svc_create callers to pass in meaningful shutdown
routine
fs/lockd/svc.c | 2 +- fs/nfsd/nfs4proc.c | 6 ++--- fs/nfsd/nfs4xdr.c | 8 +++--- fs/nfsd/nfscache.c | 4 +-- fs/nfsd/nfsfh.c | 2 +- fs/nfsd/nfssvc.c | 2 +- fs/nfsd/vfs.c | 6 ++--- include/linux/sunrpc/svc.h | 22 +++++++++-------- include/trace/events/sunrpc.h | 24 +++++++++++++----- net/sunrpc/auth_gss/svcauth_gss.c | 2 +- net/sunrpc/svc.c | 23 +++++++++--------- net/sunrpc/svc_xprt.c | 51 ++++++++++++++++++--------------------- net/sunrpc/svcsock.c | 5 +++- 13 files changed, 85 insertions(+), 72 deletions(-)
Signature · 1 line
2.1.0
PATCH 1/10 sunrpc: release svc_pool_map reference when serv allocation fails
Currently, it leaks when the allocation fails.
--- net/sunrpc/svc.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
net/sunrpc/svc.c +7 -5
@@ -505,13 +505,15 @@ svc_create_pooled(struct svc_program *prog, unsigned int bufsize,unsigned int npools =svc_pool_map_get (); serv =__svc_create (prog, bufsize, npools, shutdown); +if (!serv) + goto out_err; -if (serv !=NULL ) { - serv->sv_function = func; - serv->sv_module = mod; - } - + serv->sv_function = func; + serv->sv_module = mod;return serv; +out_err: +svc_pool_map_put (); +return NULL ; }EXPORT_SYMBOL_GPL (svc_create_pooled);
Signature · 1 line
2.1.0
PATCH 2/10 sunrpc: add a generic rq_flags field to svc_rqst and move rq_secure to it
In reply to Jeff LaytonIn a later patch, we're going to need some atomic bit flags. Since that field will need to be an unsigned long, we mitigate that space consumption by migrating some other bitflags to the new field. Start with the rq_secure flag.
--- fs/nfsd/nfscache.c | 4 ++-- fs/nfsd/nfsfh.c | 2 +- include/linux/sunrpc/svc.h | 4 ++-- include/trace/events/sunrpc.h | 17 +++++++++++++---- net/sunrpc/svc_xprt.c | 5 ++++- 5 files changed, 22 insertions(+), 10 deletions(-)
fs/nfsd/nfscache.c +2 -2
@@ -490,7 +490,7 @@ found_entry:/* From the hall of fame of impractical attacks: * Is this a user who tries to snoop on the cache? */ rtn =RC_DOIT ; -if (!rqstp->rq_secure && rp->c_secure) +if (!test_bit (RQ_SECURE , &rqstp->rq_flags) && rp->c_secure) goto out;/* Compose RPC reply header */
@@ -579,7 +579,7 @@ nfsd_cache_update(struct svc_rqst *rqstp, int cachetype, __be32 *statp)spin_lock (&b->cache_lock); drc_mem_usage += bufsize;lru_put_end (b, rp); - rp->c_secure = rqstp->rq_secure; + rp->c_secure =test_bit (RQ_SECURE , &rqstp->rq_flags); rp->c_type = cachetype; rp->c_state =RC_DONE ;spin_unlock (&b->cache_lock);
fs/nfsd/nfsfh.c +1 -1
@@ -86,7 +86,7 @@ static __be32 nfsd_setuser_and_check_port(struct svc_rqst *rqstp,int flags =nfsexp_flags (rqstp, exp);/* Check if the request originated from a secure port. */ -if (!rqstp->rq_secure && !(flags &NFSEXP_INSECURE_PORT )) { +if (!test_bit (RQ_SECURE , &rqstp->rq_flags) && !(flags &NFSEXP_INSECURE_PORT )) { RPC_IFDEBUG(char buf[RPC_MAX_ADDRBUFLEN]);dprintk ("nfsd: request from insecure port %s!\n" ,svc_print_addr (rqstp, buf,sizeof (buf)));
include/linux/sunrpc/svc.h +2 -2
@@ -253,8 +253,8 @@ struct svc_rqst {u32 rq_vers;/* program version */ u32 rq_proc;/* procedure number */ u32 rq_prot;/* IP protocol */ -unsigned short - rq_secure : 1; /* secure port */ +#define RQ_SECURE (0)/* secure port */ +unsigned long rq_flags;/* flags field */ unsigned short rq_local : 1; /* local request */void * rq_argp;/* decoded arguments */
include/trace/events/sunrpc.h +13 -4
@@ -412,6 +412,10 @@ TRACE_EVENT(xs_tcp_data_recv, __entry->copied, __entry->reclen, __entry->offset) ); +#define show_rqstp_flags (flags) \ + __print_flags(flags, "|", \ + { (1UL << RQ_SECURE), "RQ_SECURE"}) + TRACE_EVENT(svc_recv, TP_PROTO(struct svc_rqst *rqst, int status),
@@ -421,16 +425,19 @@ TRACE_EVENT(svc_recv, __field(struct sockaddr *, addr) __field(__be32, xid) __field(int, status) + __field(unsigned long, flags) ),TP_fast_assign ( __entry->addr = (struct sockaddr *)&rqst->rq_addr; __entry->xid = status >0 ? rqst->rq_xid :0 ; __entry->status = status; + __entry->flags = rqst->rq_flags; ), -TP_printk ("addr=%pIScp xid=0x%x status=%d" , __entry->addr, -be32_to_cpu (__entry->xid), __entry->status) +TP_printk ("addr=%pIScp xid=0x%x status=%d flags=%s" , __entry->addr, +be32_to_cpu (__entry->xid), __entry->status, +show_rqstp_flags (__entry->flags)) ); DECLARE_EVENT_CLASS(svc_rqst_status,
@@ -444,6 +451,7 @@ DECLARE_EVENT_CLASS(svc_rqst_status, __field(__be32, xid) __field(int, dropme) __field(int, status) + __field(unsigned long, flags) ), TP_fast_assign(
@@ -451,11 +459,12 @@ DECLARE_EVENT_CLASS(svc_rqst_status, __entry->xid = rqst->rq_xid; __entry->dropme = (int )rqst->rq_dropme; __entry->status = status; + __entry->flags = rqst->rq_flags; ), -TP_printk ("addr=%pIScp rq_xid=0x%x dropme=%d status=%d" , +TP_printk ("addr=%pIScp rq_xid=0x%x dropme=%d status=%d flags=%s" , __entry->addr,be32_to_cpu (__entry->xid), __entry->dropme, - __entry->status) + __entry->status,show_rqstp_flags (__entry->flags)) ); DEFINE_EVENT(svc_rqst_status, svc_process,
net/sunrpc/svc_xprt.c +4 -1
@@ -797,7 +797,10 @@ int svc_recv(struct svc_rqst *rqstp, long timeout)clear_bit (XPT_OLD , &xprt->xpt_flags); - rqstp->rq_secure = xprt->xpt_ops->xpo_secure_port (rqstp); +if (xprt->xpt_ops->xpo_secure_port (rqstp)) +set_bit (RQ_SECURE , &rqstp->rq_flags); +else +clear_bit (RQ_SECURE , &rqstp->rq_flags); rqstp->rq_chandle.defer = svc_defer; rqstp->rq_xid =svc_getu32 (&rqstp->rq_arg.head[0 ]);
Signature · 1 line
2.1.0
PATCH 3/10 sunrpc: move rq_local field to rq_flags
In reply to Jeff Layton--- fs/nfsd/vfs.c | 4 ++-- include/linux/sunrpc/svc.h | 2 +- include/trace/events/sunrpc.h | 3 ++- net/sunrpc/svcsock.c | 5 ++++- 4 files changed, 9 insertions(+), 5 deletions(-)
fs/nfsd/vfs.c +2 -2
@@ -940,7 +940,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file,loff_t pos = offset;unsigned int pflags = current->flags; -if (rqstp->rq_local) +if (test_bit (RQ_LOCAL , &rqstp->rq_flags))/* * We want less throttling in balance_dirty_pages() * and shrink_inactive_list() so that nfs to
@@ -981,7 +981,7 @@ out_nfserr: err =0 ; else err =nfserrno (host_err); -if (rqstp->rq_local) +if (test_bit (RQ_LOCAL , &rqstp->rq_flags))tsk_restore_flags (current, pflags,PF_LESS_THROTTLE );return err; }
include/linux/sunrpc/svc.h +1 -1
@@ -254,8 +254,8 @@ struct svc_rqst {u32 rq_proc;/* procedure number */ u32 rq_prot;/* IP protocol */ #define RQ_SECURE (0)/* secure port */ +#define RQ_LOCAL (1)/* local request */ unsigned long rq_flags;/* flags field */ - unsigned short rq_local : 1; /* local request */void * rq_argp;/* decoded arguments */ void * rq_resp;/* xdr'd results */
include/trace/events/sunrpc.h +2 -1
@@ -414,7 +414,8 @@ TRACE_EVENT(xs_tcp_data_recv,#define show_rqstp_flags (flags) \ __print_flags(flags, "|", \ - { (1UL << RQ_SECURE), "RQ_SECURE"}) + { (1UL << RQ_SECURE), "RQ_SECURE"}, \ + { (1UL << RQ_LOCAL), "RQ_LOCAL"}) TRACE_EVENT(svc_recv, TP_PROTO(struct svc_rqst *rqst, int status),
net/sunrpc/svcsock.c +4 -1
@@ -1140,7 +1140,10 @@ static int svc_tcp_recvfrom(struct svc_rqst *rqstp) rqstp->rq_xprt_ctxt =NULL ; rqstp->rq_prot =IPPROTO_TCP ; - rqstp->rq_local = !!test_bit (XPT_LOCAL , &svsk->sk_xprt.xpt_flags); +if (test_bit (XPT_LOCAL , &svsk->sk_xprt.xpt_flags)) +set_bit (RQ_LOCAL , &rqstp->rq_flags); +else +clear_bit (RQ_LOCAL , &rqstp->rq_flags); p = (__be32 *)rqstp->rq_arg.head[0 ].iov_base; calldir = p[1 ];
Signature · 1 line
2.1.0
PATCH 4/10 sunrpc: move rq_usedeferral flag to rq_flags
In reply to Jeff Layton--- fs/nfsd/nfs4proc.c | 4 ++-- include/linux/sunrpc/svc.h | 2 +- include/trace/events/sunrpc.h | 9 +++++---- net/sunrpc/svc.c | 2 +- net/sunrpc/svc_xprt.c | 2 +- 5 files changed, 10 insertions(+), 9 deletions(-)
fs/nfsd/nfs4proc.c +2 -2
@@ -1331,7 +1331,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,* Don't use the deferral mechanism for NFSv4; compounds make it * too hard to avoid non-idempotency problems. */ - rqstp->rq_usedeferral = false; +clear_bit (RQ_USEDEFERRAL , &rqstp->rq_flags);/* * According to RFC3010, this takes precedence over all other errors.
@@ -1447,7 +1447,7 @@ encode_op:BUG_ON (cstate->replay_owner); out:/* Reset deferral mechanism for RPC deferrals */ - rqstp->rq_usedeferral = true; +set_bit (RQ_USEDEFERRAL , &rqstp->rq_flags);dprintk ("nfsv4 compound returned %d\n" ,ntohl (status));return status; }
include/linux/sunrpc/svc.h +1 -1
@@ -236,7 +236,6 @@ struct svc_rqst {struct svc_cred rq_cred;/* auth info */ void * rq_xprt_ctxt;/* transport specific context ptr */ struct svc_deferred_req *rq_deferred;/* deferred request we are replaying */ -bool rq_usedeferral;/* use deferral */ size_t rq_xprt_hlen;/* xprt header len */ struct xdr_buf rq_arg;
@@ -255,6 +254,7 @@ struct svc_rqst {u32 rq_prot;/* IP protocol */ #define RQ_SECURE (0)/* secure port */ #define RQ_LOCAL (1)/* local request */ +#define RQ_USEDEFERRAL (2)/* use deferral */ unsigned long rq_flags;/* flags field */ void * rq_argp;/* decoded arguments */
include/trace/events/sunrpc.h +5 -4
@@ -412,10 +412,11 @@ TRACE_EVENT(xs_tcp_data_recv, __entry->copied, __entry->reclen, __entry->offset) ); -#define show_rqstp_flags (flags) \ - __print_flags(flags, "|", \ - { (1UL << RQ_SECURE), "RQ_SECURE"}, \ - { (1UL << RQ_LOCAL), "RQ_LOCAL"}) +#define show_rqstp_flags (flags) \ + __print_flags(flags, "|", \ + { (1UL << RQ_SECURE), "RQ_SECURE"}, \ + { (1UL << RQ_LOCAL), "RQ_LOCAL"}, \ + { (1UL << RQ_USEDEFERRAL), "RQ_USEDEFERRAL"}) TRACE_EVENT(svc_recv, TP_PROTO(struct svc_rqst *rqst, int status),
net/sunrpc/svc.c +1 -1
@@ -1090,7 +1090,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)/* Will be turned off only in gss privacy case: */ rqstp->rq_splice_ok = true;/* Will be turned off only when NFSv4 Sessions are used */ - rqstp->rq_usedeferral = true; +set_bit (RQ_USEDEFERRAL , &rqstp->rq_flags); rqstp->rq_dropme = false;/* Setup reply header */
net/sunrpc/svc_xprt.c +1 -1
@@ -1081,7 +1081,7 @@ static struct cache_deferred_req *svc_defer(struct cache_req *req) struct svc_rqst *rqstp = container_of(req, struct svc_rqst, rq_chandle);struct svc_deferred_req *dr; -if (rqstp->rq_arg.page_len || !rqstp->rq_usedeferral) +if (rqstp->rq_arg.page_len || !test_bit (RQ_USEDEFERRAL , &rqstp->rq_flags))return NULL ;/* if more than a page, give up FIXME */ if (rqstp->rq_deferred) { dr = rqstp->rq_deferred;
Signature · 1 line
2.1.0
PATCH 5/10 sunrpc: move rq_dropme flag into rq_flags
In reply to Jeff Layton--- fs/nfsd/nfssvc.c | 2 +- include/linux/sunrpc/svc.h | 2 +- include/trace/events/sunrpc.h | 8 ++++---- net/sunrpc/svc.c | 4 ++-- net/sunrpc/svc_xprt.c | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-)
fs/nfsd/nfssvc.c +1 -1
@@ -692,7 +692,7 @@ nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)/* Now call the procedure handler, and encode NFS status. */ nfserr = proc->pc_func (rqstp, rqstp->rq_argp, rqstp->rq_resp); nfserr =map_new_errors (rqstp->rq_vers, nfserr); -if (nfserr == nfserr_dropit || rqstp->rq_dropme) { +if (nfserr == nfserr_dropit ||test_bit (RQ_DROPME , &rqstp->rq_flags)) {dprintk ("nfsd: Dropping request; may be revisited later\n" );nfsd_cache_update (rqstp,RC_NOCACHE ,NULL );return 0 ;
include/linux/sunrpc/svc.h +1 -1
@@ -255,6 +255,7 @@ struct svc_rqst {#define RQ_SECURE (0)/* secure port */ #define RQ_LOCAL (1)/* local request */ #define RQ_USEDEFERRAL (2)/* use deferral */ +#define RQ_DROPME (3)/* drop current reply */ unsigned long rq_flags;/* flags field */ void * rq_argp;/* decoded arguments */
@@ -271,7 +272,6 @@ struct svc_rqst {struct cache_req rq_chandle;/* handle passed to caches for * request delaying */ -bool rq_dropme;/* Catering to nfsd */ struct auth_domain * rq_client;/* RPC peer info */ struct auth_domain * rq_gssclient;/* "gss/"-style peer info */
include/trace/events/sunrpc.h +4 -4
@@ -416,7 +416,8 @@ TRACE_EVENT(xs_tcp_data_recv,__print_flags (flags,"|" , \ { (1UL << RQ_SECURE), "RQ_SECURE"}, \ { (1UL << RQ_LOCAL), "RQ_LOCAL"}, \ - { (1UL << RQ_USEDEFERRAL), "RQ_USEDEFERRAL"}) + { (1UL << RQ_USEDEFERRAL), "RQ_USEDEFERRAL"}, \ + { (1UL << RQ_DROPME), "RQ_DROPME"}) TRACE_EVENT(svc_recv, TP_PROTO(struct svc_rqst *rqst, int status),
@@ -459,13 +460,12 @@ DECLARE_EVENT_CLASS(svc_rqst_status,TP_fast_assign ( __entry->addr = (struct sockaddr *)&rqst->rq_addr; __entry->xid = rqst->rq_xid; - __entry->dropme = (int )rqst->rq_dropme; __entry->status = status; __entry->flags = rqst->rq_flags; ), -TP_printk ("addr=%pIScp rq_xid=0x%x dropme=%d status=%d flags=%s" , - __entry->addr,be32_to_cpu (__entry->xid), __entry->dropme, +TP_printk ("addr=%pIScp rq_xid=0x%x status=%d flags=%s" , + __entry->addr,be32_to_cpu (__entry->xid), __entry->status,show_rqstp_flags (__entry->flags)) );
net/sunrpc/svc.c +2 -2
@@ -1091,7 +1091,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv) rqstp->rq_splice_ok = true;/* Will be turned off only when NFSv4 Sessions are used */ set_bit (RQ_USEDEFERRAL , &rqstp->rq_flags); - rqstp->rq_dropme = false; +clear_bit (RQ_DROPME , &rqstp->rq_flags);/* Setup reply header */ rqstp->rq_xprt->xpt_ops->xpo_prep_reply_hdr (rqstp);
@@ -1191,7 +1191,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv) *statp = procp->pc_func (rqstp, rqstp->rq_argp, rqstp->rq_resp);/* Encode reply */ -if (rqstp->rq_dropme) { +if (test_bit (RQ_DROPME , &rqstp->rq_flags)) {if (procp->pc_release) procp->pc_release (rqstp,NULL , rqstp->rq_resp); goto dropit;
net/sunrpc/svc_xprt.c +1 -1
@@ -1110,7 +1110,7 @@ static struct cache_deferred_req *svc_defer(struct cache_req *req) }svc_xprt_get (rqstp->rq_xprt); dr->xprt = rqstp->rq_xprt; - rqstp->rq_dropme = true; +set_bit (RQ_DROPME , &rqstp->rq_flags); dr->handle.revisit = svc_revisit;return &dr->handle;
Signature · 1 line
2.1.0
PATCH 6/10 sunrpc: move rq_splice_ok flag into rq_flags
In reply to Jeff Layton--- fs/nfsd/nfs4proc.c | 2 +- fs/nfsd/nfs4xdr.c | 8 ++++---- fs/nfsd/vfs.c | 2 +- include/linux/sunrpc/svc.h | 6 +++--- include/trace/events/sunrpc.h | 3 ++- net/sunrpc/auth_gss/svcauth_gss.c | 2 +- net/sunrpc/svc.c | 2 +- 7 files changed, 13 insertions(+), 12 deletions(-)
fs/nfsd/nfs4proc.c +1 -1
@@ -772,7 +772,7 @@ nfsd4_read(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,* the client wants us to do more in this compound: */ if (!nfsd4_last_compound_op (rqstp)) - rqstp->rq_splice_ok = false; +clear_bit (RQ_SPLICE_OK , &rqstp->rq_flags);/* check stateid */ if ((status = nfs4_preprocess_stateid_op(SVC_NET(rqstp),
fs/nfsd/nfs4xdr.c +4 -4
@@ -1714,7 +1714,7 @@ nfsd4_decode_compound(struct nfsd4_compoundargs *argp) argp->rqstp->rq_cachetype = cachethis ?RC_REPLBUFF :RC_NOCACHE ;if (readcount >1 || max_reply >PAGE_SIZE - auth_slack) - argp->rqstp->rq_splice_ok = false; +clear_bit (RQ_SPLICE_OK , &argp->rqstp->rq_flags);DECODE_TAIL ; }
@@ -3236,10 +3236,10 @@ nfsd4_encode_read(struct nfsd4_compoundres *resp, __be32 nfserr, p =xdr_reserve_space (xdr,8 );/* eof flag and byte count */ if (!p) { -WARN_ON_ONCE (resp->rqstp->rq_splice_ok); +WARN_ON_ONCE (test_bit (RQ_SPLICE_OK , &resp->rqstp->rq_flags));return nfserr_resource; } -if (resp->xdr.buf->page_len && resp->rqstp->rq_splice_ok) { +if (resp->xdr.buf->page_len &&test_bit (RQ_SPLICE_OK , &resp->rqstp->rq_flags)) {WARN_ON_ONCE (1 );return nfserr_resource; }
@@ -3256,7 +3256,7 @@ nfsd4_encode_read(struct nfsd4_compoundres *resp, __be32 nfserr, goto err_truncate; } -if (file->f_op->splice_read && resp->rqstp->rq_splice_ok) +if (file->f_op->splice_read &&test_bit (RQ_SPLICE_OK , &resp->rqstp->rq_flags)) err =nfsd4_encode_splice_read (resp, read, file, maxcount);else err =nfsd4_encode_readv (resp, read, file, maxcount);
fs/nfsd/vfs.c +1 -1
@@ -881,7 +881,7 @@ static __be32 nfsd_vfs_read(struct svc_rqst *rqstp, struct file *file, loff_t offset, struct kvec *vec, int vlen, unsigned long *count) { - if (file->f_op->splice_read && rqstp->rq_splice_ok) + if (file->f_op->splice_read && test_bit(RQ_SPLICE_OK, &rqstp->rq_flags)) return nfsd_splice_read(rqstp, file, offset, count); else return nfsd_readv(file, offset, vec, vlen, count);
include/linux/sunrpc/svc.h +3 -3
@@ -256,6 +256,9 @@ struct svc_rqst {#define RQ_LOCAL (1)/* local request */ #define RQ_USEDEFERRAL (2)/* use deferral */ #define RQ_DROPME (3)/* drop current reply */ +#define RQ_SPLICE_OK (4)/* turned off in gss privacy +* to prevent encrypting page +* cache pages */ unsigned long rq_flags;/* flags field */ void * rq_argp;/* decoded arguments */
@@ -277,9 +280,6 @@ struct svc_rqst {struct auth_domain * rq_gssclient;/* "gss/"-style peer info */ int rq_cachetype;struct svc_cacherep * rq_cacherep;/* cache info */ -bool rq_splice_ok;/* turned off in gss privacy -* to prevent encrypting page -* cache pages */ struct task_struct *rq_task;/* service thread */ };
include/trace/events/sunrpc.h +2 -1
@@ -417,7 +417,8 @@ TRACE_EVENT(xs_tcp_data_recv, { (1UL << RQ_SECURE), "RQ_SECURE"}, \ { (1UL << RQ_LOCAL), "RQ_LOCAL"}, \ { (1UL << RQ_USEDEFERRAL), "RQ_USEDEFERRAL"}, \ - { (1UL << RQ_DROPME), "RQ_DROPME"}) + { (1UL << RQ_DROPME), "RQ_DROPME"}, \ + { (1UL << RQ_SPLICE_OK), "RQ_SPLICE_OK"}) TRACE_EVENT(svc_recv, TP_PROTO(struct svc_rqst *rqst, int status),
net/sunrpc/auth_gss/svcauth_gss.c +1 -1
@@ -886,7 +886,7 @@ unwrap_priv_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct gsu32 priv_len, maj_stat;int pad, saved_len, remaining_len, offset; - rqstp->rq_splice_ok = false; +clear_bit (RQ_SPLICE_OK , &rqstp->rq_flags); priv_len =svc_getnl (&buf->head[0 ]);if (rqstp->rq_deferred) {
net/sunrpc/svc.c +1 -1
@@ -1088,7 +1088,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv) goto err_short_len;/* Will be turned off only in gss privacy case: */ - rqstp->rq_splice_ok = true; +set_bit (RQ_SPLICE_OK , &rqstp->rq_flags);/* Will be turned off only when NFSv4 Sessions are used */ set_bit (RQ_USEDEFERRAL , &rqstp->rq_flags);clear_bit (RQ_DROPME , &rqstp->rq_flags);
Signature · 1 line
2.1.0
PATCH 7/10 sunrpc: move rq_cachetype field to better optimize space
In reply to Jeff LaytonThere are a couple of holes in the svc_rqst field on x86_64. Move the rq_cachetype to a different location to eliminate both of them.
--- include/linux/sunrpc/svc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/sunrpc/svc.h +1 -1
@@ -252,6 +252,7 @@ struct svc_rqst {u32 rq_vers;/* program version */ u32 rq_proc;/* procedure number */ u32 rq_prot;/* IP protocol */ +int rq_cachetype;/* catering to nfsd */ #define RQ_SECURE (0)/* secure port */ #define RQ_LOCAL (1)/* local request */ #define RQ_USEDEFERRAL (2)/* use deferral */
@@ -278,7 +279,6 @@ struct svc_rqst {/* Catering to nfsd */ struct auth_domain * rq_client;/* RPC peer info */ struct auth_domain * rq_gssclient;/* "gss/"-style peer info */ -int rq_cachetype;struct svc_cacherep * rq_cacherep;/* cache info */ struct task_struct *rq_task;/* service thread */ };
Signature · 1 line
2.1.0
PATCH 8/10 sunrpc: convert sp_task_pending flag to use atomic bitops
In reply to Jeff LaytonIn a later patch, we'll want to be able to handle this flag without holding the sp_lock. Change this field to an unsigned long flags field, and declare a new flag in it that can be managed with atomic bitops.
--- include/linux/sunrpc/svc.h | 4 +++- net/sunrpc/svc_xprt.c | 7 +++---- 2 files changed, 6 insertions(+), 5 deletions(-)
include/linux/sunrpc/svc.h +3 -1
@@ -50,7 +50,9 @@ struct svc_pool {unsigned int sp_nrthreads;/* # of threads in pool */ struct list_head sp_all_threads;/* all server threads */ struct svc_pool_stats sp_stats;/* statistics on pool operation */ -int sp_task_pending;/* has pending task */ +#define SP_TASK_PENDING (0)/* still work to do even if no +* xprt is queued. */ +unsigned long sp_flags; }____cacheline_aligned_in_smp ;/*
net/sunrpc/svc_xprt.c +3 -4
@@ -509,7 +509,7 @@ void svc_wake_up(struct svc_serv *serv)*/ wake_up_process (rqstp->rq_task); }else - pool->sp_task_pending =1 ; +set_bit (SP_TASK_PENDING , &pool->sp_flags);spin_unlock_bh (&pool->sp_lock); } }
@@ -644,10 +644,9 @@ static struct svc_xprt *svc_get_next_xprt(struct svc_rqst *rqstp, long timeout)* long for cache updates. */ rqstp->rq_chandle.thread_wait =1 *HZ ; - pool->sp_task_pending =0 ; +clear_bit (SP_TASK_PENDING , &pool->sp_flags); }else { -if (pool->sp_task_pending) { - pool->sp_task_pending =0 ; +if (test_and_clear_bit (SP_TASK_PENDING , &pool->sp_flags)) { xprt =ERR_PTR (-EAGAIN ); goto out; }
Signature · 1 line
2.1.0
PATCH 9/10 sunrpc: have svc_wake_up only deal with pool 0
In reply to Jeff LaytonThe way that svc_wake_up works is a bit inefficient. It walks all of the available pools for a service and either wakes up a task in each one or sets the SP_TASK_PENDING flag in each one.
When svc_wake_up is called, there is no need to wake up more than one thread to do this work. In practice, only lockd currently uses this function and it's single threaded anyway. Thus, this just boils down to doing a wake up of a thread in pool 0 or setting a single flag.
Eliminate the for loop in this function and change it to just operate on pool 0. Also update the comments that sit above it and get rid of some code that has been commented out for years now.
--- net/sunrpc/svc_xprt.c | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-)
net/sunrpc/svc_xprt.c +16 -21
@@ -484,34 +484,29 @@ static void svc_xprt_release(struct svc_rqst *rqstp) }/* -* External function to wake up a server waiting for data -* This really only makes sense for services like lockd -* which have exactly one thread anyway. +* Some svc_serv's will have occasional work to do, even when a xprt is not +* waiting to be serviced. This function is there to "kick" a task in one of +* those services so that it can wake up and do that work. Note that we only +* bother with pool 0 as we don't need to wake up more than one thread for +* this purpose. */ void svc_wake_up (struct svc_serv *serv) {struct svc_rqst *rqstp; -unsigned int i;struct svc_pool *pool; -for (i =0 ; i < serv->sv_nrpools; i++) { - pool = &serv->sv_pools[i]; + pool = &serv->sv_pools[0 ]; -spin_lock_bh (&pool->sp_lock); -if (!list_empty (&pool->sp_threads)) { - rqstp =list_entry (pool->sp_threads.next, - struct svc_rqst, - rq_list); -dprintk ("svc: daemon %p woken up.\n" , rqstp); -/* -svc_thread_dequeue(pool, rqstp); -rqstp->rq_xprt = NULL; -*/ -wake_up_process (rqstp->rq_task); - }else -set_bit (SP_TASK_PENDING , &pool->sp_flags); -spin_unlock_bh (&pool->sp_lock); - } +spin_lock_bh (&pool->sp_lock); +if (!list_empty (&pool->sp_threads)) { + rqstp =list_entry (pool->sp_threads.next, + struct svc_rqst, + rq_list); +dprintk ("svc: daemon %p woken up.\n" , rqstp); +wake_up_process (rqstp->rq_task); + }else +set_bit (SP_TASK_PENDING , &pool->sp_flags); +spin_unlock_bh (&pool->sp_lock); }EXPORT_SYMBOL_GPL (svc_wake_up);
Signature · 1 line
2.1.0
PATCH 10/10 sunrpc: require svc_create callers to pass in meaningful shutdown routine
In reply to Jeff LaytonCurrently all svc_create callers pass in NULL for the shutdown parm, which then gets fixed up to be svc_rpcb_cleanup if the service uses rpcbind.
Simplify this by instead having the the only caller that requires it (lockd) pass in svc_rpcb_cleanup and get rid of the special casing.
--- fs/lockd/svc.c | 2 +- net/sunrpc/svc.c | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-)
fs/lockd/svc.c +1 -1
@@ -350,7 +350,7 @@ static struct svc_serv *lockd_create_svc(void)printk (KERN_WARNING "lockd_up: no pid, %d users??\n" , nlmsvc_users); - serv =svc_create (&nlmsvc_program,LOCKD_BUFSIZE ,NULL ); + serv =svc_create (&nlmsvc_program,LOCKD_BUFSIZE , svc_rpcb_cleanup);if (!serv) {printk (KERN_WARNING "lockd_up: create service failed\n" );return ERR_PTR (-ENOMEM );
net/sunrpc/svc.c +0 -3
@@ -482,9 +482,6 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,spin_lock_init (&pool->sp_lock); } -if (svc_uses_rpcbind (serv) && (!serv->sv_shutdown)) - serv->sv_shutdown = svc_rpcb_cleanup; -return serv; }
Signature · 1 line
2.1.0
Quoted 9 lines · Jeff Layton
Patch #1 in this series is a bugfix, but probably isn't worth sending to stable. The rest are just cleanups in preparation for some other patches that I have queued up. Can you consider these for 3.19?
The main one that I'm hoping to send soon for v3.19 is the one to help reduce the
pool->sp_lockcontention on busy servers. I have some preliminary numbers that look pretty good, but one of our QA folks is working on getting some better ones from a more rigorous test. I'll send those along in a few days once I have those numbers.
Thanks, I'll take a look! If not today then probably next week.
With the holiday season approaching I have some time off and travel coming up. I should still be mostly online, but anyone that notices me ignoring an urgent patch should definitely complain early and often.
--b.
Quoted 33 lines
Jeff Layton (10):
sunrpc: releasesvc_pool_mapreference when serv allocation fails
sunrpc: add a genericrq_flagsfield tosvc_rqstand moverq_secureto
it
sunrpc: moverq_localfield torq_flags
sunrpc: moverq_usedeferralflag torq_flags
sunrpc: moverq_dropmeflag intorq_flags
sunrpc: moverq_splice_okflag intorq_flags
sunrpc: moverq_cachetypefield to better optimize space
sunrpc: convertsp_task_pendingflag to use atomic bitops
sunrpc: havesvc_wake_uponly deal with pool 0
sunrpc: requiresvc_createcallers to pass in meaningful shutdown
routinefs/lockd/svc.c | 2 +- fs/nfsd/nfs4proc.c | 6 ++--- fs/nfsd/nfs4xdr.c | 8 +++--- fs/nfsd/nfscache.c | 4 +-- fs/nfsd/nfsfh.c | 2 +- fs/nfsd/nfssvc.c | 2 +- fs/nfsd/vfs.c | 6 ++--- include/linux/sunrpc/svc.h | 22 +++++++++-------- include/trace/events/sunrpc.h | 24 +++++++++++++----- net/sunrpc/auth_gss/svcauth_gss.c | 2 +- net/sunrpc/svc.c | 23 +++++++++--------- net/sunrpc/svc_xprt.c | 51 ++++++++++++++++++--------------------- net/sunrpc/svcsock.c | 5 +++- 13 files changed, 85 insertions(+), 72 deletions(-)Signature · 1 line
2.1.0
Quoted 3 lines · Jeff Layton
Patch #1 in this series is a bugfix, but probably isn't worth sending to stable. The rest are just cleanups in preparation for some other patches that I have queued up. Can you consider these for 3.19?
Sure, I've read through them, they look fine.
When I tried to apply them to my for-3.19 there were rejects due to tracing stuff in include/trace/events/sunrpc.h. I haven't tried to investigate yet.
--b.
Quoted 39 lines
The main one that I'm hoping to send soon for v3.19 is the one to help reduce the
pool->sp_lockcontention on busy servers. I have some preliminary numbers that look pretty good, but one of our QA folks is working on getting some better ones from a more rigorous test. I'll send those along in a few days once I have those numbers.Jeff Layton (10):
sunrpc: releasesvc_pool_mapreference when serv allocation fails
sunrpc: add a genericrq_flagsfield tosvc_rqstand moverq_secureto
it
sunrpc: moverq_localfield torq_flags
sunrpc: moverq_usedeferralflag torq_flags
sunrpc: moverq_dropmeflag intorq_flags
sunrpc: moverq_splice_okflag intorq_flags
sunrpc: moverq_cachetypefield to better optimize space
sunrpc: convertsp_task_pendingflag to use atomic bitops
sunrpc: havesvc_wake_uponly deal with pool 0
sunrpc: requiresvc_createcallers to pass in meaningful shutdown
routinefs/lockd/svc.c | 2 +- fs/nfsd/nfs4proc.c | 6 ++--- fs/nfsd/nfs4xdr.c | 8 +++--- fs/nfsd/nfscache.c | 4 +-- fs/nfsd/nfsfh.c | 2 +- fs/nfsd/nfssvc.c | 2 +- fs/nfsd/vfs.c | 6 ++--- include/linux/sunrpc/svc.h | 22 +++++++++-------- include/trace/events/sunrpc.h | 24 +++++++++++++----- net/sunrpc/auth_gss/svcauth_gss.c | 2 +- net/sunrpc/svc.c | 23 +++++++++--------- net/sunrpc/svc_xprt.c | 51 ++++++++++++++++++--------------------- net/sunrpc/svcsock.c | 5 +++- 13 files changed, 85 insertions(+), 72 deletions(-)Signature · 1 line
2.1.0
Quoted 5 lines · On Wed, 19 Nov 2014 16:37:58 -0500 "J. Bruce Fields" <[email protected]>…
Quoted 3 lines · Jeff Layton
Patch #1 in this series is a bugfix, but probably isn't worth sending to stable. The rest are just cleanups in preparation for some other patches that I have queued up. Can you consider these for 3.19?
Quoted 8 lines
Sure, I've read through them, they look fine.
When I tried to apply them to my for-3.19 there were rejects due to tracing stuff in
include/trace/events/sunrpc.h. I haven't tried to investigate yet.--b.
Ahh yeah. These are based on top of the tracepoint patches I sent to Trond a couple of weeks ago. I think he's planning to merge those in 3.19 too, but I don't think he's done that yet. Anna may have though...
Quoted 39 lines
Quoted 39 lines
The main one that I'm hoping to send soon for v3.19 is the one to help reduce the
pool->sp_lockcontention on busy servers. I have some preliminary numbers that look pretty good, but one of our QA folks is working on getting some better ones from a more rigorous test. I'll send those along in a few days once I have those numbers.Jeff Layton (10):
sunrpc: releasesvc_pool_mapreference when serv allocation fails
sunrpc: add a genericrq_flagsfield tosvc_rqstand moverq_secureto
it
sunrpc: moverq_localfield torq_flags
sunrpc: moverq_usedeferralflag torq_flags
sunrpc: moverq_dropmeflag intorq_flags
sunrpc: moverq_splice_okflag intorq_flags
sunrpc: moverq_cachetypefield to better optimize space
sunrpc: convertsp_task_pendingflag to use atomic bitops
sunrpc: havesvc_wake_uponly deal with pool 0
sunrpc: requiresvc_createcallers to pass in meaningful shutdown
routinefs/lockd/svc.c | 2 +- fs/nfsd/nfs4proc.c | 6 ++--- fs/nfsd/nfs4xdr.c | 8 +++--- fs/nfsd/nfscache.c | 4 +-- fs/nfsd/nfsfh.c | 2 +- fs/nfsd/nfssvc.c | 2 +- fs/nfsd/vfs.c | 6 ++--- include/linux/sunrpc/svc.h | 22 +++++++++-------- include/trace/events/sunrpc.h | 24 +++++++++++++----- net/sunrpc/auth_gss/svcauth_gss.c | 2 +- net/sunrpc/svc.c | 23 +++++++++--------- net/sunrpc/svc_xprt.c | 51 ++++++++++++++++++--------------------- net/sunrpc/svcsock.c | 5 +++- 13 files changed, 85 insertions(+), 72 deletions(-)Signature · 1 line
2.1.0
Signature · 1 line
Jeff Layton <[email protected]>
Quoted 13 lines · Jeff Layton
Quoted 10 lines · On Wed, 19 Nov 2014 16:37:58 -0500 "J. Bruce Fields" <[email protected]>…
Quoted 3 lines · Jeff Layton
Patch #1 in this series is a bugfix, but probably isn't worth sending to stable. The rest are just cleanups in preparation for some other patches that I have queued up. Can you consider these for 3.19?
Sure, I've read through them, they look fine.
When I tried to apply them to my for-3.19 there were rejects due to tracing stuff in
include/trace/events/sunrpc.h. I haven't tried to investigate yet.
Quoted 5 lines
Quoted 2 lines
--b.
Ahh yeah. These are based on top of the tracepoint patches I sent to Trond a couple of weeks ago. I think he's planning to merge those in 3.19 too, but I don't think he's done that yet. Anna may have though...
I have them in a private tree on my laptop that I use for testing. Want me to push them out somewhere?
Anna
Quoted 40 lines
Quoted 39 lines
Quoted 39 lines
The main one that I'm hoping to send soon for v3.19 is the one to help reduce the
pool->sp_lockcontention on busy servers. I have some preliminary numbers that look pretty good, but one of our QA folks is working on getting some better ones from a more rigorous test. I'll send those along in a few days once I have those numbers.Jeff Layton (10):
sunrpc: releasesvc_pool_mapreference when serv allocation fails
sunrpc: add a genericrq_flagsfield tosvc_rqstand moverq_secureto
it
sunrpc: moverq_localfield torq_flags
sunrpc: moverq_usedeferralflag torq_flags
sunrpc: moverq_dropmeflag intorq_flags
sunrpc: moverq_splice_okflag intorq_flags
sunrpc: moverq_cachetypefield to better optimize space
sunrpc: convertsp_task_pendingflag to use atomic bitops
sunrpc: havesvc_wake_uponly deal with pool 0
sunrpc: requiresvc_createcallers to pass in meaningful shutdown
routinefs/lockd/svc.c | 2 +- fs/nfsd/nfs4proc.c | 6 ++--- fs/nfsd/nfs4xdr.c | 8 +++--- fs/nfsd/nfscache.c | 4 +-- fs/nfsd/nfsfh.c | 2 +- fs/nfsd/nfssvc.c | 2 +- fs/nfsd/vfs.c | 6 ++--- include/linux/sunrpc/svc.h | 22 +++++++++-------- include/trace/events/sunrpc.h | 24 +++++++++++++----- net/sunrpc/auth_gss/svcauth_gss.c | 2 +- net/sunrpc/svc.c | 23 +++++++++--------- net/sunrpc/svc_xprt.c | 51 ++++++++++++++++++--------------------- net/sunrpc/svcsock.c | 5 +++- 13 files changed, 85 insertions(+), 72 deletions(-)Signature · 1 line
2.1.0
Quoted 14 lines · Anna Schumaker
Quoted 13 lines · Jeff Layton
Quoted 10 lines · On Wed, 19 Nov 2014 16:37:58 -0500 "J. Bruce Fields" <[email protected]>…
Quoted 3 lines · Jeff Layton
Patch #1 in this series is a bugfix, but probably isn't worth sending to stable. The rest are just cleanups in preparation for some other patches that I have queued up. Can you consider these for 3.19?
Sure, I've read through them, they look fine.
When I tried to apply them to my for-3.19 there were rejects due to tracing stuff in
include/trace/events/sunrpc.h. I haven't tried to investigate yet.
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
--b.
Ahh yeah. These are based on top of the tracepoint patches I sent to Trond a couple of weeks ago. I think he's planning to merge those in 3.19 too, but I don't think he's done that yet. Anna may have though...
I have them in a private tree on my laptop that I use for testing. Want me to push them out somewhere?
That'd help me at least test, but we need something more than a temporary testing branch to get ready for the merge window. Choices include:
- Trond merges them all now, I pull that and apply these on top.
- I merge them instead, or we divide the patches between us somehow. Worst case, it's probably even OK if there's a patch or two that we both apply.
In any case we need to get that sorted out now-ish as the merge window's probably starting next week.
--b.
Quoted 27 lines · J. Bruce Fields
Quoted 14 lines · Anna Schumaker
Quoted 13 lines · Jeff Layton
Quoted 10 lines · On Wed, 19 Nov 2014 16:37:58 -0500 "J. Bruce Fields" <[email protected]>…
Quoted 3 lines · Jeff Layton
Patch #1 in this series is a bugfix, but probably isn't worth sending to stable. The rest are just cleanups in preparation for some other patches that I have queued up. Can you consider these for 3.19?
Sure, I've read through them, they look fine.
When I tried to apply them to my for-3.19 there were rejects due to tracing stuff in
include/trace/events/sunrpc.h. I haven't tried to investigate yet.Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
--b.
Ahh yeah. These are based on top of the tracepoint patches I sent to Trond a couple of weeks ago. I think he's planning to merge those in 3.19 too, but I don't think he's done that yet. Anna may have though...
I have them in a private tree on my laptop that I use for testing. Want me to push them out somewhere?
That'd help me at least test, but we need something more than a temporary testing branch to get ready for the merge window. Choices include:
Quoted 8 lines
- Trond merges them all now, I pull that and apply these on top.
- I merge them instead, or we divide the patches between us somehow. Worst case, it's probably even OK if there's a patch or two that we both apply.In any case we need to get that sorted out now-ish as the merge window's probably starting next week.
All patches that I plan on merging for the next window were pushed out last Friday. Jeff's tracepoint and debugfs patchset should therefore be present in my nfs-for-next and linux-next branches on git.linux-nfs.org.
Signature · 5 lines
Trond Myklebust Linux NFS client maintainer, PrimaryData [email protected]
Quoted 33 lines · Trond Myklebust
Quoted 32 lines · J. Bruce Fields
Quoted 14 lines · Anna Schumaker
Quoted 13 lines · Jeff Layton
Quoted 10 lines · On Wed, 19 Nov 2014 16:37:58 -0500 "J. Bruce Fields" <[email protected]>…
Quoted 3 lines · Jeff Layton
Patch #1 in this series is a bugfix, but probably isn't worth sending to stable. The rest are just cleanups in preparation for some other patches that I have queued up. Can you consider these for 3.19?
Sure, I've read through them, they look fine.
When I tried to apply them to my for-3.19 there were rejects due to tracing stuff in
include/trace/events/sunrpc.h. I haven't tried to investigate yet.Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
--b.
Ahh yeah. These are based on top of the tracepoint patches I sent to Trond a couple of weeks ago. I think he's planning to merge those in 3.19 too, but I don't think he's done that yet. Anna may have though...
I have them in a private tree on my laptop that I use for testing. Want me to push them out somewhere?
That'd help me at least test, but we need something more than a temporary testing branch to get ready for the merge window. Choices include:
- Trond merges them all now, I pull that and apply these on top.
- I merge them instead, or we divide the patches between us somehow. Worst case, it's probably even OK if there's a patch or two that we both apply.
Quoted 8 lines
Quoted 3 lines
In any case we need to get that sorted out now-ish as the merge window's probably starting next week.
All patches that I plan on merging for the next window were pushed out last Friday. Jeff's tracepoint and debugfs patchset should therefore be present in my nfs-for-next and linux-next branches on git.linux-nfs.org.
OK, great, I'll go take a look at that and figure out what's needed, thanks.
--b.