Messages in this thread | | | Date | Wed, 3 May 2023 14:29:43 +0100 | From | Qais Yousef <> | Subject | Re: [RFC PATCH v1] sched/uclamp: Introduce SCHED_FLAG_RESET_UCLAMP_ON_FORK flag |
| |
On 04/28/23 11:12, Saravana Kannan wrote: > On Fri, Apr 28, 2023 at 4:57 AM Qais Yousef <qyousef@layalina.io> wrote: > > > > On 04/19/23 18:54, Qais Yousef wrote: > > > > [...] > > > > > I was considering to have something a bit more generic that allows selecting > > > which attributes to reset. > > > > > > For example a syscall with SCHED_FLAG_RESET_ON_FORK_SEL combined with > > > SCHED_FLAG_UCLAMP_MIN/MAX will only reset those. This should make it extensible > > > if we have other similar use cases in the future. The downside it *might* > > > require to be done in a separate syscall to the one that sets these parameter. > > > But it should be done once. > > > > > > Maybe there's a better interface, but I think it makes sense to do it in a way > > > that we won't have to do this again. Would be good to hear from maintainers > > > first before you take my word for it ;-) > > > > Actually I think we can do a better and simpler generic interface. We don't > > need a new flag. We can just add a new parameter for what to reset on fork. > > When this value is 0 (which it should be by default), it means reset > > everything. > > Isn't he default NOT to reset everything?
The default when the RESET_ON_FORK flag is set. This field will not be used otherwise. Like what happens for the other params.
> > > // pseudo code > > > > #define RESET_ON_FORK_ALL 0 > > #define RESET_ON_FORK_POLICY BIT(1) // implies resetting priority > > #define RESET_ON_FORK_PRIORITY BIT(2) > > #define RESET_ON_FORK_UCLAMP BIT(3) > > > > struct sched_attr { > > ... > > __u64 sched_reset_on_fork_flags; > > }; > > > > Also, honestly I think this is over designing for a hypothetical. We
latency_nice is coming next and most likely to require something similar. It's not hypothetical nor over designing. I think it's worthwhile spending time to plan for the future. More interfaces are confusing to the end users. And glibc already complained about evolution of sched_setattr, that's why we don't have a wrapper there yet (beside none of us pushed that hard to resolve the concerns due to lack of bandwidth).
https://public-inbox.org/libc-alpha/87va826rsb.fsf@mid.deneb.enyo.de/
(this thread reminded me linux-api must be CCed)
And there has been various discussions of the need of higher level wrappers/libraries that exposes simpler interface to app developers. So I'm actually expecting this to repeat. I think that was at LPC by Len Brown. I can find this thread on libc mailing list.
https://public-inbox.org/libc-alpha/CAMe9rOpUh1pjfEUqf_hNxce8ZX=4mg6W=n+BbdZSNFHLi7wtkw@mail.gmail.com/
These QoS hints might imply manipulating nice values and I can see ending up with a similar situation where we need to reset nice on fork without resetting other params.
Generally I don't think we should restrict users to self-managed model. A delegated model does make sense, and the latter implies the need for finer control on what to reset.
There's rtkit by the way which already an example of a delegating model to enable creating RT tasks by non privileged users.
Should rtkit force resetting uclamp when on fork? I think it's a grey area and I learn towards it shouldn't.
> have approximately 53 unused bits. By the time we run out of those, > we'd have added at least 20-50 more fields. At that point, we can > always add a flags2 field if we need it. I like David's patch as is -- > it's clear and simple. Add a flag for explicitly what we are trying to > do and extend as needed.
Fair enough. As I said if the maintainers are okay with current proposal, no objection from my side. Based on my experience I didn't expect them to be. And I do think a generic solution is not really complicated and is the better option. You can consider this as a backup plan ;-)
Cheers
-- Qais Yousef
| |