Re: [Patch] export sched_setscheduler() for kernel module use

From: Dean Nelson
Date: Mon Nov 15 2004 - 16:11:12 EST


On Mon, Nov 15, 2004 at 09:41:00PM +0100, Jan Engelhardt wrote:
> >> * Dean Nelson (dcn@xxxxxxx) wrote:
> >> > +int do_sched_setscheduler(pid_t pid, int policy, struct sched_param __user *param)
> >>
> >> this should be static.
> >
> >You're right. I made another change in that one now passes the task_struct
> >pointer to sched_setscheduler() instead of the pid. This requires that
> >the caller of sched_setscheduler() hold the tasklist_lock. The new patch
> >for people's feedback follows.
>
> Hi,
>
> can you elaborate a little why passing the task struct/pid is better/worse,
> respectively?

The idea of passing the task_struct was to avoid the potential cost
of having to search for the task again if one was working with the
task_struct already. We saw in a particular benchmark that had thousands
of processes and a kernel module that called find_process_by_pid() for
each of them as it performed its services on their behalf, that the
cost was quite high. I believe the benchmark was sped up some 16x by
eliminating the need to map a pid to a task structure (i.e., a
task_struct pointers were saved with the pid in this module's
internal tables).

In my particular case (XPC module) this is not an issue because I'm
dealing with the current task, so if I were to pass 0 for the pid,
find_process_by_pid() would return immediately with the current
task_struct pointer.

With passing the task_struct pointer the caller has to be holding
the tasklist_lock, which may be it's main detraction. If one only
has the pid, they can call find_task_by_pid_type() themselves,
since it is an exported symbol.

I'm quite open to go with whatever the community prefers on this
issue.

Thanks,
Dean
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/