Messages in this thread Patch in this message | | | Subject | Re: [PATCH v4 3/4] sched: Allow sched_{get,set}attr to change latency_nice of the task | From | Parth Shah <> | Date | Tue, 25 Feb 2020 20:33:53 +0530 |
| |
On 2/25/20 12:24 PM, Pavan Kondeti wrote: > On Mon, Feb 24, 2020 at 02:29:17PM +0530, Parth Shah wrote: > > [...] > >> diff --git a/kernel/sched/core.c b/kernel/sched/core.c >> index 65b6c00d6dac..e1dc536d4ca3 100644 >> --- a/kernel/sched/core.c >> +++ b/kernel/sched/core.c >> @@ -4723,6 +4723,8 @@ static void __setscheduler_params(struct task_struct *p, >> p->rt_priority = attr->sched_priority; >> p->normal_prio = normal_prio(p); >> set_load_weight(p, true); >> + >> + p->latency_nice = attr->sched_latency_nice; >> } > > We don't want this when SCHED_FLAG_LATENCY_NICE is not set in > attr->flags. > > The user may pass SCHED_FLAG_KEEP_PARAMS | SCHED_FLAG_LATENCY_NICE to > change only latency nice value. So we have to update latency_nice > outside __setscheduler_params(), I think.
AFAICT, passing SCHED_FLAG_KEEP_PARAMS with any other flag will prevent us from changing the latency_nice value because of the below code flow:
__sched_setscheduler() __setscheduler() if (attr->sched_flags & SCHED_FLAG_KEEP_PARAMS) return; __setscheduler_params()
whereas, one thing we still can do is add if condition when setting the value, i.e.,
@@ -4724,7 +4724,8 @@ static void __setscheduler_params(struct task_struct *p, p->normal_prio = normal_prio(p); set_load_weight(p, true);
- p->latency_nice = attr->sched_latency_nice; + if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE) + p->latency_nice = attr->sched_latency_nice; }
>> >> /* Actually do priority change: must hold pi & rq lock. */ >> @@ -4880,6 +4882,13 @@ static int __sched_setscheduler(struct task_struct *p, >> return retval; >> } >> >> + if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE) { >> + if (attr->sched_latency_nice > MAX_LATENCY_NICE) >> + return -EINVAL; >> + if (attr->sched_latency_nice < MIN_LATENCY_NICE) >> + return -EINVAL; >> + } >> + >> if (pi) >> cpuset_read_lock(); >> >> @@ -4914,6 +4923,9 @@ static int __sched_setscheduler(struct task_struct *p, >> goto change; >> if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP) >> goto change; >> + if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE && >> + attr->sched_latency_nice != p->latency_nice) >> + goto change; >> >> p->sched_reset_on_fork = reset_on_fork; >> retval = 0; >> @@ -5162,6 +5174,9 @@ static int sched_copy_attr(struct sched_attr __user *uattr, struct sched_attr *a >> size < SCHED_ATTR_SIZE_VER1) >> return -EINVAL; >> >> + if ((attr->sched_flags & SCHED_FLAG_LATENCY_NICE) && >> + size < SCHED_ATTR_SIZE_VER2) >> + return -EINVAL; >> /* >> * XXX: Do we want to be lenient like existing syscalls; or do we want >> * to be strict and return an error on out-of-bounds values? >> @@ -5391,6 +5406,8 @@ SYSCALL_DEFINE4(sched_getattr, pid_t, pid, struct sched_attr __user *, uattr, >> else >> kattr.sched_nice = task_nice(p); >> >> + kattr.sched_latency_nice = p->latency_nice; >> + > > Can you consider printing latency_nice value in proc_sched_show_task() in this > patch/series?
Sure, I will add it.
Thanks, Parth
| |