Re: git: bb5f10e51bd5 - main - linux: implement sched_getattr()
Date: Tue, 06 Oct 2026 14:42:46 UTC
> On Oct 5, 2026, at 4:22 PM, Mark Johnston <markj@FreeBSD.org> wrote: > > On Mon, Oct 05, 2026 at 10:49:29PM +0000, Enji Cooper wrote: >> The branch main has been updated by ngie: >> >> URL: https://cgit.FreeBSD.org/src/commit/?id=bb5f10e51bd53771f1a438f36f554256fcbd7d08 >> >> commit bb5f10e51bd53771f1a438f36f554256fcbd7d08 >> Author: Enji Cooper <ngie@FreeBSD.org> >> AuthorDate: 2026-10-05 22:48:22 +0000 >> Commit: Enji Cooper <ngie@FreeBSD.org> >> CommitDate: 2026-10-05 22:49:09 +0000 >> >> linux: implement sched_getattr() >> >> This change adds the necessary glue to translate Linux scheduler policy >> and priority from FreeBSD so this information is available to Linux >> applications. >> >> This unbreaks using the Linux version of Chromium on FreeBSD. >> >> This change was originally submitted via [freebsd/freebsd-src#2370][github-pr]. >> >> MFC after: 2 weeks >> PR: 297468 >> Co-Authored-By: Enji Cooper <ngie@FreeBSD.org> > > The commit author is already set to you. I was trying to be a bit more explicit about having >> Signed-off-by: Joao Bonifacio <joaoboni017@gmail.com> >> >> [github-pr]: https://github.com/freebsd/freebsd-src/pull/2370 >> >> Differential Revision: https://reviews.freebsd.org/D60214 > > https://docs.freebsd.org/en/articles/committers-guide/#_include_appropriate_metadata_in_a_footer > documents the right way to label commits with pull request metadata. > Please follow it instead of inventing your own format. Ok. In case someone is confused, these are Markdown reference links (the rendered version fills in [github-pr] with the link). I have been using them to stay under the <80 col text limit and keep things as legible as possible for humans, but all of the metadata did get shuffled around more than it needed to be. ... >> + attr.size = MIN(args->size, sizeof(attr)); … >> + return (copyout(&attr, args->attr, args->size)); > > So, this lets the caller decide how much stack data they want to copy > out, independent of sizeof(attr). The only constraint is the > `args->size > PAGE_SIZE` check at the beginning of the function. I eliminated a duplicate calculation in the copyout which contradicted the [Linux] documented interface [1]. The interface documented by Linux allows the user to specify the amount of data to copy out (https://www.man7.org/linux/man-pages/man2/sched_getattr.2.html): > The fields of the sched_attr structure are as follows: > > size This field should be set to the size of the structure in bytes, as in sizeof(struct sched_attr). If the provided structure is smaller than the kernel structure, any additional fields are assumed to be '0'. If the provided structure is larger than the kernel structure, the kernel verifies that all additional fields are 0; if they are not, sched_setattr() fails with the error E2BIG and updates size to contain the size of the kernel structure. > > The above behavior when the size of the user-space sched_attr structure does not match the size of the kernel structure allows for future extensibility of the interface. Malformed applications that pass oversize structures won't break in the future if the size of the kernel sched_attr structure is increased. In the future, it could also allow applications that know about a larger user-space sched_attr structure to determine whether they are running on an older kernel that does not support the larger structure. … > E2BIG The buffer specified by size and attr is too small. > > EINVAL size is invalid; that is, it is smaller than the initial version of the sched_attr structure (48 bytes) or larger than the system page size. > In fact, the original author of this commit got this bit right, and the > bug was introduced when you modified their patch. > > Please revert, and in the future be sure to obtain review from kernel > developers before making changes to the syscall layer. Yes, sometimes > patches go unreviewed for longer than we'd like, but that's not an > excuse to simply go ahead and commit if you aren't familiar with the > code being modified. If you're having trouble getting review, please > feel free to reach out to srcmgr@. I reverted the change in 32f5a9f3fb512908805901a1739383766417dd8d . I hit ^C while the change was being pushed because I realize the branch wasn’t clean (see 4abfb452ea2d18c4710dffee7f3e65167d543041), which resulted in the revert commit being pushed to the repo, but didn’t generate the commit email [3]. -Enji 1. I honestly think the Linux interface is overly complicated and that welcomes security issues (like the ones you’re concerned with). I was trying to implement it according to the spec/[Linux] source code (the documentation contradicts itself [2], so I needed to refer to the source code) in order to ensure the interface was compatible with Linux. 2. https://bugzilla.kernel.org/show_bug.cgi?id=222094 3. Why are emails generated synchronously as part of push operations instead of asynchronously…?