From owner-svn-src-all@freebsd.org Mon Mar 4 15:17:09 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 66E301514C93; Mon, 4 Mar 2019 15:17:09 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Received: from smtp-out-no.shaw.ca (smtp-out-no.shaw.ca [64.59.134.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "Client", Issuer "CA" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 8A8FF6EC6F; Mon, 4 Mar 2019 15:17:08 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Received: from spqr.komquats.com ([70.67.125.17]) by shaw.ca with ESMTPA id 0pKyhBnqI11ZC0pKzhKvPr; Mon, 04 Mar 2019 08:17:06 -0700 X-Authority-Analysis: v=2.3 cv=ctflbGwi c=1 sm=1 tr=0 a=VFtTW3WuZNDh6VkGe7fA3g==:117 a=VFtTW3WuZNDh6VkGe7fA3g==:17 a=IkcTkHD0fZMA:10 a=NTGMnVQrEZIA:10 a=pGLkceISAAAA:8 a=YxBL1-UpAAAA:8 a=6I5d2MoRAAAA:8 a=7Unlge_febPNYF3M47wA:9 a=naNX69SWAkzDNZm_:21 a=QSeD0WeDasypmQEK:21 a=QEXdDO2ut3YA:10 a=Ia-lj3WSrqcvXOmTRaiG:22 a=IjZwj45LgO3ly-622nXo:22 Received: from android-68f84e02b5988183.esitwifi.local (S0106788a207e2972.gv.shawcable.net [70.66.154.233]) by spqr.komquats.com (Postfix) with ESMTPSA id 4CA261F6; Mon, 4 Mar 2019 07:17:03 -0800 (PST) Date: Mon, 04 Mar 2019 07:16:38 -0800 User-Agent: K-9 Mail for Android In-Reply-To: <20190304143021.GO68879@kib.kiev.ua> References: <201903041302.x24D2aG0093620@repo.freebsd.org> <20190304132021.GN68879@kib.kiev.ua> <20190304143021.GO68879@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: svn commit: r344758 - in head/sys/fs: nfs nfsserver To: Konstantin Belousov , Edward Napierala CC: src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org From: Cy Schubert Message-ID: X-CMAE-Envelope: MS4wfP73L2ORxJ60/8mU6p7jncXOiR3FIc3UGzU2THVrGqyDkGPzy+tARlL/nqHYT5dJdNsLvU5NsCn0JYquieUviESmFWjbSaapBUOQMP99aZsSSdO7aDwY DS+Jzl/LbpyxSLuWx8s8cOZVvnrF/3BtHfsywXK1Pux0G6iOQTNL5AzC12wmF0piwGIjYXGFMV9D/pHkmgbQt0++7IUbQk35FCWqPW0q5BaDTPzV2ITCMLUz GMHWi4Zt4NKQ4i1D0BUO8z/9v4nxG9tHr/cslt+cEYAPAzkr7lnAo7iHo1TlYvnsJ9GTCvCe2qNfbUv6tVUu6g== X-Rspamd-Queue-Id: 8A8FF6EC6F X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.95 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.95)[-0.954,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; REPLY(-4.00)[] 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 15:17:09 -0000 On March 4, 2019 6:30:21 AM PST, Konstantin Belousov wrote: >On Mon, Mar 04, 2019 at 01:31:37PM +0000, Edward Napierala wrote: >> pon=2E, 4 mar 2019 o 13:20 Konstantin Belousov >napisa=C5=82(a): >> > > + p =3D curthread; >> > Why do you name it 'p', which is typical for process, and not 'td', >you are >> > changing most of the code anyway=2E >>=20 >> To keep the diff size smaller=2E 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=2E >But if you create code churn, doing it only half way is worse=2E > >>=20 >> > Also I am curious why=2E It is certainly fine to remove td when it is >used >> > as a formal placeholder argument only=2E But when the first action in >the >> > function is evaluation of curthread() it becomes less obvious=2E >>=20 >> Again, many/most of those are temporary=2E I'm trying to push td down >> in small steps, "layer by layer", so it's easy to review=2E >>=20 >> > curthread() become very cheap on modern amd64, I am not so sure >about >> > older machines or non-x86 cases=2E >>=20 >> The main reason is readability=2E Right now there's no easy way to >tell whether >> a function can be passed any td, or if it must be curthread=2E >I must admit that this is the weirdnest argument against 'td' that I >ever >heard=2E I saw more or less reasonable argumentation >- that using less arguments make one more register for argument passing > (amd64 has 6 input arg regs), >- that less arguments make smaller call code=2E >But trust me, in all cases where function can take td !=3D curthread, it >is >either obvious or well-known for anybody who works with that code=2E > >Before you start doing a lot of small changes (AKA continous churn) >please formulate your goals and get some public feedback=2E My immediate >question that I want answered before you ever start touching the code, >is what you plan to do with > sys_syscall(struct thread *td, uap) Agreed on all points=2E At the very least this group of commits should be = reviewed on phabricator=2E Can we back all these commits out until there is a proper review, please? --=20 Pardon the typos and autocorrect, small keyboard in use=2E Cheers, Cy Schubert FreeBSD UNIX: Web: http://www=2EFreeBSD=2Eorg The need of the many outweighs the greed of the few=2E