From nobody Mon Dec 06 18:05:18 2021 X-Original-To: dev-commits-src-all@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 A2E0D18CD802; Mon, 6 Dec 2021 18:05:26 +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 4J7BDQ2lqkz3shx; Mon, 6 Dec 2021 18:05:26 +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 1B6I5IZw064436 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Mon, 6 Dec 2021 20:05:21 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua 1B6I5IZw064436 Received: (from kostik@localhost) by tom.home (8.16.1/8.16.1/Submit) id 1B6I5IxJ064435; Mon, 6 Dec 2021 20:05:18 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Mon, 6 Dec 2021 20:05:18 +0200 From: Konstantin Belousov To: Brooks Davis Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: a4e4132fa3bf - main - swapoff(2): replace special device name argument with a structure Message-ID: References: <202112042221.1B4ML7Ov002151@gitrepo.freebsd.org> <20211206172124.GA40392@spindle.one-eyed-alien.net> List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20211206172124.GA40392@spindle.one-eyed-alien.net> 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.5 X-Spam-Checker-Version: SpamAssassin 3.4.5 (2021-03-20) on tom.home X-Rspamd-Queue-Id: 4J7BDQ2lqkz3shx X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-ThisMailContainsUnwantedMimeParts: N On Mon, Dec 06, 2021 at 05:21:24PM +0000, Brooks Davis wrote: > On Sat, Dec 04, 2021 at 10:21:07PM +0000, Konstantin Belousov wrote: > > The branch main has been updated by kib: > > > > URL: https://cgit.FreeBSD.org/src/commit/?id=a4e4132fa3bfadb6047fc0fa5f399f4640460300 > > > > commit a4e4132fa3bfadb6047fc0fa5f399f4640460300 > > Author: Konstantin Belousov > > AuthorDate: 2021-11-29 16:26:31 +0000 > > Commit: Konstantin Belousov > > CommitDate: 2021-12-04 22:20:58 +0000 > > > > swapoff(2): replace special device name argument with a structure > > > > For compatibility, add a placeholder pointer to the start of the > > added struct swapoff_new_args, and use it to distinguish old vs. new > > style of syscall invocation. > > I agree with Jess that this should be a new syscall. > The entry in > sycalls.master now fails to describe the memory footprint of the name > argument. No system call should be created or altered to have a memory > footprint not describable with SAL annotations unless an applicable > standard such as POSIX requires it. Why? Such requirement is not enforced in any way by the syscall processing infrastructure, and more, that annotations are not utilized in any way by the system. Also, I do not remember a discussion anywhere which would indicate that community agreed to this requirement. Since arguably I am the person that added enough new syscalls in recent times (I do not claim that I added the majority but probably quite close to it), I should have been added to the discussion for it to be fair. The only reference I can find that defines what SAL is, is https://docs.microsoft.com/en-us/cpp/c-runtime-library/sal-annotations?view=msvc-170 I can add some annotations there, but I am really surprised to see 'must' statements about it. > > > diff --git a/sys/vm/swap_pager.h b/sys/vm/swap_pager.h > > index 395fbc9957c4..469de3e8eaf4 100644 > > --- a/sys/vm/swap_pager.h > > +++ b/sys/vm/swap_pager.h > > @@ -69,6 +69,14 @@ struct swdevt { > > #define SW_UNMAPPED 0x01 > > #define SW_CLOSING 0x04 > > > > +struct swapoff_new_args { > > + const char *name_old_syscall; > > + const char *name; > > + u_int flags; > > + u_int pad0; > > + uintptr_t pad1[8]; > > +}; > > If you're going to attempt to add future-proofing, please pad with the > assumption that pointers are 128-bit sized and aligned. In this > case, that would mean an uint64_t pad before pad1. If there were done > in place, adding the pad and dropping pad1 to 6 elements would be safe. Again, this is something new and relatively arbitrary. I will do this, I do not see much harm from changing this on HEAD still.