From owner-svn-src-all@freebsd.org Mon Mar 4 15:12:24 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 401DC1514949; Mon, 4 Mar 2019 15:12:24 +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 D0CAF6E936; Mon, 4 Mar 2019 15:12:23 +0000 (UTC) (envelope-from etnapierala@gmail.com) Received: by mail-oi1-f196.google.com with SMTP id x187so4073888oia.7; Mon, 04 Mar 2019 07:12:23 -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=L2D0dWS3e/ZUseK0XtiWI0z/IubyOFG7EJUCRHTAAmw=; b=VZJ88KIQVmUDw6Wb4z5ZO6LBQukmHZ1MRIsIbj2SBbC1PMCtldJoS7csM7aCNDi1JS XHhVEO+ThAiOBjEMHi/MLUey46KNrlrq+kSeJIAYnoEDNot2hmNV/BL+kztCg3wzVptO ONyFEsfHKd+uBiepNSwyktZxGPk4kYHYMNhBqAkGIMbYda18q8ewDRNaa+XZR+zcN3hU eRN4M27wapUnSMPNs8mo1MT+kNpZaII7NoXSCLSy8/UerDuQb8OaUYHOFgGAIS1ED62S 2dUKIM/RdWYpbVa8rG22yRIZdv7xSAGIk1vAyluFg1uMkz7rY8EydcG+8QxSK/jfU+KY KdmA== X-Gm-Message-State: AHQUAuZ1p8GdwR4q2dHBuD1qcIwb5yMV2evDJOmi8/lzHlSpqkz4GF4f B0OpLUdxG0JuBrVW1h4cwp5SZf/A+Hg2h1igUFQ= X-Google-Smtp-Source: APXvYqzVc/CiN/TZJkBHrTF1GO8ku1Hz5Di00RV2Z/7PgOl/8CuTFIDo2JDfCwGi8mw/CNwHaUqEW3t7CKwAlX9Dsjw= X-Received: by 2002:aca:4d3:: with SMTP id 202mr11884022oie.31.1551712342564; Mon, 04 Mar 2019 07:12:22 -0800 (PST) MIME-Version: 1.0 References: <201903041302.x24D2aG0093620@repo.freebsd.org> <20190304132021.GN68879@kib.kiev.ua> <20190304143021.GO68879@kib.kiev.ua> In-Reply-To: <20190304143021.GO68879@kib.kiev.ua> From: Edward Napierala Date: Mon, 4 Mar 2019 15:12:11 +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: D0CAF6E936 X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.99 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.99)[-0.987,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 15:12:24 -0000 pon., 4 mar 2019 o 14:30 Konstantin Belousov napisa= =C5=82(a): > > On Mon, Mar 04, 2019 at 01:31:37PM +0000, Edward Napierala wrote: > > pon., 4 mar 2019 o 13:20 Konstantin Belousov napi= sa=C5=82(a): > > > > + p =3D curthread; > > > Why do you name it 'p', which is typical for process, and not 'td', y= ou are > > > changing most of the code anyway. > > > > To keep the diff size smaller. You're right, this touches a lot of stu= ff, > > but most of those added lines are temporary anyway - they will be > > removed later, when the td is pushed down even more. > But if you create code churn, doing it only half way is worse. But this way I create less churn - if I renamed it to 'td', then first I'd change the calls to other functions to take 'td' instead of 'p', and then I'd remo= ve this argument altogether in subsequent commit. This would make diffs larger for no good reason. > > > Also I am curious why. It is certainly fine to remove td when it is u= sed > > > as a formal placeholder argument only. But when the first action in t= he > > > 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. > I must admit that this is the weirdnest argument against 'td' that I ever > heard. 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. > 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. Ah, ok. So, yes, you are right that from a high level point of view it's a poor argument. But at this point I'm not trying to change stuff throughout the kernel; just the NFS server. And the reason for doing this is to make it obvious that the 'td' argument it passes to VOPs and other kernel APIs is always equal to curthread. Doing it layer by layer makes it easy to review. > Before you start doing a lot of small changes (AKA continous churn) > please formulate your goals and get some public feedback. I'll do that; I won't go changing kernel APIs like that. But I'd like to untangle things that complicate the picture, and the NFS server code is one of them. > 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) Probably nothing; those things pretty much always actually use the thread pointer.