svn commit: r344758 - in head/sys/fs: nfs nfsserver

[Edward Napierala]

pon., 4 mar 2019 o 13:20 Konstantin Belousov <kostikbel at gmail.com> napisał(a):
>
> On Mon, Mar 04, 2019 at 01:02:36PM +0000, Edward Tomasz Napierala wrote:
> > Author: trasz
> > Date: Mon Mar  4 13:02:36 2019
> > New Revision: 344758
> > URL: https://svnweb.freebsd.org/changeset/base/344758
> >
> > Log:
> >   Push down td in nfsrvd_dorpc() - make it use curthread instead
> >   of it being explicitly passed as an argument. No functional changes.
> >
> >   The big picture here is that I want to get rid of the 'td' argument
> >   being passed everywhere, and this is the first piece that affects
> >   the NFS server.
> >
> >   Reviewed by:        rmacklem
> >   MFC after:  2 weeks
> >   Sponsored by:       DARPA, AFRL
> >   Differential Revision:      https://reviews.freebsd.org/D19417
> >
> > Modified:
> >   head/sys/fs/nfs/nfs_var.h
> >   head/sys/fs/nfsserver/nfs_nfsdkrpc.c
> >   head/sys/fs/nfsserver/nfs_nfsdsocket.c
> >
> > Modified: head/sys/fs/nfs/nfs_var.h
> > ==============================================================================
> > --- head/sys/fs/nfs/nfs_var.h Mon Mar  4 11:33:49 2019        (r344757)
> > +++ head/sys/fs/nfs/nfs_var.h Mon Mar  4 13:02:36 2019        (r344758)
> > @@ -283,8 +283,7 @@ int nfsrvd_notsupp(struct nfsrv_descript *, int,
> >
> >  /* nfs_nfsdsocket.c */
> >  void nfsrvd_rephead(struct nfsrv_descript *);
> > -void nfsrvd_dorpc(struct nfsrv_descript *, int, u_char *, int, u_int32_t,
> > -    NFSPROC_T *);
> > +void nfsrvd_dorpc(struct nfsrv_descript *, int, u_char *, int, u_int32_t);
> >
> >  /* nfs_nfsdcache.c */
> >  void nfsrvd_initcache(void);
> >
> > Modified: head/sys/fs/nfsserver/nfs_nfsdkrpc.c
> > ==============================================================================
> > --- head/sys/fs/nfsserver/nfs_nfsdkrpc.c      Mon Mar  4 11:33:49 2019        (r344757)
> > +++ head/sys/fs/nfsserver/nfs_nfsdkrpc.c      Mon Mar  4 13:02:36 2019        (r344758)
> > @@ -323,7 +323,6 @@ static int
> >  nfs_proc(struct nfsrv_descript *nd, u_int32_t xid, SVCXPRT *xprt,
> >      struct nfsrvcache **rpp)
> >  {
> > -     struct thread *td = curthread;
> >       int cacherep = RC_DOIT, isdgram, taglen = -1;
> >       struct mbuf *m;
> >       u_char tag[NFSV4_SMALLSTR + 1], *tagstr = NULL;
> > @@ -384,7 +383,7 @@ nfs_proc(struct nfsrv_descript *nd, u_int32_t xid, SVC
> >       if (cacherep == RC_DOIT) {
> >               if ((nd->nd_flag & ND_NFSV41) != 0)
> >                       nd->nd_xprt = xprt;
> > -             nfsrvd_dorpc(nd, isdgram, tagstr, taglen, minorvers, td);
> > +             nfsrvd_dorpc(nd, isdgram, tagstr, taglen, minorvers);
> >               if ((nd->nd_flag & ND_NFSV41) != 0) {
> >                       if (nd->nd_repstat != NFSERR_REPLYFROMCACHE &&
> >                           (nd->nd_flag & ND_SAVEREPLY) != 0) {
> >
> > Modified: head/sys/fs/nfsserver/nfs_nfsdsocket.c
> > ==============================================================================
> > --- head/sys/fs/nfsserver/nfs_nfsdsocket.c    Mon Mar  4 11:33:49 2019        (r344757)
> > +++ head/sys/fs/nfsserver/nfs_nfsdsocket.c    Mon Mar  4 13:02:36 2019        (r344758)
> > @@ -367,7 +367,7 @@ int nfsrv_writerpc[NFS_NPROCS] = { 0, 0, 1, 0, 0, 0, 0
> >
> >  /* local functions */
> >  static void nfsrvd_compound(struct nfsrv_descript *nd, int isdgram,
> > -    u_char *tag, int taglen, u_int32_t minorvers, NFSPROC_T *p);
> > +    u_char *tag, int taglen, u_int32_t minorvers);
> >
> >
> >  /*
> > @@ -475,14 +475,17 @@ nfsrvd_statend(int op, uint64_t bytes, struct bintime
> >   */
> >  APPLESTATIC void
> >  nfsrvd_dorpc(struct nfsrv_descript *nd, int isdgram, u_char *tag, int taglen,
> > -    u_int32_t minorvers, NFSPROC_T *p)
> > +    u_int32_t minorvers)
> >  {
> >       int error = 0, lktype;
> >       vnode_t vp;
> >       mount_t mp = NULL;
> >       struct nfsrvfh fh;
> >       struct nfsexstuff nes;
> > +     struct thread *p;
> >
> > +     p = curthread;
> > +
> >       /*
> >        * Get a locked vnode for the first file handle
> >        */
> > @@ -557,7 +560,7 @@ nfsrvd_dorpc(struct nfsrv_descript *nd, int isdgram, u
> >        * The group is indicated by the value in nfs_retfh[].
> >        */
> >       if (nd->nd_flag & ND_NFSV4) {
> > -             nfsrvd_compound(nd, isdgram, tag, taglen, minorvers, p);
> > +             nfsrvd_compound(nd, isdgram, tag, taglen, minorvers);
> >       } else {
> >               struct bintime start_time;
> >
> > @@ -620,7 +623,7 @@ out:
> >   */
> >  static void
> >  nfsrvd_compound(struct nfsrv_descript *nd, int isdgram, u_char *tag,
> > -    int taglen, u_int32_t minorvers, NFSPROC_T *p)
> > +    int taglen, u_int32_t minorvers)
> >  {
> >       int i, lktype, op, op0 = 0, statsinprog = 0;
> >       u_int32_t *tl;
> > @@ -635,6 +638,9 @@ nfsrvd_compound(struct nfsrv_descript *nd, int isdgram
> >       fsid_t cur_fsid, save_fsid;
> >       static u_int64_t compref = 0;
> >       struct bintime start_time;
> > +     struct thread *p;
> > +
> > +     p = curthread;
> Why do you name it 'p', which is typical for process, and not 'td', you are
> changing most of the code anyway.

To keep the diff size smaller.  You're right, this touches a lot of stuff,
but most of those added lines are temporary anyway - they will be
removed later, when the td is pushed down even more.

> Also I am curious why. It is certainly fine to remove td when it is used
> as a formal placeholder argument only. But when the first action in the
> function is evaluation of curthread() it becomes less obvious.

Again, many/most of those are temporary.  I'm trying to push td down
in small steps, "layer by layer", so it's easy to review.

> curthread() become very cheap on modern amd64, I am not so sure about
> older machines or non-x86 cases.

The main reason is readability.  Right now there's no easy way to tell whether
a function can be passed any td, or if it must be curthread.