Date: Mon, 04 Mar 2019 07:16:38 -0800 From: Cy Schubert <Cy.Schubert@cschubert.com> To: Konstantin Belousov <kostikbel@gmail.com>, Edward Napierala <trasz@freebsd.org> 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: <FF146246-2FFD-4FC7-9720-7D72D1AFEB29@cschubert.com> In-Reply-To: <20190304143021.GO68879@kib.kiev.ua> References: <201903041302.x24D2aG0093620@repo.freebsd.org> <20190304132021.GN68879@kib.kiev.ua> <CAFLM3-pLSQ8sBawC9YBTgxdMKhtNtoQG1bn2QVDuw-2tDKb4Gg@mail.gmail.com> <20190304143021.GO68879@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On March 4, 2019 6:30:21 AM PST, Konstantin Belousov <kostikbel@gmail=2Ecom= > wrote: >On Mon, Mar 04, 2019 at 01:31:37PM +0000, Edward Napierala wrote: >> pon=2E, 4 mar 2019 o 13:20 Konstantin Belousov <kostikbel@gmail=2Ecom> >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 <Cy=2ESchubert@cschubert=2Ecom> FreeBSD UNIX: <cy@FreeBSD=2Eorg> Web: http://www=2EFreeBSD=2Eorg The need of the many outweighs the greed of the few=2E
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?FF146246-2FFD-4FC7-9720-7D72D1AFEB29>