From owner-svn-src-all@freebsd.org Mon Mar 4 14:01:38 2019 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id F187615127AC; Mon, 4 Mar 2019 14:01:37 +0000 (UTC) (envelope-from etnapierala@gmail.com) Received: from mail-oi1-f196.google.com (mail-oi1-f196.google.com [209.85.167.196]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 946D46C021; Mon, 4 Mar 2019 14:01:37 +0000 (UTC) (envelope-from etnapierala@gmail.com) Received: by mail-oi1-f196.google.com with SMTP id z14so3902578oid.0; Mon, 04 Mar 2019 06:01:37 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=JrmA+6FqOlXDsqqZR15yvYTw9axKiuKUF5hFxDHiqKg=; b=av3+ZfvOxSoQKfggHIGXnnte32dVofewkXx0/LFFI11oaFAmdBynh6AHgDwK9KhE0n xHI5nb2NIZqMDlxxUAXoi5b+xonfPTjuq/119T1UslCt4gE2BO9HmHbWzVquPpYngtgX dpfMGADmbv2T1OdR0+YCe0FvhN1T7a2H6XSkrRzDejbgDtJLSfhJH+kGti7F8dxfQsZb zFBBO87e6BJtI/UT0G0jMeU1lkCflAwFe+1frf3yMtBac6FG5xwTGPrdbGJt52rDXYxQ ckDIRub5Klvsv67h5W1iTAr+59vZMOu9m6noShQhb3+9aq0rPmpMpoDfoJlwGua90Tqp QRdg== X-Gm-Message-State: AHQUAuZCjakQjUv1lXjg0ZoZ0k0lLysOYtWpOqPqJEQoSVYk8x2r2qrX 8xQwjEVWnnRZcDk7EsIgdNpFodgGD9wKTuyFMXaB0Q== X-Google-Smtp-Source: APXvYqwhTaDCzsuLDGVKqkEg2q/hCpyZG5dO5iHSNkBXSumIjcIcoHTAxKHwEjFAyJ2ztlcQzWqiwsOxItCt1I8CNHw= X-Received: by 2002:aca:5652:: with SMTP id k79mr12079646oib.19.1551706308446; Mon, 04 Mar 2019 05:31:48 -0800 (PST) MIME-Version: 1.0 References: <201903041302.x24D2aG0093620@repo.freebsd.org> <20190304132021.GN68879@kib.kiev.ua> In-Reply-To: <20190304132021.GN68879@kib.kiev.ua> From: Edward Napierala Date: Mon, 4 Mar 2019 13:31:37 +0000 Message-ID: Subject: Re: svn commit: r344758 - in head/sys/fs: nfs nfsserver To: Konstantin Belousov Cc: src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 946D46C021 X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.97 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.97)[-0.968,0]; REPLY(-4.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0] X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 04 Mar 2019 14:01:38 -0000 pon., 4 mar 2019 o 13:20 Konstantin Belousov 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.