lkml.org 
[lkml]   [2010]   [Nov]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC][PATCH v2 5/7] taskstats: Improve cumulative CPU time accounting
    From
    Date
    Hello Oleg,

    On Sat, 2010-11-13 at 19:38 +0100, Oleg Nesterov wrote:
    > I already asked you to split these changes, perhaps you can do this?
    > Say, bacct_add_tsk() looks overcomplicated, the change in copy_process()
    > shouldn't introduce the new CLONE_THREAD check, not sure I understand
    > why release_task() was chosen for reparenting, other nits...

    I want to establish the new hierarchy when a new process is forked and
    not for new threads, therefore the check for CLONE_THREAD in
    copy_process(). I do the reparenting with reparent_acct() when a process
    dies, therefore the check for "group_dead" in exit_signal().

    > But it is not easy to discuss these completely different things
    > looking at the single patch.
    >
    > Imho, it would be much better if you make a separate patch which
    > adds acct_parent/etc and implements the parallel hierarchy. This
    > also makes sense because it touches the core kernel.
    >
    > Personally I think that even "struct cdata" + __account_ctime() helper
    > needs a separate patch, and perhaps this change makes sense by itself
    > as cleanup. And this way the "trivial" changes (like the changes in
    > k_getrusage) won't distract from the functional changes.
    >
    > The final change should introduce cdata_acct and actually implement
    > the complete cumulative accounting.

    So you want to have the following three patches:
    * Introduce "struct cdata" + __account_ctime() (no functional change)
    * Add cdata_acct accounting + parallel accounting hierarchy
    * Add taskstats interface to export the data to userspace

    Correct?

    Michael



    \
     
     \ /
      Last update: 2010-11-16 18:37    [W:0.027 / U:0.004 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site