On Mon, Sep 13, 2021 at 11:37:31AM +0000, CGEL wrote: > On Mon, Sep 13, 2021 at 10:13:54AM +0200, Peter Zijlstra wrote: > > On Sun, Sep 12, 2021 at 04:12:23AM +0000, cgel.zte@xxxxxxxxx wrote: > > > From: Yang Yang <yang.yang29@xxxxxxxxxx> > > > > > > The old version sysctl has some problems. First, it allows set value > > > bigger than 1, which is unnecessary. Second, it didn't follow the > > > rule of capabilities. Thirdly, it didn't use static key. This new > > > version fixes all the problems. > > > > Does any of that actually matter? > > For the first problem, I think the reason why sysctl_schedstats() only > accepts 0 or 1, is suitbale for sysctl_child_runs_first(). Since > task_fork_fair() only need sysctl_sched_child_runs_first to be > zero or non-zero. This could potentially break people that already write a larger value in it -- by accident or otherwise. > For the second problem, I remember there is a rule: try to > administration system through capilities but not depends on > root identity. Just like sysctl_schedstats() or other > sysctl_xx(). It seems entirely daft to me; those files are already 644, if root opens the file and passes it along, it gets to keep the pieces. > For the thirdly problem, sysctl_child_runs_first maynot changes > often, but may accessed often, like static_key delayacct_key > controlled by sysctl_delayacct(). Can you actually show it makes a performance difference in a fork micro-bench? Given the amount of gunk fork() already does, I don't think it'll matter one way or the other, and in that case, simpler is better.
