Date: Wed, 23 Aug 2023 02:18:12 +0100 From: Jessica Clarke <jrtc27@freebsd.org> To: Konstantin Belousov <kib@FreeBSD.org> Cc: "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org> Subject: Re: git: 4a69fc16a583 - main - Add membarrier(2) Message-ID: <748B7A01-5011-44EE-BB04-282AE96F9B5B@freebsd.org> In-Reply-To: <202308230007.37N07cOK082906@gitrepo.freebsd.org> References: <202308230007.37N07cOK082906@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 23 Aug 2023, at 01:07, Konstantin Belousov <kib@FreeBSD.org> wrote: >=20 > The branch main has been updated by kib: >=20 > URL: = https://cgit.FreeBSD.org/src/commit/?id=3D4a69fc16a583face922319c476f3e739= d9ce9140 >=20 > commit 4a69fc16a583face922319c476f3e739d9ce9140 > Author: Konstantin Belousov <kib@FreeBSD.org> > AuthorDate: 2021-10-07 21:10:07 +0000 > Commit: Konstantin Belousov <kib@FreeBSD.org> > CommitDate: 2023-08-23 00:02:21 +0000 >=20 > Add membarrier(2) >=20 > This is an attempt at clean-room implementation of the Linux' > membarrier(2) syscall. For documentation, you would need to read > both membarrier(2) Linux man page, the comments in Linux > kernel/sched/membarrier.c implementation and possibly look at > actual uses. >=20 > Sponsored by: The FreeBSD Foundation > MFC after: 1 week > Differential revision: https://reviews.freebsd.org/D32360 > --- > ... > diff --git a/sys/sys/membarrier.h b/sys/sys/membarrier.h > new file mode 100644 > index 000000000000..958b769da23e > --- /dev/null > +++ b/sys/sys/membarrier.h > @@ -0,0 +1,70 @@ > +/*- > + * Copyright (c) 2021 The FreeBSD Foundation > + * > + * This software were developed by Konstantin Belousov = <kib@FreeBSD.org> > + * under sponsorship from the FreeBSD Foundation. > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * 1. Redistributions of source code must retain the above copyright > + * notice, this list of conditions and the following disclaimer. > + * 2. Redistributions in binary form must reproduce the above = copyright > + * notice, this list of conditions and the following disclaimer in = the > + * documentation and/or other materials provided with the = distribution. > + * > + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' = AND > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, = THE > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR = PURPOSE > + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE = LIABLE > + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR = CONSEQUENTIAL > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE = GOODS > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS = INTERRUPTION) > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN = CONTRACT, STRICT > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN = ANY WAY > + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE = POSSIBILITY OF > + * SUCH DAMAGE. > + */ > + > +#ifndef __SYS_MEMBARRIER_H__ > +#define __SYS_MEMBARRIER_H__ > + > +#include <sys/cdefs.h> > + > +/* > + * The enum membarrier_cmd values are bits. The MEMBARRIER_CMD_QUERY > + * command returns a bitset indicating which commands are supported. > + * Also the value of MEMBARRIER_CMD_QUERY is zero, so it is > + * effectively not returned by the query. > + */ > +enum membarrier_cmd { > + MEMBARRIER_CMD_QUERY =3D 0x00000000, > + MEMBARRIER_CMD_GLOBAL =3D 0x00000001, > + MEMBARRIER_CMD_SHARED =3D MEMBARRIER_CMD_GLOBAL, > + MEMBARRIER_CMD_GLOBAL_EXPEDITED =3D 0x00000002, > + MEMBARRIER_CMD_REGISTER_GLOBAL_EXPEDITED =3D 0x00000004, > + MEMBARRIER_CMD_PRIVATE_EXPEDITED =3D 0x00000008, > + MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED =3D 0x00000010, > + MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE =3D 0x00000020, > + MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE =3D 0x00000040, > + > + /* > + * RSEQ constants are defined for source compatibility but are > + * not yes supported, MEMBARRIER_CMD_QUERY does not return > + * them in the mask. > + */ > + MEMBARRIER_CMD_PRIVATE_EXPEDITED_RSEQ =3D 0x00000080, > + MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_RSEQ =3D 0x00000100, Or we could not define them. membarrier(2) came into Linux in 4.3, but these two are only since 5.10. Defining something we may or may not ever implement that code should have compatibility for anyway seems like a bad idea. Or should we provide all of the Linux UAPI to FreeBSD processes with ENOSYS stubs? Clearly not. So why is this special? IMO these should not be added unless we have an rseq implementation, which has itself received several objections on the review. I also don=E2=80=99t see why this is suddenly being landed now without = warning. There=E2=80=99s been no activity by you on the review since just over a = year ago. And landing new syscalls days before the 14 branch point doesn=E2=80=99= t seem like great practice to me... Jess
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?748B7A01-5011-44EE-BB04-282AE96F9B5B>