lkml.org 
[lkml]   [2004]   [Nov]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [Lse-tech] [PATCH 2.6.9 2/2] enhanced accounting data collection

    Andrew Morton wrote:
    > Please don't send multiple patches under the same Subject:. It confuses me
    > and breaks my patch processing tools (I strip out the "1/2" numbering
    > because it becomes irrelevant).
    >
    > Please choose a meaningful and distinct title for each patch. See
    > http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt

    Will do better next time :)

    >
    > Jay Lan <jlan@engr.sgi.com> wrote:
    >
    >>2/2: acct_mm
    >>
    >>Enhanced MM accounting data collection.
    >>
    >
    >
    >>Index: linux/include/linux/sched.h
    >>===================================================================
    >>--- linux.orig/include/linux/sched.h 2004-10-01 17:16:35.105905373 -0700
    >>+++ linux/include/linux/sched.h 2004-10-14 12:15:33.450280955 -0700
    >>@@ -249,6 +249,8 @@
    >> struct kioctx *ioctx_list;
    >>
    >> struct kioctx default_kioctx;
    >>+
    >>+ unsigned long hiwater_rss, hiwater_vm;
    >> };
    >
    >
    > unsigned long hiwater_rss; /* comment goes here */
    > unsigned long hiwater_vm; /* and here */

    Will do.

    >
    >
    >>
    >> extern int mmlist_nr;
    >>@@ -593,6 +595,10 @@
    >>
    >> /* i/o counters(bytes read/written, #syscalls */
    >> unsigned long rchar, wchar, syscr, syscw;
    >>+#if defined(CONFIG_BSD_PROCESS_ACCT)
    >>+ u64 acct_rss_mem1, acct_vm_mem1;
    >>+ clock_t acct_stimexpd;
    >>+#endif
    >
    >
    > Please place the above three fields on separate lines and document them.
    >
    > It's not clear to me what, semantically, these fields represent. That's
    > something which is appropriate for the supporting changelog entry.

    Certainly!

    >
    >
    >>+/* Update highwater values */
    >>+static inline void update_mem_hiwater(void)
    >>+{
    >>+ if (current->mm) {
    >>+ if (current->mm->hiwater_rss < current->mm->rss) {
    >>+ current->mm->hiwater_rss = current->mm->rss;
    >>+ }
    >>+ if (current->mm->hiwater_vm < current->mm->total_vm) {
    >>+ current->mm->hiwater_vm = current->mm->total_vm;
    >>+ }
    >>+ }
    >>+}
    >
    >
    > If this has more than one callsite then it it too big to inline.
    >
    > If it has a single callsite then it's OK to inline it, but it can and
    > should be moved into the .c file.
    >
    >
    >>+
    >>+static inline void acct_update_integrals(void)
    >>+{
    >>+ long delta;
    >>+
    >>+ if (current->mm) {
    >>+ delta = current->stime - current->acct_stimexpd;
    >>+ current->acct_stimexpd = current->stime;
    >>+ current->acct_rss_mem1 += delta * current->mm->rss;
    >>+ current->acct_vm_mem1 += delta * current->mm->total_vm;
    >>+ }
    >>+}
    >
    >
    > Consider caching `current' in a local variable - sometimes gcc likes to
    > reevaluate it each time and it takes 14 bytes of code per pop.
    >
    > This function is too big to inline.

    If not inline, where these functions do you suggest to place? How
    about kernel/acct.c?

    >
    >
    >>+static inline void acct_clear_integrals(struct task_struct *tsk)
    >>+{
    >>+ if (tsk) {
    >>+ tsk->acct_stimexpd = 0;
    >>+ tsk->acct_rss_mem1 = 0;
    >>+ tsk->acct_vm_mem1 = 0;
    >>+ }
    >>+}
    >
    >
    > Do any of the callers pass in a null `tsk'?

    No. Just to be safe, i guess :(

    ***

    The revised version of acct_io and acct_mm should be posted soon.

    Thank you very much for reviewing and comments.

    - jay

    >
    >
    >
    > -------------------------------------------------------
    > This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
    > Use IT products in your business? Tell us what you think of them. Give us
    > Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
    > http://productguide.itmanagersjournal.com/guidepromo.tmpl
    > _______________________________________________
    > Lse-tech mailing list
    > Lse-tech@lists.sourceforge.net
    > https://lists.sourceforge.net/lists/listinfo/lse-tech

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 14:07    [W:0.038 / U:1.332 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site