On Thu, 2007-08-30 at 13:49 +0400, Oleg Nesterov wrote:
> On 08/30, Peter Zijlstra wrote:
> >
> > On Thu, 2007-08-30 at 09:50 +0200, Vitaly Mayatskikh wrote:
> > > Short-living process returns its timeslice to the parent, this affects
> > > process that creates a lot of such short-living threads, because its
> > > not a parent for new threads. Patch fixes this issue and doesn't break
> > > kabi as does the patch from reporter: http://lkml.org/lkml/2007/4/7/21
> >
> > > plain text document attachment (2.6.21-timeslice.patch), "proposed
> > > patch"
> > > diff -up -bB ./include/linux/sched.h.orig ./include/linux/sched.h
> > > --- ./include/linux/sched.h.orig 2007-08-21 09:20:22.000000000 +0200
> > > +++ ./include/linux/sched.h 2007-08-27 10:14:06.000000000 +0200
> > > @@ -827,7 +827,9 @@ struct task_struct {
> > >
> > > unsigned long policy;
> > > cpumask_t cpus_allowed;
> > > - unsigned int time_slice, first_time_slice;
> > > + unsigned int time_slice;
> > > + /* Pid of creator */
> > > + unsigned int cpid;
> >
> > might as well make that pid_t, or maybe even a struct pid* and keep a
> > reference on it - the struct pid police might have an opinion.
>
> I agree, "struct pid*" is better, because
>
> 1. we don't need a costly find_pid() in sched_exit()
> 2. we don't suffer from pid re-use problem
Ah, good arguments indeed.
> > > void fastcall sched_exit(struct task_struct *p)
> > > {
> > > unsigned long flags;
> > > struct rq *rq;
> > > -
> > > + struct task_struct* creator = NULL;
> > > /*
> > > * If the child was a (relative-) CPU hog then decrease
> > > - * the sleep_avg of the parent as well.
> > > + * the sleep_avg of the creator as well.
> > > */
> > > - rq = task_rq_lock(p->parent, &flags);
> > > - if (p->first_time_slice && task_cpu(p) == task_cpu(p->parent)) {
> > > - p->parent->time_slice += p->time_slice;
> > > - if (unlikely(p->parent->time_slice > task_timeslice(p)))
> > > - p->parent->time_slice = task_timeslice(p);
> > > + if (p->cpid) {
> > > + struct pid *pid = find_get_pid((pid_t)p->cpid);
> > > + if (pid) {
> > > + creator = get_pid_task(pid, PIDTYPE_PID);
> > > + put_pid(pid);
>
> sched_exit() was removed in 2.6.23-rc.
>
> If you are going to re-introduce this logic, please don't do sched_exit()
> from release_task(). It was done this way just because we can't access
> ->parent after release_task(). But release_task() is called either too
> early, or too late for timeslice accounting, depending on ->exit_signal == -1.
>
> I'd suggest to do this in do_exit(), before the last schedule(). Without
> write_unlock_irq() the code above needs a couple of rcu_read_lock()'s.
>
> I am not sure Ingo will like this change though...
This is not intended as re-introduction of the feature, this stems from
fixing this issue in older (read distro) kernels.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
