From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.1 required=5.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 2923 invoked from network); 2 Oct 2023 19:13:20 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 2 Oct 2023 19:13:20 -0000 Received: (qmail 17792 invoked by uid 550); 2 Oct 2023 19:13:16 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 17771 invoked from network); 2 Oct 2023 19:13:16 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1696273984; x=1696878784; darn=lists.openwall.com; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=UrdshLJ6yY4TrRO+7HKqgHBt/I+RLSjFGYVl7KEYqTI=; b=Y8De7M2e9fX7/aOk/7iIxog9BHcWCKxBTj83kIQMIMj29hzDmycuKOTUwUQqBAtoTJ q766LRhgOzKdnwCdSV1Q8cl57sv2gIuxAEe5/bDfBS2m8ReuIKlPlHF1G+5JuCJibFM/ qRG2pvtITc2FxPEjvtCeqs2UVWeroXA2ETIzSPRusFZHI4SdDA2WTEZv7Hz+k6lyoA6K rcvSxXlUpu/gsHBEU+NORkvSTGu2z746CZLtzaO3EaPC6VvhfGTXllQbI5Z5zhZ2UX24 gJtJaFJVeCJzqwxT9Nq4bZ4HgbhI0BFF3Hm1ygRxLGugeXHY7J0QBJEVbCnCsVfKiGrc HT7Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696273984; x=1696878784; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=UrdshLJ6yY4TrRO+7HKqgHBt/I+RLSjFGYVl7KEYqTI=; b=T5yVx9hoH3bnF77rpvJnpXl6bqvd1r4dXCF6NzebVY0S+bx1ftT7N0/Uncy/CE6t1U h/AvUtzGD3bz7qyFsxPeoKEf0HCowy6Spvff6kVtZdwiCvWcUHEZd6IiXVM4U+B+lGwF TiIhOXVkyLqPJbGbEP+SjXqnzu8iGOoszlb1cRwnozsoqklm1lIGdQk2/78sF8lKQxgK 4UwVG9R9NaDNSLUbfooMEm+U7ct/19ZGcMVXVt00JoP3Unx59Vj+FyYK1GlHmaXFz3oZ 5kBm4UFJsL7+QgRJxUGou/AhBgZ0MQZJ+XkKlqtEEZvrDzx7GRVaoTq3ZW6+7LsTOrfD /01Q== X-Gm-Message-State: AOJu0Yz+CoWoTwbRRDLLjoEHBkybwfRIZ3hla0rQHMa4CKU/om9RYUlH 0Az+aW5jxX9Sh5Mai68bJiLU5XjNUez1UK4asqJN2g== X-Google-Smtp-Source: AGHT+IH1wn5ECQeHv7HdplVxaqOth1RWCOBlGsjk711xwfVqBlnsjjZuBUv1oJq5F0LPcPyQSrvzzHrEmFhKsEkZkSQ= X-Received: by 2002:a0c:b541:0:b0:655:ea8d:666d with SMTP id w1-20020a0cb541000000b00655ea8d666dmr11587167qvd.44.1696273983581; Mon, 02 Oct 2023 12:13:03 -0700 (PDT) MIME-Version: 1.0 References: <20230808030357.1213829-1-kolyshkin@gmail.com> In-Reply-To: <20230808030357.1213829-1-kolyshkin@gmail.com> From: enh Date: Mon, 2 Oct 2023 12:12:48 -0700 Message-ID: To: Kir Kolyshkin Cc: linux-kernel@vger.kernel.org, libc-alpha@sourceware.org, musl@lists.openwall.com, linux-api@vger.kernel.org, Ingo Molnar , Peter Zijlstra , Joel Fernandes , Christian Brauner Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: [musl] Re: [PATCH] sched/headers: move struct sched_param out of uapi On Mon, Aug 7, 2023 at 8:04=E2=80=AFPM Kir Kolyshkin via Libc-alpha wrote: > > Both glibc and musl define struct sched_param in sched.h, while kernel > has it in uapi/linux/sched/types.h, making it cumbersome to use > sched_getattr(2) or sched_setattr(2) from userspace. > > For example, something like this: > > #include > #include > > struct sched_attr sa; > > will result in "error: redefinition of =E2=80=98struct sched_param=E2=80= =99" (note the > code doesn't need sched_param at all -- it needs struct sched_attr > plus some stuff from sched.h). note that `struct sched_attr` is still pretty problematic for userspace because it keeps changing. i (Android's bionic libc maintainer) get pretty frequent complaints about the lack of a wrapper for this in libc, but that doesn't seem plausible if it keeps changing. worse than that, we do find projects copy & pasting `struct sched_attr` (ltp, for example), which causes problems when bionic -- which uses the latest released uapi headers directly -- and those projects' duplicates don't match. it would be helpful (or at least "less problematic") if each new variant was called sched_attr_v1, sched_attr_v2 etc. ironically, the end result of the requests for `struct sched_attr` to be in and to have wrappers for the syscalls is that i'm seriously considering _removing_ `struct sched_attr` from our uapi headers [because when i said we use them "directly", they do actually go through a python script] on the assumption that "everyone just carries around the specific version they want, and no-one has to worry about ABI differences" is less problematic than the current situation. (to be clear: personally i've only seen source incompatibility. although one _could_ pass `struct sched_attr` across library boundaries and have ABI incompatibility, i haven't yet seen that. unless you count "that's the reason why there's no libc wrapper for this syscall, despite it being by far my most-demanded syscall wrapper".) > The situation is, glibc is not going to provide a wrapper for > sched_{get,set}attr, thus the need to include linux/sched_types.h > directly, which leads to the above problem. > > Thus, the userspace is left with a few sub-par choices when it wants to > use e.g. sched_setattr(2), such as maintaining a copy of struct > sched_attr definition, or using some other ugly tricks. > > OTOH, struct sched_param is well known, defined in POSIX, and it won't > be ever changed (as that would break backward compatibility). > > So, while struct sched_param is indeed part of the kernel uapi, > exposing it the way it's done now creates an issue, and hiding it > (like this patch does) fixes that issue, hopefully without creating > another one: common userspace software rely on libc headers, and as > for "special" software (like libc), it looks like glibc and musl > do not rely on kernel headers for struct sched_param definition > (but let's Cc their mailing lists in case it's otherwise). getting back to your actual point about `struct sched_param`, yes, this sgtm too. bionic has the exact same vs duplication. > The alternative to this patch would be to move struct sched_attr to, > say, linux/sched.h, or linux/sched/attr.h (the new file). as long as everyone promises never to change `struct sched_param`, that would be my personal preference --- my _ideal_ is that i never define anything that's uapi and get it "direct" from the [very lightly modified] upstream uapi headers instead. > Oh, and here is the previous attempt to fix the issue: > https://lore.kernel.org/all/20200528135552.GA87103@google.com/ > While I support Linus arguments, the issue is still here > and needs to be fixed. > > Cc: libc-alpha@sourceware.org > Cc: musl@lists.openwall.com > Cc: linux-api@vger.kernel.org > Cc: Ingo Molnar > Cc: Peter Zijlstra > Cc: Joel Fernandes > Cc: Christian Brauner > Fixes: e2d1e2aec572 ("sched/headers: Move various ABI definitions to ") > Signed-off-by: Kir Kolyshkin > --- > include/linux/sched.h | 5 ++++- > include/uapi/linux/sched/types.h | 4 ---- > 2 files changed, 4 insertions(+), 5 deletions(-) > > diff --git a/include/linux/sched.h b/include/linux/sched.h > index 609bde814cb0..3167e97a6b04 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -63,7 +63,6 @@ struct robust_list_head; > struct root_domain; > struct rq; > struct sched_attr; > -struct sched_param; > struct seq_file; > struct sighand_struct; > struct signal_struct; > @@ -370,6 +369,10 @@ extern struct root_domain def_root_domain; > extern struct mutex sched_domains_mutex; > #endif > > +struct sched_param { > + int sched_priority; > +}; > + > struct sched_info { > #ifdef CONFIG_SCHED_INFO > /* Cumulative counters: */ > diff --git a/include/uapi/linux/sched/types.h b/include/uapi/linux/sched/= types.h > index f2c4589d4dbf..90662385689b 100644 > --- a/include/uapi/linux/sched/types.h > +++ b/include/uapi/linux/sched/types.h > @@ -4,10 +4,6 @@ > > #include > > -struct sched_param { > - int sched_priority; > -}; > - > #define SCHED_ATTR_SIZE_VER0 48 /* sizeof first published struct = */ > #define SCHED_ATTR_SIZE_VER1 56 /* add: util_{min,max} */ > > -- > 2.41.0 >