linux-kernel - Re: [PATCH V2] sched: Forward deadline for early tick

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <831bd791-8602-41da-9cc3-a44d91010cfb@126.com>
Date: Mon, 20 Jan 2025 19:42:12 +0800
From: Honglei Wang <jameshongleiwang@....com>
To: zihan zhou <15645113830zzh@...il.com>, vincent.guittot@...aro.org
Cc: bsegall@...gle.com, dietmar.eggemann@....com, juri.lelli@...hat.com,
 linux-kernel@...r.kernel.org, mgorman@...e.de, mingo@...hat.com,
 peterz@...radead.org, rostedt@...dmis.org, vschneid@...hat.com,
 yaowenchao@...com, yaozhenguo@...com
Subject: Re: [PATCH V2] sched: Forward deadline for early tick

Hi Zihan,

On 2025/1/20 15:38, zihan zhou wrote:
> Thanks for your reply!
> 
>> For sure we should provide recommendation but I'm not sure it's worth
>> a warning but better a documentation for user who wants to set their
>> slice
> 
>> Could you create a patch that only updates sysctl_sched_base_slice
>> and normalized_sysctl_sched_base_slice to 700000UL with figures and
>> explanation
> 
> Thank you for your suggestion! Adding a warning is not worth it. I will
> submit a new patch soon, which changes the default value of slice
> and add the code comments about this change. I am not sure if I
> should change the files in the Documentation directory. If I should,
> I will move the content in the comments to the Documentation directory.
> 
>> sysctl_sched_base_slice is updated by update_sysctl() for each cpu
>> hotplug so you can't really set it directly
>>
>> You can then try a 2nd patch which updates it with debugfs but I'm not
>> convince at all by this benefit because it should update the
>> normalized_sysctl_sched_base_slice which is really meaningless for
>> user
> 
> There is indeed a big problem with this, and I overlooked it. Thank you
> for your corrections. I actually intended to add a numerical range
> requirement to slice through sched_base_slice_fops, because I noticed that
> syscalls has a range requirement (NSEC_PER_MSEC/10, NSEC_PER_MSEC*100)
> for modifying slice. However, it seems that the existence of this range is
> not important.
> 
>>> diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
>>> index ff0e5ab4e37c..d0f90d61398f 100644
>>> --- a/kernel/sched/syscalls.c
>>> +++ b/kernel/sched/syscalls.c
>>> @@ -309,6 +309,10 @@ static void __setscheduler_params(struct task_struct *p,
>>>                         p->se.slice = clamp_t(u64, attr->sched_runtime,
>>>                                               NSEC_PER_MSEC/10,   /* HZ=1000 * 10 */
>>>                                               NSEC_PER_MSEC*100); /* HZ=100  / 10 */
>>> +
>>> +                       if (p->se.slice % TICK_NSEC == 0)
>>> +                               pr_warn("It is not recommended to set the slice to an integer multiple of the tick\n");
>>
>> You should better add documentation than a warning
>>
>> Also, this code has moved in fair.c now
> 
> I'm a bit confused here. Has this code been moved from syscalls.c to
> fair.c file? I haven't seen this change on the master kernel.
> Is there any other code repository?
> 

Just FYI. I think Vincent meant this one 2cf9ac40073d ("sched/fair:
Encapsulate set custom slice in a __setparam_fair() function"). You can
check it at the sched/core branch here:
https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git/commit/?h=sched/core&id=2cf9ac40073ddb6a70dd5ef94f1cf45501b696ed

Thanks,
Honglei