Skip to content

Commit b30f5a7

Browse files
uclamp: Fix sched_setattr call
The struct sched_param passed to sched_setattr() for uclamp needs to have a valid policy and priority set. Currently, sched_data->prio is set to a special value THREAD_PRIORITY_UNCHANGED when the phase does not declare a specific priority, and sched_data->policy is set to "same". This is problematic as the kernel expects a valid policy and priority. We currently pass the special values to the kernel, which makes the syscall fail. Fix that by maintaining a valid policy and priority in thread_data_t->curr_sched_data->{policy,prio} and using it where appropriate.
1 parent 85f29b6 commit b30f5a7

File tree

1 file changed

+69
-16
lines changed

1 file changed

+69
-16
lines changed

src/rt-app.c

Lines changed: 69 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -929,12 +929,17 @@ static void set_thread_membind(thread_data_t *data, numaset_data_t * numa_data)
929929
* sched_priority is only meaningful for RT tasks. Otherwise, it must be
930930
* set to 0 for the setattr syscall to succeed.
931931
*/
932-
static int __sched_priority(thread_data_t *data, sched_data_t *sched_data)
932+
static int __sched_priority(thread_data_t *data, sched_data_t *sched_data, int curr_prio)
933933
{
934+
int prio = sched_data->prio;
935+
936+
if (prio == THREAD_PRIORITY_UNCHANGED)
937+
prio = curr_prio;
938+
934939
switch (sched_data->policy) {
935940
case rr:
936941
case fifo:
937-
return sched_data->prio;
942+
return prio;
938943
}
939944

940945
return 0;
@@ -960,7 +965,13 @@ static bool __set_thread_policy_priority(thread_data_t *data,
960965
struct sched_param param;
961966
int ret;
962967

963-
param.sched_priority = __sched_priority(data, sched_data);
968+
if (sched_data->prio == THREAD_PRIORITY_UNCHANGED) {
969+
log_error("Cannot resolve the priority of the thread %s",
970+
data->name);
971+
exit(EXIT_FAILURE);
972+
}
973+
974+
param.sched_priority = __sched_priority(data, sched_data, sched_data->prio);
964975

965976
ret = pthread_setschedparam(pthread_self(),
966977
sched_data->policy,
@@ -1028,12 +1039,20 @@ static void _set_thread_rt(thread_data_t *data, sched_data_t *sched_data)
10281039
}
10291040

10301041
/* deadline can't rely on the default __set_thread_policy_priority */
1031-
static void _set_thread_deadline(thread_data_t *data, sched_data_t *sched_data)
1042+
static void _set_thread_deadline(thread_data_t *data, sched_data_t *sched_data,
1043+
sched_data_t *curr_sched_data)
10321044
{
10331045
struct sched_attr sa_params = {0};
10341046
unsigned int flags = 0;
10351047
pid_t tid;
10361048
int ret;
1049+
int curr_prio;
1050+
1051+
if (curr_sched_data)
1052+
curr_prio = curr_sched_data->prio;
1053+
else
1054+
/* The value does not matter as it will not be used */
1055+
curr_prio = THREAD_PRIORITY_UNCHANGED;
10371056

10381057
log_debug("[%d] setting scheduler %s exec %lu, deadline %lu"
10391058
" period %lu",
@@ -1051,7 +1070,7 @@ static void _set_thread_deadline(thread_data_t *data, sched_data_t *sched_data)
10511070
sa_params.size = sizeof(struct sched_attr);
10521071
sa_params.sched_flags = 0;
10531072
sa_params.sched_policy = SCHED_DEADLINE;
1054-
sa_params.sched_priority = __sched_priority(data, sched_data);
1073+
sa_params.sched_priority = __sched_priority(data, sched_data, curr_prio);
10551074
sa_params.sched_runtime = sched_data->runtime;
10561075
sa_params.sched_deadline = sched_data->deadline;
10571076
sa_params.sched_period = sched_data->period;
@@ -1066,19 +1085,39 @@ static void _set_thread_deadline(thread_data_t *data, sched_data_t *sched_data)
10661085
}
10671086
}
10681087

1069-
static void _set_thread_uclamp(thread_data_t *data, sched_data_t *sched_data)
1088+
static void _set_thread_uclamp(thread_data_t *data, sched_data_t *sched_data, sched_data_t *curr_sched_data)
10701089
{
10711090
struct sched_attr sa_params = {0};
10721091
unsigned int flags = 0;
10731092
pid_t tid;
10741093
int ret;
1094+
policy_t policy;
1095+
int curr_prio;
10751096

10761097
if ((sched_data->util_min == -2 &&
10771098
sched_data->util_max == -2))
10781099
return;
10791100

1080-
sa_params.sched_policy = sched_data->policy;
1081-
sa_params.sched_priority = __sched_priority(data, sched_data);
1101+
if (curr_sched_data) {
1102+
if (sched_data->policy == same)
1103+
policy = curr_sched_data->policy;
1104+
else
1105+
policy = sched_data->policy;
1106+
1107+
curr_prio = curr_sched_data->prio;
1108+
} else {
1109+
curr_prio = THREAD_PRIORITY_UNCHANGED;
1110+
}
1111+
1112+
1113+
if (policy == same) {
1114+
log_error("Cannot resolve the policy of the thread %s",
1115+
data->name);
1116+
exit(EXIT_FAILURE);
1117+
}
1118+
1119+
sa_params.sched_policy = policy;
1120+
sa_params.sched_priority = __sched_priority(data, sched_data, curr_prio);
10821121
sa_params.size = sizeof(struct sched_attr);
10831122
sa_params.sched_flags = SCHED_FLAG_KEEP_ALL;
10841123
tid = gettid();
@@ -1114,37 +1153,51 @@ static void _set_thread_uclamp(thread_data_t *data, sched_data_t *sched_data)
11141153

11151154
static void set_thread_param(thread_data_t *data, sched_data_t *sched_data)
11161155
{
1156+
sched_data_t *curr_sched_data = data->curr_sched_data;
1157+
11171158
if (!sched_data)
11181159
return;
11191160

1120-
if (data->curr_sched_data == sched_data)
1121-
return;
1161+
if (curr_sched_data) {
1162+
if (curr_sched_data == sched_data)
1163+
return;
1164+
1165+
if (sched_data->prio == curr_sched_data->prio)
1166+
sched_data->prio = THREAD_PRIORITY_UNCHANGED;
11221167

1123-
/* if no policy is specified, reuse the previous one */
1124-
if ((sched_data->policy == same) && data->curr_sched_data)
1125-
sched_data->policy = data->curr_sched_data->policy;
1168+
/* if no policy is specified, reuse the previous one */
1169+
if (sched_data->policy == same)
1170+
sched_data->policy = curr_sched_data->policy;
1171+
}
11261172

11271173
switch (sched_data->policy) {
11281174
case rr:
11291175
case fifo:
11301176
_set_thread_rt(data, sched_data);
1131-
_set_thread_uclamp(data, sched_data);
1177+
_set_thread_uclamp(data, sched_data, curr_sched_data);
11321178
break;
11331179
case other:
11341180
case idle:
11351181
_set_thread_cfs(data, sched_data);
1136-
_set_thread_uclamp(data, sched_data);
1182+
_set_thread_uclamp(data, sched_data, curr_sched_data);
11371183
data->lock_pages = 0; /* forced off */
11381184
break;
11391185
case deadline:
1140-
_set_thread_deadline(data, sched_data);
1186+
_set_thread_deadline(data, sched_data, curr_sched_data);
11411187
break;
11421188
default:
11431189
log_error("Unknown scheduling policy %d",
11441190
sched_data->policy);
11451191
exit(EXIT_FAILURE);
11461192
}
11471193

1194+
/* Ensure we have an actual valid priority in curr_sched_data at all
1195+
* time, since it could be needed in a later phase for sched_setattr()
1196+
* syscall
1197+
*/
1198+
if (sched_data->prio == THREAD_PRIORITY_UNCHANGED)
1199+
sched_data->prio = curr_sched_data->prio;
1200+
11481201
data->curr_sched_data = sched_data;
11491202
}
11501203

0 commit comments

Comments
 (0)