From nobody Sat Sep 24 22:47:19 2022 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4MZkgD64WVz4cTdx; Sat, 24 Sep 2022 22:47:36 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4MZkgD4Dtnz3m5b; Sat, 24 Sep 2022 22:47:36 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.16.1/8.16.1) with ESMTPS id 28OMlJIV047360 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Sun, 25 Sep 2022 01:47:22 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by tom.home (8.16.1/8.16.1/Submit) id 28OMlJlY047359; Sun, 25 Sep 2022 01:47:19 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Sun, 25 Sep 2022 01:47:19 +0300 From: Konstantin Belousov To: Mateusz Guzik Cc: Benjamin Kaduk , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 2c2ef670a79b - main - pseudofs: use the vget_prep/vget_finish idiom Message-ID: References: <202209192009.28JK924f075123@gitrepo.freebsd.org> List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FORGED_GMAIL_RCVD,FREEMAIL_FROM, NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on tom.home X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Rspamd-Queue-Id: 4MZkgD4Dtnz3m5b X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-ThisMailContainsUnwantedMimeParts: N On Sat, Sep 24, 2022 at 09:20:13PM +0200, Mateusz Guzik wrote: > On 9/24/22, Benjamin Kaduk wrote: > > On Thu, Sep 22, 2022 at 3:11 AM Mateusz Guzik wrote: > > > >> On 9/19/22, Benjamin Kaduk wrote: > >> > On Mon, Sep 19, 2022 at 1:09 PM Mateusz Guzik wrote: > >> > > >> >> The branch main has been updated by mjg: > >> >> > >> >> URL: > >> >> > >> https://cgit.FreeBSD.org/src/commit/?id=2c2ef670a79b7f8fa84796a04885a3f76c914762 > >> >> > >> >> commit 2c2ef670a79b7f8fa84796a04885a3f76c914762 > >> >> Author: Mateusz Guzik > >> >> AuthorDate: 2022-09-19 20:07:10 +0000 > >> >> Commit: Mateusz Guzik > >> >> CommitDate: 2022-09-19 20:08:40 +0000 > >> >> > >> >> pseudofs: use the vget_prep/vget_finish idiom > >> >> > >> >> > >> > > >> > Picking an arbitrary commit to reply to: could you please add a bit > >> > more > >> > detail about the "why" to commit messages in the future? > >> > Having looked a little bit, it seems that this would be "as part of the > >> > broader effort to remove the vnode interlock [from a specific class of > >> > operations?]". A pointer to a bigger-picture doc would be great as > >> > well. > >> > >> The idiom at hand is over 3 years old now and employing it at this > >> point should not require a justification. > >> > >> One can easily see not taking the interlock is faster both single- and > >> multi-threaded, which makes the change worthwhile regardless of > >> support for the flag in locking primitives going away or not. > >> > >> > > Maybe you can easily see so, and maybe I can easily see so as well. > > But it is not guaranteed to be the case for everyone looking at the commit > > history. > > The "what" of the commit is contained in the commit itself, but the "why" > > can easily > > be lost to the depths of time; that is why I see it as more important to > > record in the commit > > message. > > > > The what is to employ an in idiom, it being an idiom should require no > justification in the commit message. > > > Indeed, when I was left here to guess at the "why", I got it wrong! So is > > it so hard to add > > another sentence or even a clause like "an easy microoptimization" to the > > commit message? > > Having some hint at the "why" greatly improves the quality of the record > > that is the commit. > > > > What did you think it is? VI_LOCK + vhold disappear in favor of > vget_prep, quick look inside shows there is no interlock taken > whatsoever and that's how one finds out it gets avoided. Performance > implications should be obvious from that point. > > The way I see it extra content in this case decreases the quality > significantly. Anyone familiar with the idiom can comfortably skip > this commit while browsing the history because the one liner indicates > there is nothing more to it. Should extra sentences be present it > would suggest something non-standard is going on and attention is > required, when it is not. It's equivalent to adding comments like "/* > free the buffer */" just prior to "free(buf);". Anyone not familiar > with the idiom can quickly find out what it is. Explaining something > like this every single time a change of the sort is made makes no > sense to me whatsoever. I completely disagree. Commit message must include the justification why the change is correct, in this case. If not to teach the reader why, then at least to allow the reader to clearly agree or not with the justification. I believe this rule is required for any changes that modify synchronization.