Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 4 Mar 2019 13:31:37 +0000
From:      Edward Napierala <trasz@freebsd.org>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
Subject:   Re: svn commit: r344758 - in head/sys/fs: nfs nfsserver
Message-ID:  <CAFLM3-pLSQ8sBawC9YBTgxdMKhtNtoQG1bn2QVDuw-2tDKb4Gg@mail.gmail.com>
In-Reply-To: <20190304132021.GN68879@kib.kiev.ua>
References:  <201903041302.x24D2aG0093620@repo.freebsd.org> <20190304132021.GN68879@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help 
pon., 4 mar 2019 o 13:20 Konstantin Belousov <kostikbel@gmail.com> napisa=
=C5=82(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
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D
> > --- 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
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D
> > --- 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 =3D curthread;
> >       int cacherep =3D RC_DOIT, isdgram, taglen =3D -1;
> >       struct mbuf *m;
> >       u_char tag[NFSV4_SMALLSTR + 1], *tagstr =3D NULL;
> > @@ -384,7 +383,7 @@ nfs_proc(struct nfsrv_descript *nd, u_int32_t xid, =
SVC
> >       if (cacherep =3D=3D RC_DOIT) {
> >               if ((nd->nd_flag & ND_NFSV41) !=3D 0)
> >                       nd->nd_xprt =3D xprt;
> > -             nfsrvd_dorpc(nd, isdgram, tagstr, taglen, minorvers, td);
> > +             nfsrvd_dorpc(nd, isdgram, tagstr, taglen, minorvers);
> >               if ((nd->nd_flag & ND_NFSV41) !=3D 0) {
> >                       if (nd->nd_repstat !=3D NFSERR_REPLYFROMCACHE &&
> >                           (nd->nd_flag & ND_SAVEREPLY) !=3D 0) {
> >
> > Modified: head/sys/fs/nfsserver/nfs_nfsdsocket.c
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D
> > --- 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] =3D { 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 bin=
time
> >   */
> >  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 =3D 0, lktype;
> >       vnode_t vp;
> >       mount_t mp =3D NULL;
> >       struct nfsrvfh fh;
> >       struct nfsexstuff nes;
> > +     struct thread *p;
> >
> > +     p =3D 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 =3D 0, statsinprog =3D 0;
> >       u_int32_t *tl;
> > @@ -635,6 +638,9 @@ nfsrvd_compound(struct nfsrv_descript *nd, int isdg=
ram
> >       fsid_t cur_fsid, save_fsid;
> >       static u_int64_t compref =3D 0;
> >       struct bintime start_time;
> > +     struct thread *p;
> > +
> > +     p =3D curthread;
> Why do you name it 'p', which is typical for process, and not 'td', you a=
re
> 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 whet=
her
a function can be passed any td, or if it must be curthread.

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAFLM3-pLSQ8sBawC9YBTgxdMKhtNtoQG1bn2QVDuw-2tDKb4Gg>