| Message ID | 168966228863.11075.8173252015139876869.stgit@noble.brown (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Refactor SUNRPC svc thread code, and use llist | expand |
On Tue, Jul 18, 2023 at 04:38:08PM +1000, NeilBrown wrote: > When a sequence of numbers are needed for internal-use only, an enum is > typically best. The sequence will inevitably need to be changed one > day, and having an enum means the developer doesn't need to think about > renumbering after insertion or deletion. The patch will be easier to > review. Last sentence needs to define the antecedant of "The patch". > Signed-off-by: NeilBrown <neilb@suse.de> > --- > include/linux/sunrpc/cache.h | 11 +++++++---- > include/linux/sunrpc/svc.h | 34 ++++++++++++++++++++-------------- > include/linux/sunrpc/svc_xprt.h | 39 +++++++++++++++++++++------------------ > include/linux/sunrpc/svcauth.h | 29 ++++++++++++++++------------- > include/linux/sunrpc/xprtsock.h | 25 +++++++++++++------------ > 5 files changed, 77 insertions(+), 61 deletions(-) > > diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h > index 518bd28f5ab8..3cc4f4f0c764 100644 > --- a/include/linux/sunrpc/cache.h > +++ b/include/linux/sunrpc/cache.h > @@ -56,10 +56,13 @@ struct cache_head { > struct kref ref; > unsigned long flags; > }; > -#define CACHE_VALID 0 /* Entry contains valid data */ > -#define CACHE_NEGATIVE 1 /* Negative entry - there is no match for the key */ > -#define CACHE_PENDING 2 /* An upcall has been sent but no reply received yet*/ > -#define CACHE_CLEANED 3 /* Entry has been cleaned from cache */ > +/* cache_head.flags */ > +enum { > + CACHE_VALID, /* Entry contains valid data */ > + CACHE_NEGATIVE, /* Negative entry - there is no match for the key */ > + CACHE_PENDING, /* An upcall has been sent but no reply received yet*/ > + CACHE_CLEANED, /* Entry has been cleaned from cache */ > +}; Weird comment of the day: Please use a double-tab before the comments to leave room for larger flag names in the future. > #define CACHE_NEW_EXPIRY 120 /* keep new things pending confirmation for 120 seconds */ > > diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h > index f3df7f963653..83f31a09c853 100644 > --- a/include/linux/sunrpc/svc.h > +++ b/include/linux/sunrpc/svc.h > @@ -31,7 +31,7 @@ > * node traffic on multi-node NUMA NFS servers. > */ > struct svc_pool { > - unsigned int sp_id; /* pool id; also node id on NUMA */ > + unsigned int sp_id; /* pool id; also node id on NUMA */ > spinlock_t sp_lock; /* protects all fields */ > struct list_head sp_sockets; /* pending sockets */ > unsigned int sp_nrthreads; /* # of threads in pool */ > @@ -44,12 +44,15 @@ struct svc_pool { > struct percpu_counter sp_threads_starved; > struct percpu_counter sp_threads_no_work; > > -#define SP_TASK_PENDING (0) /* still work to do even if no > - * xprt is queued. */ > -#define SP_CONGESTED (1) > unsigned long sp_flags; > } ____cacheline_aligned_in_smp; > > +/* bits for sp_flags */ > +enum { > + SP_TASK_PENDING, /* still work to do even if no xprt is queued */ > + SP_CONGESTED, /* all threads are busy, none idle */ > +}; > + > /* > * RPC service. > * > @@ -232,16 +235,6 @@ struct svc_rqst { > 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 */ > -#define RQ_DROPME (3) /* drop current reply */ > -#define RQ_SPLICE_OK (4) /* turned off in gss privacy > - * to prevent encrypting page > - * cache pages */ > -#define RQ_VICTIM (5) /* about to be shut down */ > -#define RQ_BUSY (6) /* request is busy */ > -#define RQ_DATA (7) /* request has data */ > unsigned long rq_flags; /* flags field */ > ktime_t rq_qtime; /* enqueue time */ > > @@ -272,6 +265,19 @@ struct svc_rqst { > void ** rq_lease_breaker; /* The v4 client breaking a lease */ > }; > > +/* bits for rq_flags */ > +enum { > + RQ_SECURE, /* secure port */ > + RQ_LOCAL, /* local request */ > + RQ_USEDEFERRAL, /* use deferral */ > + RQ_DROPME, /* drop current reply */ > + RQ_SPLICE_OK, /* turned off in gss privacy to prevent encrypting page > + * cache pages */ > + RQ_VICTIM, /* about to be shut down */ > + RQ_BUSY, /* request is busy */ > + RQ_DATA, /* request has data */ > +}; > + Also here -- two tab stops instead of one. > #define SVC_NET(rqst) (rqst->rq_xprt ? rqst->rq_xprt->xpt_net : rqst->rq_bc_net) > > /* > diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h > index a6b12631db21..af383d0349b3 100644 > --- a/include/linux/sunrpc/svc_xprt.h > +++ b/include/linux/sunrpc/svc_xprt.h > @@ -56,26 +56,9 @@ struct svc_xprt { > struct list_head xpt_list; > struct list_head xpt_ready; > unsigned long xpt_flags; > -#define XPT_BUSY 0 /* enqueued/receiving */ > -#define XPT_CONN 1 /* conn pending */ > -#define XPT_CLOSE 2 /* dead or dying */ > -#define XPT_DATA 3 /* data pending */ > -#define XPT_TEMP 4 /* connected transport */ > -#define XPT_DEAD 6 /* transport closed */ > -#define XPT_CHNGBUF 7 /* need to change snd/rcv buf sizes */ > -#define XPT_DEFERRED 8 /* deferred request pending */ > -#define XPT_OLD 9 /* used for xprt aging mark+sweep */ > -#define XPT_LISTENER 10 /* listening endpoint */ > -#define XPT_CACHE_AUTH 11 /* cache auth info */ > -#define XPT_LOCAL 12 /* connection from loopback interface */ > -#define XPT_KILL_TEMP 13 /* call xpo_kill_temp_xprt before closing */ > -#define XPT_CONG_CTRL 14 /* has congestion control */ > -#define XPT_HANDSHAKE 15 /* xprt requests a handshake */ > -#define XPT_TLS_SESSION 16 /* transport-layer security established */ > -#define XPT_PEER_AUTH 17 /* peer has been authenticated */ > > struct svc_serv *xpt_server; /* service for transport */ > - atomic_t xpt_reserved; /* space on outq that is rsvd */ > + atomic_t xpt_reserved; /* space on outq that is rsvd */ > atomic_t xpt_nr_rqsts; /* Number of requests */ > struct mutex xpt_mutex; /* to serialize sending data */ > spinlock_t xpt_lock; /* protects sk_deferred > @@ -96,6 +79,26 @@ struct svc_xprt { > struct rpc_xprt *xpt_bc_xprt; /* NFSv4.1 backchannel */ > struct rpc_xprt_switch *xpt_bc_xps; /* NFSv4.1 backchannel */ > }; > +/* flag bits for xpt_flags */ > +enum { > + XPT_BUSY, /* enqueued/receiving */ > + XPT_CONN, /* conn pending */ > + XPT_CLOSE, /* dead or dying */ > + XPT_DATA, /* data pending */ > + XPT_TEMP, /* connected transport */ > + XPT_DEAD, /* transport closed */ > + XPT_CHNGBUF, /* need to change snd/rcv buf sizes */ > + XPT_DEFERRED, /* deferred request pending */ > + XPT_OLD, /* used for xprt aging mark+sweep */ > + XPT_LISTENER, /* listening endpoint */ > + XPT_CACHE_AUTH, /* cache auth info */ > + XPT_LOCAL, /* connection from loopback interface */ > + XPT_KILL_TEMP, /* call xpo_kill_temp_xprt before closing */ > + XPT_CONG_CTRL, /* has congestion control */ > + XPT_HANDSHAKE, /* xprt requests a handshake */ > + XPT_TLS_SESSION, /* transport-layer security established */ > + XPT_PEER_AUTH, /* peer has been authenticated */ > +}; > > static inline void unregister_xpt_user(struct svc_xprt *xpt, struct svc_xpt_user *u) > { > diff --git a/include/linux/sunrpc/svcauth.h b/include/linux/sunrpc/svcauth.h > index 6d9cc9080aca..8d1d0d0721d2 100644 > --- a/include/linux/sunrpc/svcauth.h > +++ b/include/linux/sunrpc/svcauth.h > @@ -133,19 +133,22 @@ struct auth_ops { > int (*set_client)(struct svc_rqst *rq); > }; > > -#define SVC_GARBAGE 1 > -#define SVC_SYSERR 2 > -#define SVC_VALID 3 > -#define SVC_NEGATIVE 4 > -#define SVC_OK 5 > -#define SVC_DROP 6 > -#define SVC_CLOSE 7 /* Like SVC_DROP, but request is definitely > - * lost so if there is a tcp connection, it > - * should be closed > - */ > -#define SVC_DENIED 8 > -#define SVC_PENDING 9 > -#define SVC_COMPLETE 10 > +/*return values for svc functions that analyse request */ > +enum { > + SVC_GARBAGE, > + SVC_SYSERR, > + SVC_VALID, > + SVC_NEGATIVE, > + SVC_OK, > + SVC_DROP, > + SVC_CLOSE, /* Like SVC_DROP, but request is definitely > + * lost so if there is a tcp connection, it > + * should be closed > + */ > + SVC_DENIED, > + SVC_PENDING, > + SVC_COMPLETE, > +}; > > struct svc_xprt; > > diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h > index 700a1e6c047c..1ed2f446010b 100644 > --- a/include/linux/sunrpc/xprtsock.h > +++ b/include/linux/sunrpc/xprtsock.h > @@ -81,17 +81,18 @@ struct sock_xprt { > }; > > /* > - * TCP RPC flags > + * TCP RPC flags in ->sock_state > */ > -#define XPRT_SOCK_CONNECTING 1U > -#define XPRT_SOCK_DATA_READY (2) > -#define XPRT_SOCK_UPD_TIMEOUT (3) > -#define XPRT_SOCK_WAKE_ERROR (4) > -#define XPRT_SOCK_WAKE_WRITE (5) > -#define XPRT_SOCK_WAKE_PENDING (6) > -#define XPRT_SOCK_WAKE_DISCONNECT (7) > -#define XPRT_SOCK_CONNECT_SENT (8) > -#define XPRT_SOCK_NOSPACE (9) > -#define XPRT_SOCK_IGNORE_RECV (10) > - > +enum { > + XPRT_SOCK_CONNECTING, > + XPRT_SOCK_DATA_READY, > + XPRT_SOCK_UPD_TIMEOUT, > + XPRT_SOCK_WAKE_ERROR, > + XPRT_SOCK_WAKE_WRITE, > + XPRT_SOCK_WAKE_PENDING, > + XPRT_SOCK_WAKE_DISCONNECT, > + XPRT_SOCK_CONNECT_SENT, > + XPRT_SOCK_NOSPACE, > + XPRT_SOCK_IGNORE_RECV, > +}; > #endif /* _LINUX_SUNRPC_XPRTSOCK_H */ > > Let's not change client-side code in this patch. Please split this hunk out and send it to Trond and Anna separately.
On Tue, Jul 18, 2023 at 09:37:58AM -0400, Chuck Lever wrote: > On Tue, Jul 18, 2023 at 04:38:08PM +1000, NeilBrown wrote: > > When a sequence of numbers are needed for internal-use only, an enum is > > typically best. The sequence will inevitably need to be changed one > > day, and having an enum means the developer doesn't need to think about > > renumbering after insertion or deletion. The patch will be easier to > > review. > > Last sentence needs to define the antecedant of "The patch". I've changed the last sentence in the description to "Such patches will be easier ..." I've applied 1/5 through 5/5, with a few cosmetic changes, to the SUNRPC threads topic branch. 6/6 needed more work: I've split this into one patch for each new enum. The XPT_ flags change needed TRACE_DEFINE_ENUM macros to make show_svc_xprt_flags() work properly. Added. The new enum for SVC_GARBAGE and friends... those aren't bit flags. So I've made that a named enum so that it can be used for type- checking function return values properly. I dropped the hunk that changes XPRT_SOCK_CONNECTING and friends. That should be submitted separately to Anna and Trond. All this will appear in the nfsd repo later today. > > Signed-off-by: NeilBrown <neilb@suse.de> > > --- > > include/linux/sunrpc/cache.h | 11 +++++++---- > > include/linux/sunrpc/svc.h | 34 ++++++++++++++++++++-------------- > > include/linux/sunrpc/svc_xprt.h | 39 +++++++++++++++++++++------------------ > > include/linux/sunrpc/svcauth.h | 29 ++++++++++++++++------------- > > include/linux/sunrpc/xprtsock.h | 25 +++++++++++++------------ > > 5 files changed, 77 insertions(+), 61 deletions(-) > > > > diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h > > index 518bd28f5ab8..3cc4f4f0c764 100644 > > --- a/include/linux/sunrpc/cache.h > > +++ b/include/linux/sunrpc/cache.h > > @@ -56,10 +56,13 @@ struct cache_head { > > struct kref ref; > > unsigned long flags; > > }; > > -#define CACHE_VALID 0 /* Entry contains valid data */ > > -#define CACHE_NEGATIVE 1 /* Negative entry - there is no match for the key */ > > -#define CACHE_PENDING 2 /* An upcall has been sent but no reply received yet*/ > > -#define CACHE_CLEANED 3 /* Entry has been cleaned from cache */ > > +/* cache_head.flags */ > > +enum { > > + CACHE_VALID, /* Entry contains valid data */ > > + CACHE_NEGATIVE, /* Negative entry - there is no match for the key */ > > + CACHE_PENDING, /* An upcall has been sent but no reply received yet*/ > > + CACHE_CLEANED, /* Entry has been cleaned from cache */ > > +}; > > Weird comment of the day: Please use a double-tab before the comments > to leave room for larger flag names in the future. > > > > #define CACHE_NEW_EXPIRY 120 /* keep new things pending confirmation for 120 seconds */ > > > > diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h > > index f3df7f963653..83f31a09c853 100644 > > --- a/include/linux/sunrpc/svc.h > > +++ b/include/linux/sunrpc/svc.h > > @@ -31,7 +31,7 @@ > > * node traffic on multi-node NUMA NFS servers. > > */ > > struct svc_pool { > > - unsigned int sp_id; /* pool id; also node id on NUMA */ > > + unsigned int sp_id; /* pool id; also node id on NUMA */ > > spinlock_t sp_lock; /* protects all fields */ > > struct list_head sp_sockets; /* pending sockets */ > > unsigned int sp_nrthreads; /* # of threads in pool */ > > @@ -44,12 +44,15 @@ struct svc_pool { > > struct percpu_counter sp_threads_starved; > > struct percpu_counter sp_threads_no_work; > > > > -#define SP_TASK_PENDING (0) /* still work to do even if no > > - * xprt is queued. */ > > -#define SP_CONGESTED (1) > > unsigned long sp_flags; > > } ____cacheline_aligned_in_smp; > > > > +/* bits for sp_flags */ > > +enum { > > + SP_TASK_PENDING, /* still work to do even if no xprt is queued */ > > + SP_CONGESTED, /* all threads are busy, none idle */ > > +}; > > + > > /* > > * RPC service. > > * > > @@ -232,16 +235,6 @@ struct svc_rqst { > > 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 */ > > -#define RQ_DROPME (3) /* drop current reply */ > > -#define RQ_SPLICE_OK (4) /* turned off in gss privacy > > - * to prevent encrypting page > > - * cache pages */ > > -#define RQ_VICTIM (5) /* about to be shut down */ > > -#define RQ_BUSY (6) /* request is busy */ > > -#define RQ_DATA (7) /* request has data */ > > unsigned long rq_flags; /* flags field */ > > ktime_t rq_qtime; /* enqueue time */ > > > > @@ -272,6 +265,19 @@ struct svc_rqst { > > void ** rq_lease_breaker; /* The v4 client breaking a lease */ > > }; > > > > +/* bits for rq_flags */ > > +enum { > > + RQ_SECURE, /* secure port */ > > + RQ_LOCAL, /* local request */ > > + RQ_USEDEFERRAL, /* use deferral */ > > + RQ_DROPME, /* drop current reply */ > > + RQ_SPLICE_OK, /* turned off in gss privacy to prevent encrypting page > > + * cache pages */ > > + RQ_VICTIM, /* about to be shut down */ > > + RQ_BUSY, /* request is busy */ > > + RQ_DATA, /* request has data */ > > +}; > > + > > Also here -- two tab stops instead of one. > > > > #define SVC_NET(rqst) (rqst->rq_xprt ? rqst->rq_xprt->xpt_net : rqst->rq_bc_net) > > > > /* > > diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h > > index a6b12631db21..af383d0349b3 100644 > > --- a/include/linux/sunrpc/svc_xprt.h > > +++ b/include/linux/sunrpc/svc_xprt.h > > @@ -56,26 +56,9 @@ struct svc_xprt { > > struct list_head xpt_list; > > struct list_head xpt_ready; > > unsigned long xpt_flags; > > -#define XPT_BUSY 0 /* enqueued/receiving */ > > -#define XPT_CONN 1 /* conn pending */ > > -#define XPT_CLOSE 2 /* dead or dying */ > > -#define XPT_DATA 3 /* data pending */ > > -#define XPT_TEMP 4 /* connected transport */ > > -#define XPT_DEAD 6 /* transport closed */ > > -#define XPT_CHNGBUF 7 /* need to change snd/rcv buf sizes */ > > -#define XPT_DEFERRED 8 /* deferred request pending */ > > -#define XPT_OLD 9 /* used for xprt aging mark+sweep */ > > -#define XPT_LISTENER 10 /* listening endpoint */ > > -#define XPT_CACHE_AUTH 11 /* cache auth info */ > > -#define XPT_LOCAL 12 /* connection from loopback interface */ > > -#define XPT_KILL_TEMP 13 /* call xpo_kill_temp_xprt before closing */ > > -#define XPT_CONG_CTRL 14 /* has congestion control */ > > -#define XPT_HANDSHAKE 15 /* xprt requests a handshake */ > > -#define XPT_TLS_SESSION 16 /* transport-layer security established */ > > -#define XPT_PEER_AUTH 17 /* peer has been authenticated */ > > > > struct svc_serv *xpt_server; /* service for transport */ > > - atomic_t xpt_reserved; /* space on outq that is rsvd */ > > + atomic_t xpt_reserved; /* space on outq that is rsvd */ > > atomic_t xpt_nr_rqsts; /* Number of requests */ > > struct mutex xpt_mutex; /* to serialize sending data */ > > spinlock_t xpt_lock; /* protects sk_deferred > > @@ -96,6 +79,26 @@ struct svc_xprt { > > struct rpc_xprt *xpt_bc_xprt; /* NFSv4.1 backchannel */ > > struct rpc_xprt_switch *xpt_bc_xps; /* NFSv4.1 backchannel */ > > }; > > +/* flag bits for xpt_flags */ > > +enum { > > + XPT_BUSY, /* enqueued/receiving */ > > + XPT_CONN, /* conn pending */ > > + XPT_CLOSE, /* dead or dying */ > > + XPT_DATA, /* data pending */ > > + XPT_TEMP, /* connected transport */ > > + XPT_DEAD, /* transport closed */ > > + XPT_CHNGBUF, /* need to change snd/rcv buf sizes */ > > + XPT_DEFERRED, /* deferred request pending */ > > + XPT_OLD, /* used for xprt aging mark+sweep */ > > + XPT_LISTENER, /* listening endpoint */ > > + XPT_CACHE_AUTH, /* cache auth info */ > > + XPT_LOCAL, /* connection from loopback interface */ > > + XPT_KILL_TEMP, /* call xpo_kill_temp_xprt before closing */ > > + XPT_CONG_CTRL, /* has congestion control */ > > + XPT_HANDSHAKE, /* xprt requests a handshake */ > > + XPT_TLS_SESSION, /* transport-layer security established */ > > + XPT_PEER_AUTH, /* peer has been authenticated */ > > +}; > > > > static inline void unregister_xpt_user(struct svc_xprt *xpt, struct svc_xpt_user *u) > > { > > diff --git a/include/linux/sunrpc/svcauth.h b/include/linux/sunrpc/svcauth.h > > index 6d9cc9080aca..8d1d0d0721d2 100644 > > --- a/include/linux/sunrpc/svcauth.h > > +++ b/include/linux/sunrpc/svcauth.h > > @@ -133,19 +133,22 @@ struct auth_ops { > > int (*set_client)(struct svc_rqst *rq); > > }; > > > > -#define SVC_GARBAGE 1 > > -#define SVC_SYSERR 2 > > -#define SVC_VALID 3 > > -#define SVC_NEGATIVE 4 > > -#define SVC_OK 5 > > -#define SVC_DROP 6 > > -#define SVC_CLOSE 7 /* Like SVC_DROP, but request is definitely > > - * lost so if there is a tcp connection, it > > - * should be closed > > - */ > > -#define SVC_DENIED 8 > > -#define SVC_PENDING 9 > > -#define SVC_COMPLETE 10 > > +/*return values for svc functions that analyse request */ > > +enum { > > + SVC_GARBAGE, > > + SVC_SYSERR, > > + SVC_VALID, > > + SVC_NEGATIVE, > > + SVC_OK, > > + SVC_DROP, > > + SVC_CLOSE, /* Like SVC_DROP, but request is definitely > > + * lost so if there is a tcp connection, it > > + * should be closed > > + */ > > + SVC_DENIED, > > + SVC_PENDING, > > + SVC_COMPLETE, > > +}; > > > > struct svc_xprt; > > > > diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h > > index 700a1e6c047c..1ed2f446010b 100644 > > --- a/include/linux/sunrpc/xprtsock.h > > +++ b/include/linux/sunrpc/xprtsock.h > > @@ -81,17 +81,18 @@ struct sock_xprt { > > }; > > > > /* > > - * TCP RPC flags > > + * TCP RPC flags in ->sock_state > > */ > > -#define XPRT_SOCK_CONNECTING 1U > > -#define XPRT_SOCK_DATA_READY (2) > > -#define XPRT_SOCK_UPD_TIMEOUT (3) > > -#define XPRT_SOCK_WAKE_ERROR (4) > > -#define XPRT_SOCK_WAKE_WRITE (5) > > -#define XPRT_SOCK_WAKE_PENDING (6) > > -#define XPRT_SOCK_WAKE_DISCONNECT (7) > > -#define XPRT_SOCK_CONNECT_SENT (8) > > -#define XPRT_SOCK_NOSPACE (9) > > -#define XPRT_SOCK_IGNORE_RECV (10) > > - > > +enum { > > + XPRT_SOCK_CONNECTING, > > + XPRT_SOCK_DATA_READY, > > + XPRT_SOCK_UPD_TIMEOUT, > > + XPRT_SOCK_WAKE_ERROR, > > + XPRT_SOCK_WAKE_WRITE, > > + XPRT_SOCK_WAKE_PENDING, > > + XPRT_SOCK_WAKE_DISCONNECT, > > + XPRT_SOCK_CONNECT_SENT, > > + XPRT_SOCK_NOSPACE, > > + XPRT_SOCK_IGNORE_RECV, > > +}; > > #endif /* _LINUX_SUNRPC_XPRTSOCK_H */ > > > > > > Let's not change client-side code in this patch. Please split this > hunk out and send it to Trond and Anna separately. >
On Mon, 31 Jul 2023, Chuck Lever wrote: > On Tue, Jul 18, 2023 at 09:37:58AM -0400, Chuck Lever wrote: > > On Tue, Jul 18, 2023 at 04:38:08PM +1000, NeilBrown wrote: > > > When a sequence of numbers are needed for internal-use only, an enum is > > > typically best. The sequence will inevitably need to be changed one > > > day, and having an enum means the developer doesn't need to think about > > > renumbering after insertion or deletion. The patch will be easier to > > > review. > > > > Last sentence needs to define the antecedant of "The patch". > > I've changed the last sentence in the description to "Such patches > will be easier ..." > > I've applied 1/5 through 5/5, with a few cosmetic changes, to the > SUNRPC threads topic branch. 6/6 needed more work: Ah - ok. I was all set to resubmit with various changes and re-ordering. I guess I waited too long. > All this will appear in the nfsd repo later today. Not yet.... Thanks, NeilBrown
On Mon, 31 Jul 2023, NeilBrown wrote: > On Mon, 31 Jul 2023, Chuck Lever wrote: > > On Tue, Jul 18, 2023 at 09:37:58AM -0400, Chuck Lever wrote: > > > On Tue, Jul 18, 2023 at 04:38:08PM +1000, NeilBrown wrote: > > > > When a sequence of numbers are needed for internal-use only, an enum is > > > > typically best. The sequence will inevitably need to be changed one > > > > day, and having an enum means the developer doesn't need to think about > > > > renumbering after insertion or deletion. The patch will be easier to > > > > review. > > > > > > Last sentence needs to define the antecedant of "The patch". > > > > I've changed the last sentence in the description to "Such patches > > will be easier ..." > > > > I've applied 1/5 through 5/5, with a few cosmetic changes, to the > > SUNRPC threads topic branch. 6/6 needed more work: > > Ah - ok. I was all set to resubmit with various changes and > re-ordering. I guess I waited too long. > > > All this will appear in the nfsd repo later today. > > Not yet.... No, they are there - the top date looked rather old so I thought there was nothing new yet. Sorry. NeilBrown
> On Jul 30, 2023, at 6:14 PM, NeilBrown <neilb@suse.de> wrote: > > On Mon, 31 Jul 2023, NeilBrown wrote: >> On Mon, 31 Jul 2023, Chuck Lever wrote: >>> On Tue, Jul 18, 2023 at 09:37:58AM -0400, Chuck Lever wrote: >>>> On Tue, Jul 18, 2023 at 04:38:08PM +1000, NeilBrown wrote: >>>>> When a sequence of numbers are needed for internal-use only, an enum is >>>>> typically best. The sequence will inevitably need to be changed one >>>>> day, and having an enum means the developer doesn't need to think about >>>>> renumbering after insertion or deletion. The patch will be easier to >>>>> review. >>>> >>>> Last sentence needs to define the antecedant of "The patch". >>> >>> I've changed the last sentence in the description to "Such patches >>> will be easier ..." >>> >>> I've applied 1/5 through 5/5, with a few cosmetic changes, to the >>> SUNRPC threads topic branch. 6/6 needed more work: >> >> Ah - ok. I was all set to resubmit with various changes and >> re-ordering. I guess I waited too long. It's a topic branch. Send diffs and I can squash them in. >>> All this will appear in the nfsd repo later today. >> >> Not yet.... > > No, they are there - the top date looked rather old so I thought there > was nothing new yet. Sorry. There are some old changes in there already, but I'm still testing the latest set. -- Chuck Lever
On Mon, 31 Jul 2023, Chuck Lever wrote: > On Tue, Jul 18, 2023 at 09:37:58AM -0400, Chuck Lever wrote: > > On Tue, Jul 18, 2023 at 04:38:08PM +1000, NeilBrown wrote: > > > When a sequence of numbers are needed for internal-use only, an enum is > > > typically best. The sequence will inevitably need to be changed one > > > day, and having an enum means the developer doesn't need to think about > > > renumbering after insertion or deletion. The patch will be easier to > > > review. > > > > Last sentence needs to define the antecedant of "The patch". > > I've changed the last sentence in the description to "Such patches > will be easier ..." > > I've applied 1/5 through 5/5, with a few cosmetic changes, to the > SUNRPC threads topic branch. 6/6 needed more work: > > I've split this into one patch for each new enum. I don't see this in topic-sunrpc-thread-scheduling Commit 11a5027fd416 ("SUNRPC: change various server-side #defines to enum") contains 3 new enums, and the XPT_ and SVC_GARBAGE improvements you mention below cannot be found. Thanks, NeilBrown > > The XPT_ flags change needed TRACE_DEFINE_ENUM macros to make > show_svc_xprt_flags() work properly. Added. > > The new enum for SVC_GARBAGE and friends... those aren't bit flags. > So I've made that a named enum so that it can be used for type- > checking function return values properly. > > I dropped the hunk that changes XPRT_SOCK_CONNECTING and friends. > That should be submitted separately to Anna and Trond. > > All this will appear in the nfsd repo later today.
> On Jul 30, 2023, at 6:56 PM, NeilBrown <neilb@suse.de> wrote: > > On Mon, 31 Jul 2023, Chuck Lever wrote: >> On Tue, Jul 18, 2023 at 09:37:58AM -0400, Chuck Lever wrote: >>> On Tue, Jul 18, 2023 at 04:38:08PM +1000, NeilBrown wrote: >>>> When a sequence of numbers are needed for internal-use only, an enum is >>>> typically best. The sequence will inevitably need to be changed one >>>> day, and having an enum means the developer doesn't need to think about >>>> renumbering after insertion or deletion. The patch will be easier to >>>> review. >>> >>> Last sentence needs to define the antecedant of "The patch". >> >> I've changed the last sentence in the description to "Such patches >> will be easier ..." >> >> I've applied 1/5 through 5/5, with a few cosmetic changes, to the >> SUNRPC threads topic branch. 6/6 needed more work: >> >> I've split this into one patch for each new enum. > > I don't see this in topic-sunrpc-thread-scheduling > Commit 11a5027fd416 ("SUNRPC: change various server-side #defines to enum") > contains 3 new enums, and the XPT_ and SVC_GARBAGE improvements you > mention below cannot be found. Still testing this. >> The XPT_ flags change needed TRACE_DEFINE_ENUM macros to make >> show_svc_xprt_flags() work properly. Added. >> >> The new enum for SVC_GARBAGE and friends... those aren't bit flags. >> So I've made that a named enum so that it can be used for type- >> checking function return values properly. >> >> I dropped the hunk that changes XPRT_SOCK_CONNECTING and friends. >> That should be submitted separately to Anna and Trond. >> >> All this will appear in the nfsd repo later today. -- Chuck Lever
> On Jul 30, 2023, at 11:57 AM, Chuck Lever <chuck.lever@oracle.com> wrote: > > On Tue, Jul 18, 2023 at 09:37:58AM -0400, Chuck Lever wrote: >> On Tue, Jul 18, 2023 at 04:38:08PM +1000, NeilBrown wrote: >>> When a sequence of numbers are needed for internal-use only, an enum is >>> typically best. The sequence will inevitably need to be changed one >>> day, and having an enum means the developer doesn't need to think about >>> renumbering after insertion or deletion. The patch will be easier to >>> review. >> >> Last sentence needs to define the antecedant of "The patch". > > I've changed the last sentence in the description to "Such patches > will be easier ..." > > I've applied 1/5 through 5/5, with a few cosmetic changes, to the > SUNRPC threads topic branch. 6/6 needed more work: > > I've split this into one patch for each new enum. > > The XPT_ flags change needed TRACE_DEFINE_ENUM macros to make > show_svc_xprt_flags() work properly. Added. > > The new enum for SVC_GARBAGE and friends... those aren't bit flags. > So I've made that a named enum so that it can be used for type- > checking function return values properly. > > I dropped the hunk that changes XPRT_SOCK_CONNECTING and friends. > That should be submitted separately to Anna and Trond. > > All this will appear in the nfsd repo later today. I just pushed these changes to topic-sunrpc-thread-scheduling. >>> Signed-off-by: NeilBrown <neilb@suse.de> >>> --- >>> include/linux/sunrpc/cache.h | 11 +++++++---- >>> include/linux/sunrpc/svc.h | 34 ++++++++++++++++++++-------------- >>> include/linux/sunrpc/svc_xprt.h | 39 +++++++++++++++++++++------------------ >>> include/linux/sunrpc/svcauth.h | 29 ++++++++++++++++------------- >>> include/linux/sunrpc/xprtsock.h | 25 +++++++++++++------------ >>> 5 files changed, 77 insertions(+), 61 deletions(-) >>> >>> diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h >>> index 518bd28f5ab8..3cc4f4f0c764 100644 >>> --- a/include/linux/sunrpc/cache.h >>> +++ b/include/linux/sunrpc/cache.h >>> @@ -56,10 +56,13 @@ struct cache_head { >>> struct kref ref; >>> unsigned long flags; >>> }; >>> -#define CACHE_VALID 0 /* Entry contains valid data */ >>> -#define CACHE_NEGATIVE 1 /* Negative entry - there is no match for the key */ >>> -#define CACHE_PENDING 2 /* An upcall has been sent but no reply received yet*/ >>> -#define CACHE_CLEANED 3 /* Entry has been cleaned from cache */ >>> +/* cache_head.flags */ >>> +enum { >>> + CACHE_VALID, /* Entry contains valid data */ >>> + CACHE_NEGATIVE, /* Negative entry - there is no match for the key */ >>> + CACHE_PENDING, /* An upcall has been sent but no reply received yet*/ >>> + CACHE_CLEANED, /* Entry has been cleaned from cache */ >>> +}; >> >> Weird comment of the day: Please use a double-tab before the comments >> to leave room for larger flag names in the future. >> >> >>> #define CACHE_NEW_EXPIRY 120 /* keep new things pending confirmation for 120 seconds */ >>> >>> diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h >>> index f3df7f963653..83f31a09c853 100644 >>> --- a/include/linux/sunrpc/svc.h >>> +++ b/include/linux/sunrpc/svc.h >>> @@ -31,7 +31,7 @@ >>> * node traffic on multi-node NUMA NFS servers. >>> */ >>> struct svc_pool { >>> - unsigned int sp_id; /* pool id; also node id on NUMA */ >>> + unsigned int sp_id; /* pool id; also node id on NUMA */ >>> spinlock_t sp_lock; /* protects all fields */ >>> struct list_head sp_sockets; /* pending sockets */ >>> unsigned int sp_nrthreads; /* # of threads in pool */ >>> @@ -44,12 +44,15 @@ struct svc_pool { >>> struct percpu_counter sp_threads_starved; >>> struct percpu_counter sp_threads_no_work; >>> >>> -#define SP_TASK_PENDING (0) /* still work to do even if no >>> - * xprt is queued. */ >>> -#define SP_CONGESTED (1) >>> unsigned long sp_flags; >>> } ____cacheline_aligned_in_smp; >>> >>> +/* bits for sp_flags */ >>> +enum { >>> + SP_TASK_PENDING, /* still work to do even if no xprt is queued */ >>> + SP_CONGESTED, /* all threads are busy, none idle */ >>> +}; >>> + >>> /* >>> * RPC service. >>> * >>> @@ -232,16 +235,6 @@ struct svc_rqst { >>> 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 */ >>> -#define RQ_DROPME (3) /* drop current reply */ >>> -#define RQ_SPLICE_OK (4) /* turned off in gss privacy >>> - * to prevent encrypting page >>> - * cache pages */ >>> -#define RQ_VICTIM (5) /* about to be shut down */ >>> -#define RQ_BUSY (6) /* request is busy */ >>> -#define RQ_DATA (7) /* request has data */ >>> unsigned long rq_flags; /* flags field */ >>> ktime_t rq_qtime; /* enqueue time */ >>> >>> @@ -272,6 +265,19 @@ struct svc_rqst { >>> void ** rq_lease_breaker; /* The v4 client breaking a lease */ >>> }; >>> >>> +/* bits for rq_flags */ >>> +enum { >>> + RQ_SECURE, /* secure port */ >>> + RQ_LOCAL, /* local request */ >>> + RQ_USEDEFERRAL, /* use deferral */ >>> + RQ_DROPME, /* drop current reply */ >>> + RQ_SPLICE_OK, /* turned off in gss privacy to prevent encrypting page >>> + * cache pages */ >>> + RQ_VICTIM, /* about to be shut down */ >>> + RQ_BUSY, /* request is busy */ >>> + RQ_DATA, /* request has data */ >>> +}; >>> + >> >> Also here -- two tab stops instead of one. >> >> >>> #define SVC_NET(rqst) (rqst->rq_xprt ? rqst->rq_xprt->xpt_net : rqst->rq_bc_net) >>> >>> /* >>> diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h >>> index a6b12631db21..af383d0349b3 100644 >>> --- a/include/linux/sunrpc/svc_xprt.h >>> +++ b/include/linux/sunrpc/svc_xprt.h >>> @@ -56,26 +56,9 @@ struct svc_xprt { >>> struct list_head xpt_list; >>> struct list_head xpt_ready; >>> unsigned long xpt_flags; >>> -#define XPT_BUSY 0 /* enqueued/receiving */ >>> -#define XPT_CONN 1 /* conn pending */ >>> -#define XPT_CLOSE 2 /* dead or dying */ >>> -#define XPT_DATA 3 /* data pending */ >>> -#define XPT_TEMP 4 /* connected transport */ >>> -#define XPT_DEAD 6 /* transport closed */ >>> -#define XPT_CHNGBUF 7 /* need to change snd/rcv buf sizes */ >>> -#define XPT_DEFERRED 8 /* deferred request pending */ >>> -#define XPT_OLD 9 /* used for xprt aging mark+sweep */ >>> -#define XPT_LISTENER 10 /* listening endpoint */ >>> -#define XPT_CACHE_AUTH 11 /* cache auth info */ >>> -#define XPT_LOCAL 12 /* connection from loopback interface */ >>> -#define XPT_KILL_TEMP 13 /* call xpo_kill_temp_xprt before closing */ >>> -#define XPT_CONG_CTRL 14 /* has congestion control */ >>> -#define XPT_HANDSHAKE 15 /* xprt requests a handshake */ >>> -#define XPT_TLS_SESSION 16 /* transport-layer security established */ >>> -#define XPT_PEER_AUTH 17 /* peer has been authenticated */ >>> >>> struct svc_serv *xpt_server; /* service for transport */ >>> - atomic_t xpt_reserved; /* space on outq that is rsvd */ >>> + atomic_t xpt_reserved; /* space on outq that is rsvd */ >>> atomic_t xpt_nr_rqsts; /* Number of requests */ >>> struct mutex xpt_mutex; /* to serialize sending data */ >>> spinlock_t xpt_lock; /* protects sk_deferred >>> @@ -96,6 +79,26 @@ struct svc_xprt { >>> struct rpc_xprt *xpt_bc_xprt; /* NFSv4.1 backchannel */ >>> struct rpc_xprt_switch *xpt_bc_xps; /* NFSv4.1 backchannel */ >>> }; >>> +/* flag bits for xpt_flags */ >>> +enum { >>> + XPT_BUSY, /* enqueued/receiving */ >>> + XPT_CONN, /* conn pending */ >>> + XPT_CLOSE, /* dead or dying */ >>> + XPT_DATA, /* data pending */ >>> + XPT_TEMP, /* connected transport */ >>> + XPT_DEAD, /* transport closed */ >>> + XPT_CHNGBUF, /* need to change snd/rcv buf sizes */ >>> + XPT_DEFERRED, /* deferred request pending */ >>> + XPT_OLD, /* used for xprt aging mark+sweep */ >>> + XPT_LISTENER, /* listening endpoint */ >>> + XPT_CACHE_AUTH, /* cache auth info */ >>> + XPT_LOCAL, /* connection from loopback interface */ >>> + XPT_KILL_TEMP, /* call xpo_kill_temp_xprt before closing */ >>> + XPT_CONG_CTRL, /* has congestion control */ >>> + XPT_HANDSHAKE, /* xprt requests a handshake */ >>> + XPT_TLS_SESSION, /* transport-layer security established */ >>> + XPT_PEER_AUTH, /* peer has been authenticated */ >>> +}; >>> >>> static inline void unregister_xpt_user(struct svc_xprt *xpt, struct svc_xpt_user *u) >>> { >>> diff --git a/include/linux/sunrpc/svcauth.h b/include/linux/sunrpc/svcauth.h >>> index 6d9cc9080aca..8d1d0d0721d2 100644 >>> --- a/include/linux/sunrpc/svcauth.h >>> +++ b/include/linux/sunrpc/svcauth.h >>> @@ -133,19 +133,22 @@ struct auth_ops { >>> int (*set_client)(struct svc_rqst *rq); >>> }; >>> >>> -#define SVC_GARBAGE 1 >>> -#define SVC_SYSERR 2 >>> -#define SVC_VALID 3 >>> -#define SVC_NEGATIVE 4 >>> -#define SVC_OK 5 >>> -#define SVC_DROP 6 >>> -#define SVC_CLOSE 7 /* Like SVC_DROP, but request is definitely >>> - * lost so if there is a tcp connection, it >>> - * should be closed >>> - */ >>> -#define SVC_DENIED 8 >>> -#define SVC_PENDING 9 >>> -#define SVC_COMPLETE 10 >>> +/*return values for svc functions that analyse request */ >>> +enum { >>> + SVC_GARBAGE, >>> + SVC_SYSERR, >>> + SVC_VALID, >>> + SVC_NEGATIVE, >>> + SVC_OK, >>> + SVC_DROP, >>> + SVC_CLOSE, /* Like SVC_DROP, but request is definitely >>> + * lost so if there is a tcp connection, it >>> + * should be closed >>> + */ >>> + SVC_DENIED, >>> + SVC_PENDING, >>> + SVC_COMPLETE, >>> +}; >>> >>> struct svc_xprt; >>> >>> diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h >>> index 700a1e6c047c..1ed2f446010b 100644 >>> --- a/include/linux/sunrpc/xprtsock.h >>> +++ b/include/linux/sunrpc/xprtsock.h >>> @@ -81,17 +81,18 @@ struct sock_xprt { >>> }; >>> >>> /* >>> - * TCP RPC flags >>> + * TCP RPC flags in ->sock_state >>> */ >>> -#define XPRT_SOCK_CONNECTING 1U >>> -#define XPRT_SOCK_DATA_READY (2) >>> -#define XPRT_SOCK_UPD_TIMEOUT (3) >>> -#define XPRT_SOCK_WAKE_ERROR (4) >>> -#define XPRT_SOCK_WAKE_WRITE (5) >>> -#define XPRT_SOCK_WAKE_PENDING (6) >>> -#define XPRT_SOCK_WAKE_DISCONNECT (7) >>> -#define XPRT_SOCK_CONNECT_SENT (8) >>> -#define XPRT_SOCK_NOSPACE (9) >>> -#define XPRT_SOCK_IGNORE_RECV (10) >>> - >>> +enum { >>> + XPRT_SOCK_CONNECTING, >>> + XPRT_SOCK_DATA_READY, >>> + XPRT_SOCK_UPD_TIMEOUT, >>> + XPRT_SOCK_WAKE_ERROR, >>> + XPRT_SOCK_WAKE_WRITE, >>> + XPRT_SOCK_WAKE_PENDING, >>> + XPRT_SOCK_WAKE_DISCONNECT, >>> + XPRT_SOCK_CONNECT_SENT, >>> + XPRT_SOCK_NOSPACE, >>> + XPRT_SOCK_IGNORE_RECV, >>> +}; >>> #endif /* _LINUX_SUNRPC_XPRTSOCK_H */ >>> >>> >> >> Let's not change client-side code in this patch. Please split this >> hunk out and send it to Trond and Anna separately. >> > > -- > Chuck Lever -- Chuck Lever
diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h index 518bd28f5ab8..3cc4f4f0c764 100644 --- a/include/linux/sunrpc/cache.h +++ b/include/linux/sunrpc/cache.h @@ -56,10 +56,13 @@ struct cache_head { struct kref ref; unsigned long flags; }; -#define CACHE_VALID 0 /* Entry contains valid data */ -#define CACHE_NEGATIVE 1 /* Negative entry - there is no match for the key */ -#define CACHE_PENDING 2 /* An upcall has been sent but no reply received yet*/ -#define CACHE_CLEANED 3 /* Entry has been cleaned from cache */ +/* cache_head.flags */ +enum { + CACHE_VALID, /* Entry contains valid data */ + CACHE_NEGATIVE, /* Negative entry - there is no match for the key */ + CACHE_PENDING, /* An upcall has been sent but no reply received yet*/ + CACHE_CLEANED, /* Entry has been cleaned from cache */ +}; #define CACHE_NEW_EXPIRY 120 /* keep new things pending confirmation for 120 seconds */ diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h index f3df7f963653..83f31a09c853 100644 --- a/include/linux/sunrpc/svc.h +++ b/include/linux/sunrpc/svc.h @@ -31,7 +31,7 @@ * node traffic on multi-node NUMA NFS servers. */ struct svc_pool { - unsigned int sp_id; /* pool id; also node id on NUMA */ + unsigned int sp_id; /* pool id; also node id on NUMA */ spinlock_t sp_lock; /* protects all fields */ struct list_head sp_sockets; /* pending sockets */ unsigned int sp_nrthreads; /* # of threads in pool */ @@ -44,12 +44,15 @@ struct svc_pool { struct percpu_counter sp_threads_starved; struct percpu_counter sp_threads_no_work; -#define SP_TASK_PENDING (0) /* still work to do even if no - * xprt is queued. */ -#define SP_CONGESTED (1) unsigned long sp_flags; } ____cacheline_aligned_in_smp; +/* bits for sp_flags */ +enum { + SP_TASK_PENDING, /* still work to do even if no xprt is queued */ + SP_CONGESTED, /* all threads are busy, none idle */ +}; + /* * RPC service. * @@ -232,16 +235,6 @@ struct svc_rqst { 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 */ -#define RQ_DROPME (3) /* drop current reply */ -#define RQ_SPLICE_OK (4) /* turned off in gss privacy - * to prevent encrypting page - * cache pages */ -#define RQ_VICTIM (5) /* about to be shut down */ -#define RQ_BUSY (6) /* request is busy */ -#define RQ_DATA (7) /* request has data */ unsigned long rq_flags; /* flags field */ ktime_t rq_qtime; /* enqueue time */ @@ -272,6 +265,19 @@ struct svc_rqst { void ** rq_lease_breaker; /* The v4 client breaking a lease */ }; +/* bits for rq_flags */ +enum { + RQ_SECURE, /* secure port */ + RQ_LOCAL, /* local request */ + RQ_USEDEFERRAL, /* use deferral */ + RQ_DROPME, /* drop current reply */ + RQ_SPLICE_OK, /* turned off in gss privacy to prevent encrypting page + * cache pages */ + RQ_VICTIM, /* about to be shut down */ + RQ_BUSY, /* request is busy */ + RQ_DATA, /* request has data */ +}; + #define SVC_NET(rqst) (rqst->rq_xprt ? rqst->rq_xprt->xpt_net : rqst->rq_bc_net) /* diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h index a6b12631db21..af383d0349b3 100644 --- a/include/linux/sunrpc/svc_xprt.h +++ b/include/linux/sunrpc/svc_xprt.h @@ -56,26 +56,9 @@ struct svc_xprt { struct list_head xpt_list; struct list_head xpt_ready; unsigned long xpt_flags; -#define XPT_BUSY 0 /* enqueued/receiving */ -#define XPT_CONN 1 /* conn pending */ -#define XPT_CLOSE 2 /* dead or dying */ -#define XPT_DATA 3 /* data pending */ -#define XPT_TEMP 4 /* connected transport */ -#define XPT_DEAD 6 /* transport closed */ -#define XPT_CHNGBUF 7 /* need to change snd/rcv buf sizes */ -#define XPT_DEFERRED 8 /* deferred request pending */ -#define XPT_OLD 9 /* used for xprt aging mark+sweep */ -#define XPT_LISTENER 10 /* listening endpoint */ -#define XPT_CACHE_AUTH 11 /* cache auth info */ -#define XPT_LOCAL 12 /* connection from loopback interface */ -#define XPT_KILL_TEMP 13 /* call xpo_kill_temp_xprt before closing */ -#define XPT_CONG_CTRL 14 /* has congestion control */ -#define XPT_HANDSHAKE 15 /* xprt requests a handshake */ -#define XPT_TLS_SESSION 16 /* transport-layer security established */ -#define XPT_PEER_AUTH 17 /* peer has been authenticated */ struct svc_serv *xpt_server; /* service for transport */ - atomic_t xpt_reserved; /* space on outq that is rsvd */ + atomic_t xpt_reserved; /* space on outq that is rsvd */ atomic_t xpt_nr_rqsts; /* Number of requests */ struct mutex xpt_mutex; /* to serialize sending data */ spinlock_t xpt_lock; /* protects sk_deferred @@ -96,6 +79,26 @@ struct svc_xprt { struct rpc_xprt *xpt_bc_xprt; /* NFSv4.1 backchannel */ struct rpc_xprt_switch *xpt_bc_xps; /* NFSv4.1 backchannel */ }; +/* flag bits for xpt_flags */ +enum { + XPT_BUSY, /* enqueued/receiving */ + XPT_CONN, /* conn pending */ + XPT_CLOSE, /* dead or dying */ + XPT_DATA, /* data pending */ + XPT_TEMP, /* connected transport */ + XPT_DEAD, /* transport closed */ + XPT_CHNGBUF, /* need to change snd/rcv buf sizes */ + XPT_DEFERRED, /* deferred request pending */ + XPT_OLD, /* used for xprt aging mark+sweep */ + XPT_LISTENER, /* listening endpoint */ + XPT_CACHE_AUTH, /* cache auth info */ + XPT_LOCAL, /* connection from loopback interface */ + XPT_KILL_TEMP, /* call xpo_kill_temp_xprt before closing */ + XPT_CONG_CTRL, /* has congestion control */ + XPT_HANDSHAKE, /* xprt requests a handshake */ + XPT_TLS_SESSION, /* transport-layer security established */ + XPT_PEER_AUTH, /* peer has been authenticated */ +}; static inline void unregister_xpt_user(struct svc_xprt *xpt, struct svc_xpt_user *u) { diff --git a/include/linux/sunrpc/svcauth.h b/include/linux/sunrpc/svcauth.h index 6d9cc9080aca..8d1d0d0721d2 100644 --- a/include/linux/sunrpc/svcauth.h +++ b/include/linux/sunrpc/svcauth.h @@ -133,19 +133,22 @@ struct auth_ops { int (*set_client)(struct svc_rqst *rq); }; -#define SVC_GARBAGE 1 -#define SVC_SYSERR 2 -#define SVC_VALID 3 -#define SVC_NEGATIVE 4 -#define SVC_OK 5 -#define SVC_DROP 6 -#define SVC_CLOSE 7 /* Like SVC_DROP, but request is definitely - * lost so if there is a tcp connection, it - * should be closed - */ -#define SVC_DENIED 8 -#define SVC_PENDING 9 -#define SVC_COMPLETE 10 +/*return values for svc functions that analyse request */ +enum { + SVC_GARBAGE, + SVC_SYSERR, + SVC_VALID, + SVC_NEGATIVE, + SVC_OK, + SVC_DROP, + SVC_CLOSE, /* Like SVC_DROP, but request is definitely + * lost so if there is a tcp connection, it + * should be closed + */ + SVC_DENIED, + SVC_PENDING, + SVC_COMPLETE, +}; struct svc_xprt; diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h index 700a1e6c047c..1ed2f446010b 100644 --- a/include/linux/sunrpc/xprtsock.h +++ b/include/linux/sunrpc/xprtsock.h @@ -81,17 +81,18 @@ struct sock_xprt { }; /* - * TCP RPC flags + * TCP RPC flags in ->sock_state */ -#define XPRT_SOCK_CONNECTING 1U -#define XPRT_SOCK_DATA_READY (2) -#define XPRT_SOCK_UPD_TIMEOUT (3) -#define XPRT_SOCK_WAKE_ERROR (4) -#define XPRT_SOCK_WAKE_WRITE (5) -#define XPRT_SOCK_WAKE_PENDING (6) -#define XPRT_SOCK_WAKE_DISCONNECT (7) -#define XPRT_SOCK_CONNECT_SENT (8) -#define XPRT_SOCK_NOSPACE (9) -#define XPRT_SOCK_IGNORE_RECV (10) - +enum { + XPRT_SOCK_CONNECTING, + XPRT_SOCK_DATA_READY, + XPRT_SOCK_UPD_TIMEOUT, + XPRT_SOCK_WAKE_ERROR, + XPRT_SOCK_WAKE_WRITE, + XPRT_SOCK_WAKE_PENDING, + XPRT_SOCK_WAKE_DISCONNECT, + XPRT_SOCK_CONNECT_SENT, + XPRT_SOCK_NOSPACE, + XPRT_SOCK_IGNORE_RECV, +}; #endif /* _LINUX_SUNRPC_XPRTSOCK_H */
When a sequence of numbers are needed for internal-use only, an enum is typically best. The sequence will inevitably need to be changed one day, and having an enum means the developer doesn't need to think about renumbering after insertion or deletion. The patch will be easier to review. Signed-off-by: NeilBrown <neilb@suse.de> --- include/linux/sunrpc/cache.h | 11 +++++++---- include/linux/sunrpc/svc.h | 34 ++++++++++++++++++++-------------- include/linux/sunrpc/svc_xprt.h | 39 +++++++++++++++++++++------------------ include/linux/sunrpc/svcauth.h | 29 ++++++++++++++++------------- include/linux/sunrpc/xprtsock.h | 25 +++++++++++++------------ 5 files changed, 77 insertions(+), 61 deletions(-)