Hello,

[email protected], le lun. 27 avril 2026 00:07:12 +0100, a ecrit:
> From: Diego Nieto Cid <[email protected]>
> 
> task_priority_override, which requires a host privileged
> port right, sets the task priority, its procesor set max
> priotrity and its threads priorite and max priority if
> required.
> ---
>  include/mach/mach_host.defs | 11 +++++
>  kern/task.c                 | 94 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 105 insertions(+)
> 
> diff --git a/include/mach/mach_host.defs b/include/mach/mach_host.defs
> index 70a2e866..ce1f89cb 100644
> --- a/include/mach/mach_host.defs
> +++ b/include/mach/mach_host.defs
> @@ -400,3 +400,14 @@ routine  host_get_uptime64(
>  routine processor_set_processors(
>               set_name        : processor_set_name_t;
>       out     processors_list : processor_name_array_t);
> +
> +/*
> + *   Override the priority of a task, its threads
> + *   and processor set, increasing the max priority
> + *   if required.
> + *   Only available to privileged users.
> + */
> +routine      task_priority_override(

I'd rather call it task_override_priority, to follow the existing
practice of putting the verb before the actioned item (set_time,
set_info, etc.)

> +             host_priv       : host_priv_t;
> +             task            : task_t;
> +             priority        : int);
> diff --git a/kern/task.c b/kern/task.c
> index 061a8fa4..2cfd0dc5 100644
> --- a/kern/task.c
> +++ b/kern/task.c
> @@ -1375,3 +1375,97 @@ register_new_task_notification(
>       new_task_notification = notification;
>       return KERN_SUCCESS;
>  }
> +
> +/*
> + *   thread_set_priority:
> + *
> + *   Sets the priority of the given threead.
> + *   Will adjust max_priority if necessary.
> + */
> +static kern_return_t
> +thread_set_priority(
> +     thread_t        thread,
> +     int             priority)

I'd rather call it thread_override_priority, to express that it
overrides/updates the max_priority field of the thread.

> +{
> +     spl_t   s;
> +
> +     s = splsched();
> +     thread_lock(thread);
> +
> +     if (priority < thread->max_priority)
> +             thread->max_priority = priority;
> +     thread->priority = priority;
> +     compute_priority(thread, TRUE);
> +
> +     thread_unlock(thread);
> +     (void) splx(s);
> +
> +     return KERN_SUCCESS;
> +}
> +
> +/*
> + *   Override the priority of a task, its threads
> + *   and processor set, increasing the max priority
> + *   if required.
> + *   Only available to privileged users.
> + */
> +kern_return_t
> +task_priority_override(
> +     const host_t    host,
> +     const task_t    task,
> +     int             priority)
> +{
> +     kern_return_t   ret = KERN_SUCCESS;
> +     processor_set_t pset;
> +
> +     if ((host == HOST_NULL) ||(task == TASK_NULL) ||
> +         invalid_pri(priority))
> +             return KERN_INVALID_ARGUMENT;
> +
> +
> +     /*
> +      * FIXME? thread_create says that processor set must
> +      * be locked first. So we lock the task, get the pset
> +      * then unlock the task to accuire the locks in the right order.
> +      *
> +      * My concerns are:
> +      *
> +      * 1. What happens if a thread is created and we are holding
> +      *    the task lock? May it deadlock if we don't accuire them
> +      *    in the right order?

I don't see why creating a thread would be a concern for acquiring the
task lock?

> +      * 2. Should we call pset_reference just to adjust it's max priority?

Yes, because otherwise it may disappear unexpectedly as soon as
you unlock the task, and writing to a field of an object that has
disappeared means writing into whatever recycled the memory.

That being said, I'm surprised that you'd have to adjust the pset max
priority. Is pset not something shared by several processes?

I would rather see introducing a max_priority in the task itself,
inherited between tasks and checked against the pset max_priority, and
threads would check against the task max_priority in addition to the
pset max_priority.

The default pset max_priority should then be lowered to BASEPRI_SYSTEM,
so that root can go down to that.

That way we can get the unix-like behavior while keeping the capacity to
control priorities via the pset.

> +      * 3. Do all threads run on the same processsor set as the task?

Apparently no since the task processor_set is documented to be for new
threads, and each thread has its own processor_set.

> +      *    If not, shall we care?

You'd want to check against the processor set max priority for each
thread.

> +     task_unlock(task);
> +
> +     pset_lock(pset);
> +     task_lock(task);

The pset may indeed have changed here, you'd need to catch up by
retrying, like is done in thread_create.

But again, I don't think you need to update the pset, just check against
its max_priority value, and thus no need to actually lock it.

> +     task->priority = priority;
> +     pset->max_priority = priority;
> +
> +     {
> +             thread_t        thread;
> +             queue_head_t    *list;
> +
> +             list = &task->thread_list;
> +             queue_iterate(list, thread, thread_t, thread_list) {
> +                     if (thread_set_priority(thread, priority)
> +                             != KERN_SUCCESS)
> +                                     ret = KERN_FAILURE;
> +             }
> +     }
> +
> +     task_unlock(task);
> +     pset_unlock(pset);
> +
> +     return ret;
> +}

Reply via email to