lkml.org 
[lkml]   [2010]   [Nov]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [patch 1/4] taskstats: Introduce "struct cdata"
    From
    Date
    Hello Oleg,

    On Thu, 2010-11-25 at 15:23 +0100, Oleg Nesterov wrote:
    > On 11/19, Michael Holzheu wrote:
    > > Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
    > > ---
    > > fs/binfmt_elf.c | 4 +-
    > > fs/exec.c | 2 -
    > > fs/proc/array.c | 16 ++++----
    > > include/linux/sched.h | 22 +++++++----
    > > kernel/exit.c | 86 ++++++++++++++++++++++++----------------------
    > > kernel/posix-cpu-timers.c | 12 +++---
    > > kernel/sys.c | 44 ++++++++++++-----------
    > > 7 files changed, 100 insertions(+), 86 deletions(-)
    >
    > Looks good. In fact, to me it looks like a cleanup.
    >
    > But. You seem to forgot to change kernel/signal.c, no?

    Yes, you are right.

    >
    > And cosmetic nit,
    >
    > > void thread_group_cputime(struct task_struct *tsk, struct task_cputime *times)
    > > {
    > > - struct signal_struct *sig = tsk->signal;
    > > + struct cdata *tcd = &tsk->signal->cdata_threads;
    > > struct task_struct *t;
    > >
    > > - times->utime = sig->utime;
    > > - times->stime = sig->stime;
    > > - times->sum_exec_runtime = sig->sum_sched_runtime;
    > > + times->utime = tcd->utime;
    > > + times->stime = tcd->stime;
    > > + times->sum_exec_runtime = tsk->signal->sum_sched_runtime;
    >
    > Feel free to ignore, but I don't understand why you removed "sig".
    > Afaics,
    >
    > - times->utime = sig->utime;
    > - times->stime = sig->stime;
    > + times->utime = sig->cdata_threads->utime;
    > + times->stime = sig->cdata_threads->stime;
    >
    > looks a bit better.

    Yes, this looks better.

    I attached the corrected patch.

    Michael
    ---
    Subject: taskstats: Introduce "struct cdata"

    From: Michael Holzheu <holzheu@linux.vnet.ibm.com>

    This patch introduces a new structure "struct cdata" that is used to
    store cumulative resource counters for dead child processes and threads.

    Note that there is one asymmetry:
    For "struct task_io_accounting" (ioc) there is no extra accounting field for
    dead threads. One field is used for both dead processes and threads.

    This patch introduces no functional change.

    Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
    ---
    fs/binfmt_elf.c | 4 +-
    fs/exec.c | 2 -
    fs/proc/array.c | 16 ++++----
    include/linux/sched.h | 22 +++++++----
    kernel/exit.c | 86 ++++++++++++++++++++++++----------------------
    kernel/posix-cpu-timers.c | 8 ++--
    kernel/signal.c | 4 +-
    kernel/sys.c | 44 ++++++++++++-----------
    8 files changed, 100 insertions(+), 86 deletions(-)

    --- a/fs/binfmt_elf.c
    +++ b/fs/binfmt_elf.c
    @@ -1296,8 +1296,8 @@ static void fill_prstatus(struct elf_prs
    cputime_to_timeval(p->utime, &prstatus->pr_utime);
    cputime_to_timeval(p->stime, &prstatus->pr_stime);
    }
    - cputime_to_timeval(p->signal->cutime, &prstatus->pr_cutime);
    - cputime_to_timeval(p->signal->cstime, &prstatus->pr_cstime);
    + cputime_to_timeval(p->signal->cdata_wait.utime, &prstatus->pr_cutime);
    + cputime_to_timeval(p->signal->cdata_wait.stime, &prstatus->pr_cstime);
    }

    static int fill_psinfo(struct elf_prpsinfo *psinfo, struct task_struct *p,
    --- a/fs/exec.c
    +++ b/fs/exec.c
    @@ -894,7 +894,7 @@ static int de_thread(struct task_struct

    no_thread_group:
    if (current->mm)
    - setmax_mm_hiwater_rss(&sig->maxrss, current->mm);
    + setmax_mm_hiwater_rss(&sig->cdata_threads.maxrss, current->mm);

    exit_itimers(sig);
    flush_itimer_signals();
    --- a/fs/proc/array.c
    +++ b/fs/proc/array.c
    @@ -413,11 +413,11 @@ static int do_task_stat(struct seq_file
    num_threads = get_nr_threads(task);
    collect_sigign_sigcatch(task, &sigign, &sigcatch);

    - cmin_flt = sig->cmin_flt;
    - cmaj_flt = sig->cmaj_flt;
    - cutime = sig->cutime;
    - cstime = sig->cstime;
    - cgtime = sig->cgtime;
    + cmin_flt = sig->cdata_wait.min_flt;
    + cmaj_flt = sig->cdata_wait.maj_flt;
    + cutime = sig->cdata_wait.utime;
    + cstime = sig->cdata_wait.stime;
    + cgtime = sig->cdata_wait.gtime;
    rsslim = ACCESS_ONCE(sig->rlim[RLIMIT_RSS].rlim_cur);

    /* add up live thread stats at the group level */
    @@ -430,10 +430,10 @@ static int do_task_stat(struct seq_file
    t = next_thread(t);
    } while (t != task);

    - min_flt += sig->min_flt;
    - maj_flt += sig->maj_flt;
    + min_flt += sig->cdata_threads.min_flt;
    + maj_flt += sig->cdata_threads.maj_flt;
    thread_group_times(task, &utime, &stime);
    - gtime = cputime_add(gtime, sig->gtime);
    + gtime = cputime_add(gtime, sig->cdata_threads.gtime);
    }

    sid = task_session_nr_ns(task, ns);
    --- a/include/linux/sched.h
    +++ b/include/linux/sched.h
    @@ -510,6 +510,17 @@ struct thread_group_cputimer {
    };

    /*
    + * Cumulative resource counters
    + */
    +struct cdata {
    + cputime_t utime, stime, gtime;
    + unsigned long nvcsw, nivcsw;
    + unsigned long min_flt, maj_flt;
    + unsigned long inblock, oublock;
    + unsigned long maxrss;
    +};
    +
    +/*
    * NOTE! "signal_struct" does not have it's own
    * locking, because a shared signal_struct always
    * implies a shared sighand_struct, so locking
    @@ -582,17 +593,12 @@ struct signal_struct {
    * Live threads maintain their own counters and add to these
    * in __exit_signal, except for the group leader.
    */
    - cputime_t utime, stime, cutime, cstime;
    - cputime_t gtime;
    - cputime_t cgtime;
    + struct cdata cdata_wait;
    + struct cdata cdata_threads;
    + struct task_io_accounting ioac;
    #ifndef CONFIG_VIRT_CPU_ACCOUNTING
    cputime_t prev_utime, prev_stime;
    #endif
    - unsigned long nvcsw, nivcsw, cnvcsw, cnivcsw;
    - unsigned long min_flt, maj_flt, cmin_flt, cmaj_flt;
    - unsigned long inblock, oublock, cinblock, coublock;
    - unsigned long maxrss, cmaxrss;
    - struct task_io_accounting ioac;

    /*
    * Cumulative ns of schedule CPU time fo dead threads in the
    --- a/kernel/exit.c
    +++ b/kernel/exit.c
    @@ -95,6 +95,7 @@ static void __exit_signal(struct task_st
    tty = sig->tty;
    sig->tty = NULL;
    } else {
    + struct cdata *tcd = &sig->cdata_threads;
    /*
    * This can only happen if the caller is de_thread().
    * FIXME: this is the temporary hack, we should teach
    @@ -122,15 +123,15 @@ static void __exit_signal(struct task_st
    * We won't ever get here for the group leader, since it
    * will have been the last reference on the signal_struct.
    */
    - sig->utime = cputime_add(sig->utime, tsk->utime);
    - sig->stime = cputime_add(sig->stime, tsk->stime);
    - sig->gtime = cputime_add(sig->gtime, tsk->gtime);
    - sig->min_flt += tsk->min_flt;
    - sig->maj_flt += tsk->maj_flt;
    - sig->nvcsw += tsk->nvcsw;
    - sig->nivcsw += tsk->nivcsw;
    - sig->inblock += task_io_get_inblock(tsk);
    - sig->oublock += task_io_get_oublock(tsk);
    + tcd->utime = cputime_add(tcd->utime, tsk->utime);
    + tcd->stime = cputime_add(tcd->stime, tsk->stime);
    + tcd->gtime = cputime_add(tcd->gtime, tsk->gtime);
    + tcd->min_flt += tsk->min_flt;
    + tcd->maj_flt += tsk->maj_flt;
    + tcd->nvcsw += tsk->nvcsw;
    + tcd->nivcsw += tsk->nivcsw;
    + tcd->inblock += task_io_get_inblock(tsk);
    + tcd->oublock += task_io_get_oublock(tsk);
    task_io_accounting_add(&sig->ioac, &tsk->ioac);
    sig->sum_sched_runtime += tsk->se.sum_exec_runtime;
    }
    @@ -960,10 +961,11 @@ NORET_TYPE void do_exit(long code)
    sync_mm_rss(tsk, tsk->mm);
    group_dead = atomic_dec_and_test(&tsk->signal->live);
    if (group_dead) {
    + struct cdata *tcd = &tsk->signal->cdata_threads;
    hrtimer_cancel(&tsk->signal->real_timer);
    exit_itimers(tsk->signal);
    if (tsk->mm)
    - setmax_mm_hiwater_rss(&tsk->signal->maxrss, tsk->mm);
    + setmax_mm_hiwater_rss(&tcd->maxrss, tsk->mm);
    }
    acct_collect(code, group_dead);
    if (group_dead)
    @@ -1225,8 +1227,7 @@ static int wait_task_zombie(struct wait_
    * !task_detached() to filter out sub-threads.
    */
    if (likely(!traced) && likely(!task_detached(p))) {
    - struct signal_struct *psig;
    - struct signal_struct *sig;
    + struct cdata *cd, *pcd, *tcd;
    unsigned long maxrss;
    cputime_t tgutime, tgstime;

    @@ -1251,40 +1252,43 @@ static int wait_task_zombie(struct wait_
    */
    thread_group_times(p, &tgutime, &tgstime);
    spin_lock_irq(&p->real_parent->sighand->siglock);
    - psig = p->real_parent->signal;
    - sig = p->signal;
    - psig->cutime =
    - cputime_add(psig->cutime,
    + pcd = &p->real_parent->signal->cdata_wait;
    + tcd = &p->signal->cdata_threads;
    + cd = &p->signal->cdata_wait;
    +
    + pcd->utime =
    + cputime_add(pcd->utime,
    cputime_add(tgutime,
    - sig->cutime));
    - psig->cstime =
    - cputime_add(psig->cstime,
    + cd->utime));
    + pcd->stime =
    + cputime_add(pcd->stime,
    cputime_add(tgstime,
    - sig->cstime));
    - psig->cgtime =
    - cputime_add(psig->cgtime,
    + cd->stime));
    + pcd->gtime =
    + cputime_add(pcd->gtime,
    cputime_add(p->gtime,
    - cputime_add(sig->gtime,
    - sig->cgtime)));
    - psig->cmin_flt +=
    - p->min_flt + sig->min_flt + sig->cmin_flt;
    - psig->cmaj_flt +=
    - p->maj_flt + sig->maj_flt + sig->cmaj_flt;
    - psig->cnvcsw +=
    - p->nvcsw + sig->nvcsw + sig->cnvcsw;
    - psig->cnivcsw +=
    - p->nivcsw + sig->nivcsw + sig->cnivcsw;
    - psig->cinblock +=
    + cputime_add(tcd->gtime,
    + cd->gtime)));
    + pcd->min_flt +=
    + p->min_flt + tcd->min_flt + cd->min_flt;
    + pcd->maj_flt +=
    + p->maj_flt + tcd->maj_flt + cd->maj_flt;
    + pcd->nvcsw +=
    + p->nvcsw + tcd->nvcsw + cd->nvcsw;
    + pcd->nivcsw +=
    + p->nivcsw + tcd->nivcsw + cd->nivcsw;
    + pcd->inblock +=
    task_io_get_inblock(p) +
    - sig->inblock + sig->cinblock;
    - psig->coublock +=
    + tcd->inblock + cd->inblock;
    + pcd->oublock +=
    task_io_get_oublock(p) +
    - sig->oublock + sig->coublock;
    - maxrss = max(sig->maxrss, sig->cmaxrss);
    - if (psig->cmaxrss < maxrss)
    - psig->cmaxrss = maxrss;
    - task_io_accounting_add(&psig->ioac, &p->ioac);
    - task_io_accounting_add(&psig->ioac, &sig->ioac);
    + tcd->oublock + cd->oublock;
    + maxrss = max(tcd->maxrss, cd->maxrss);
    + if (pcd->maxrss < maxrss)
    + pcd->maxrss = maxrss;
    + task_io_accounting_add(&p->real_parent->signal->ioac, &p->ioac);
    + task_io_accounting_add(&p->real_parent->signal->ioac,
    + &p->signal->ioac);
    spin_unlock_irq(&p->real_parent->sighand->siglock);
    }

    --- a/kernel/posix-cpu-timers.c
    +++ b/kernel/posix-cpu-timers.c
    @@ -235,8 +235,8 @@ void thread_group_cputime(struct task_st
    struct signal_struct *sig = tsk->signal;
    struct task_struct *t;

    - times->utime = sig->utime;
    - times->stime = sig->stime;
    + times->utime = sig->cdata_threads.utime;
    + times->stime = sig->cdata_threads.stime;
    times->sum_exec_runtime = sig->sum_sched_runtime;

    rcu_read_lock();
    @@ -516,8 +516,8 @@ void posix_cpu_timers_exit_group(struct
    struct signal_struct *const sig = tsk->signal;

    cleanup_timers(tsk->signal->cpu_timers,
    - cputime_add(tsk->utime, sig->utime),
    - cputime_add(tsk->stime, sig->stime),
    + cputime_add(tsk->utime, sig->cdata_threads.utime),
    + cputime_add(tsk->stime, sig->cdata_threads.stime),
    tsk->se.sum_exec_runtime + sig->sum_sched_runtime);
    }

    --- a/kernel/signal.c
    +++ b/kernel/signal.c
    @@ -1476,9 +1476,9 @@ int do_notify_parent(struct task_struct
    rcu_read_unlock();

    info.si_utime = cputime_to_clock_t(cputime_add(tsk->utime,
    - tsk->signal->utime));
    + tsk->signal->cdata_threads.utime));
    info.si_stime = cputime_to_clock_t(cputime_add(tsk->stime,
    - tsk->signal->stime));
    + tsk->signal->cdata_threads.stime));

    info.si_status = tsk->exit_code & 0x7f;
    if (tsk->exit_code & 0x80)
    --- a/kernel/sys.c
    +++ b/kernel/sys.c
    @@ -884,8 +884,8 @@ void do_sys_times(struct tms *tms)

    spin_lock_irq(&current->sighand->siglock);
    thread_group_times(current, &tgutime, &tgstime);
    - cutime = current->signal->cutime;
    - cstime = current->signal->cstime;
    + cutime = current->signal->cdata_wait.utime;
    + cstime = current->signal->cdata_wait.stime;
    spin_unlock_irq(&current->sighand->siglock);
    tms->tms_utime = cputime_to_clock_t(tgutime);
    tms->tms_stime = cputime_to_clock_t(tgstime);
    @@ -1490,6 +1490,7 @@ static void k_getrusage(struct task_stru
    unsigned long flags;
    cputime_t tgutime, tgstime, utime, stime;
    unsigned long maxrss = 0;
    + struct cdata *cd;

    memset((char *) r, 0, sizeof *r);
    utime = stime = cputime_zero;
    @@ -1497,7 +1498,7 @@ static void k_getrusage(struct task_stru
    if (who == RUSAGE_THREAD) {
    task_times(current, &utime, &stime);
    accumulate_thread_rusage(p, r);
    - maxrss = p->signal->maxrss;
    + maxrss = p->signal->cdata_threads.maxrss;
    goto out;
    }

    @@ -1507,31 +1508,34 @@ static void k_getrusage(struct task_stru
    switch (who) {
    case RUSAGE_BOTH:
    case RUSAGE_CHILDREN:
    - utime = p->signal->cutime;
    - stime = p->signal->cstime;
    - r->ru_nvcsw = p->signal->cnvcsw;
    - r->ru_nivcsw = p->signal->cnivcsw;
    - r->ru_minflt = p->signal->cmin_flt;
    - r->ru_majflt = p->signal->cmaj_flt;
    - r->ru_inblock = p->signal->cinblock;
    - r->ru_oublock = p->signal->coublock;
    - maxrss = p->signal->cmaxrss;
    + cd = &p->signal->cdata_wait;
    + utime = cd->utime;
    + stime = cd->stime;
    + r->ru_nvcsw = cd->nvcsw;
    + r->ru_nivcsw = cd->nivcsw;
    + r->ru_minflt = cd->min_flt;
    + r->ru_majflt = cd->maj_flt;
    + r->ru_inblock = cd->inblock;
    + r->ru_oublock = cd->oublock;
    + maxrss = cd->maxrss;

    if (who == RUSAGE_CHILDREN)
    break;

    case RUSAGE_SELF:
    + cd = &p->signal->cdata_threads;
    thread_group_times(p, &tgutime, &tgstime);
    utime = cputime_add(utime, tgutime);
    stime = cputime_add(stime, tgstime);
    - r->ru_nvcsw += p->signal->nvcsw;
    - r->ru_nivcsw += p->signal->nivcsw;
    - r->ru_minflt += p->signal->min_flt;
    - r->ru_majflt += p->signal->maj_flt;
    - r->ru_inblock += p->signal->inblock;
    - r->ru_oublock += p->signal->oublock;
    - if (maxrss < p->signal->maxrss)
    - maxrss = p->signal->maxrss;
    + r->ru_nvcsw += cd->nvcsw;
    + r->ru_nivcsw += cd->nivcsw;
    + r->ru_minflt += cd->min_flt;
    + r->ru_majflt += cd->maj_flt;
    + r->ru_inblock += cd->inblock;
    + r->ru_oublock += cd->oublock;
    + if (maxrss < cd->maxrss)
    + maxrss = cd->maxrss;
    +
    t = p;
    do {
    accumulate_thread_rusage(t, r);



    \
     
     \ /
      Last update: 2010-11-25 17:41    [W:0.052 / U:61.984 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site