lkml.org 
[lkml]   [2013]   [Nov]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 02/14] sched: add extended scheduling interface. (new ABI)

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Thu, Nov 28, 2013 at 12:14:03PM +0100, Juri Lelli wrote:
> > +SYSCALL_DEFINE2(sched_getattr, pid_t, pid, struct sched_attr __user *, attr)
> > {
> > - struct sched_param2 lp;
> > + struct sched_attr lp;
> > struct task_struct *p;
> > int retval;
> >
> > - if (!param2 || pid < 0)
> > + if (!attr || pid < 0)
> > return -EINVAL;
> >
> > + memset(&lp, 0, sizeof(struct sched_attr));
> > +
> > rcu_read_lock();
> > p = find_process_by_pid(pid);
> > retval = -ESRCH;
> > @@ -3427,7 +3495,7 @@ SYSCALL_DEFINE2(sched_getparam2, pid_t, pid, struct sched_param2 __user *, param
> > lp.sched_priority = p->rt_priority;
> > rcu_read_unlock();
> >
> > - retval = copy_to_user(param2, &lp, sizeof(lp)) ? -EFAULT : 0;
> > + retval = copy_to_user(attr, &lp, sizeof(lp)) ? -EFAULT : 0;
> > return retval;
> >
> > out_unlock:
>
>
> So this side needs a bit more care; suppose the kernel has a larger attr
> than userspace knows about.
>
> What would make more sense; add another syscall argument with the
> userspace sizeof(struct sched_attr), or expect userspace to initialize
> attr->size to the right value before calling sched_getattr() ?
>
> To me the extra argument makes more sense; that is:
>
> struct sched_attr attr;
>
> ret = sched_getattr(0, &attr, sizeof(attr));
>
> seems like a saner thing than:
>
> struct sched_attr attr = { .size = sizeof(attr), };
>
> ret = sched_getattr(0, &attr);
>
> Mostly because the former has a clear separation between input and
> output arguments, whereas for the second form the attr argument is
> both input and output.
>
> Ingo?

I suppose so - in the sys_perf_event_open() case we ran out of
arguments, so attr::size was the only sane way to do it.

[ Btw., perf events side note: for completeness and for debugging we
probably want to add a sys_perf_event_get() method as well, to
recover the attributes of an existing event. Unidirectional APIs are
not very nice. ]

Thanks,

Ingo


\
 
 \ /
  Last update: 2013-11-30 15:21    [W:0.126 / U:0.620 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site