lkml.org 
[lkml]   [2008]   [Sep]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH 2.6.27-rc5 resubmit] Fix itimer/many thread hang.
    From
    Date
    On Tue, 2008-09-16 at 10:41 +0200, Ingo Molnar wrote:
    > * Frank Mayhar <fmayhar@google.com> wrote:
    >
    > > > the wider question is, shouldnt the UP case be just the same as the
    > > > SMP case? Especially the type assymetry struct thread_group_cputime
    > > > looks ugly. (and assymetries like that tend to be a constant source
    > > > of breakage like the one below.)
    > >
    > > I'm not overly fond of this one, either; I did it at Roland's
    > > suggestion (it's all _his_ fault, yeah, _that's_ the ticket! :-); his
    > > opinion IIRC was that the UP case will perform better without the
    > > extra pointer dereferences. I agree that it's a potential source of
    > > pain such as the one you point out.
    >
    > i dont know ...
    >
    > lets try that simplification as a delta patch, ok? Please check the
    > before/after size of 'vmlinux' in a 'make defconfig' [with SMP disabled
    > after make defconfig] UP build. If there's visible size difference then
    > Roland's point holds.

    I finally finished this. Yeah, there's a visible size difference,
    although it's small:

    bobble ~/build/linux-2.6>size vmlinux
    text data bss dec hex filename
    6417840 824160 578416 7820416 775480 vmlinux

    bobble ~/build/up-simplify-tree>size vmlinux
    text data bss dec hex filename
    6417999 824160 578416 7820575 77551f vmlinux

    For a difference of 159 bytes. Not big, but nonzero. The delta patch
    (relative to a tree containing all suggested changes _except_ this
    simplification) follows:

    --- ../linux-2.6/include/linux/sched.h 2008-09-16 14:39:26.000000000 -0700
    +++ ./include/linux/sched.h 2008-09-17 10:28:33.000000000 -0700
    @@ -454,15 +454,9 @@ struct task_cputime {
    * This structure contains the version of task_cputime, above, that is
    * used for thread group CPU clock calculations.
    */
    -#ifdef CONFIG_SMP
    struct thread_group_cputime {
    struct task_cputime *totals;
    };
    -#else
    -struct thread_group_cputime {
    - struct task_cputime totals;
    -};
    -#endif

    /*
    * NOTE! "signal_struct" does not have it's own
    @@ -2124,10 +2118,8 @@ static inline int spin_needbreak(spinloc
    /*
    * Thread group CPU time accounting.
    */
    -#ifdef CONFIG_SMP

    -extern int thread_group_cputime_alloc_smp(struct task_struct *);
    -extern void thread_group_cputime_smp(struct task_struct *, struct task_cputime *);
    +extern int thread_group_cputime_alloc(struct task_struct *);

    static inline void thread_group_cputime_init(struct signal_struct *sig)
    {
    @@ -2138,9 +2130,13 @@ static inline int thread_group_cputime_c
    {
    if (curr->signal->cputime.totals)
    return 0;
    - return thread_group_cputime_alloc_smp(curr);
    + return thread_group_cputime_alloc(curr);
    }

    +#ifdef CONFIG_SMP
    +
    +extern void thread_group_cputime_smp(struct task_struct *, struct task_cputime *);
    +
    static inline void thread_group_cputime_free(struct signal_struct *sig)
    {
    free_percpu(sig->cputime.totals);
    @@ -2161,31 +2157,15 @@ static inline void thread_group_cputime(

    #else /* CONFIG_SMP */

    -static inline void thread_group_cputime_init(struct signal_struct *sig)
    -{
    - sig->cputime.totals.utime = cputime_zero;
    - sig->cputime.totals.stime = cputime_zero;
    - sig->cputime.totals.sum_exec_runtime = 0;
    -}
    -
    -static inline int thread_group_cputime_alloc(struct task_struct *tsk)
    -{
    - return 0;
    -}
    -
    static inline void thread_group_cputime_free(struct signal_struct *sig)
    {
    -}
    -
    -static inline int thread_group_cputime_clone_thread(struct task_struct *curr)
    -{
    - return 0;
    + kfree(sig->cputime.totals);
    }

    static inline void thread_group_cputime(struct task_struct *tsk,
    struct task_cputime *cputime)
    {
    - *cputime = tsk->signal->cputime.totals;
    + *cputime = *(tsk->signal->cputime.totals);
    }

    #endif /* CONFIG_SMP */
    --- ../linux-2.6/kernel/posix-cpu-timers.c 2008-09-17 11:18:56.000000000 -0700
    +++ ./kernel/posix-cpu-timers.c 2008-09-17 11:19:20.000000000 -0700
    @@ -16,7 +16,7 @@
    * via thread_group_cputime_clone_thread() when adding a second or subsequent
    * thread to a thread group. Assumes interrupts are enabled when called.
    */
    -int thread_group_cputime_alloc_smp(struct task_struct *tsk)
    +int thread_group_cputime_alloc(struct task_struct *tsk)
    {
    struct signal_struct *sig = tsk->signal;
    struct task_cputime *cputime;
    @@ -80,6 +80,40 @@ void thread_group_cputime_smp(
    }
    }

    +#else /* CONFIG_SMP */
    +
    +/*
    + * Allocate the thread_group_cputime structure appropriately for UP kernels
    + * and fill in the current values of the fields. Called from copy_signal()
    + * via thread_group_cputime_clone_thread() when adding a second or subsequent
    + * thread to a thread group. Assumes interrupts are enabled when called.
    + */
    +int thread_group_cputime_alloc(struct task_struct *tsk)
    +{
    + struct signal_struct *sig = tsk->signal;
    + struct task_cputime *cputime;
    +
    + /*
    + * If we have multiple threads and we don't already have a
    + * per-CPU task_cputime struct (checked in the caller), allocate
    + * one and fill it in with the times accumulated so far. We're
    + * UP but we may have slept in kmalloc so doublecheck to see if
    + * another thread got in before us.
    + */
    + cputime = kmalloc(sizeof(struct task_cputime), GFP_KERNEL);
    + if (cputime == NULL)
    + return -ENOMEM;
    + if (sig->cputime.totals) {
    + kfree(cputime);
    + return 0;
    + }
    + sig->cputime.totals = cputime;
    + cputime->utime = tsk->utime;
    + cputime->stime = tsk->stime;
    + cputime->sum_exec_runtime = tsk->se.sum_exec_runtime;
    + return 0;
    +}
    +
    #endif /* CONFIG_SMP */

    /*
    --- ../linux-2.6/kernel/sched_stats.h 2008-09-17 10:30:06.000000000 -0700
    +++ ./kernel/sched_stats.h 2008-09-17 10:31:31.000000000 -0700
    @@ -354,21 +354,21 @@ static inline void thread_group_cputime_
    struct thread_group_cputime *tgtimes,
    cputime_t cputime)
    {
    - tgtimes->totals.utime = cputime_add(tgtimes->totals.utime, cputime);
    + tgtimes->totals->utime = cputime_add(tgtimes->totals->utime, cputime);
    }

    static inline void thread_group_cputime_account_system(
    struct thread_group_cputime *tgtimes,
    cputime_t cputime)
    {
    - tgtimes->totals.stime = cputime_add(tgtimes->totals.stime, cputime);
    + tgtimes->totals->stime = cputime_add(tgtimes->totals->stime, cputime);
    }

    static inline void thread_group_cputime_account_exec_runtime(
    struct thread_group_cputime *tgtimes,
    unsigned long long ns)
    {
    - tgtimes->totals.sum_exec_runtime += ns;
    + tgtimes->totals->sum_exec_runtime += ns;
    }

    #endif /* CONFIG_SMP */
    --
    Frank Mayhar <fmayhar@google.com>
    Google, Inc.



    \
     
     \ /
      Last update: 2008-09-17 21:07    [W:0.043 / U:61.316 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site