public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: KaiGai Kohei <kaigai@ak.jp.nec.com>
To: Andrew Morton <akpm@osdl.org>
Cc: KaiGai Kohei <kaigai@ak.jp.nec.com>, linux-kernel@vger.kernel.org
Subject: Re: Add pacct_struct to fix some pacct bugs.
Date: Thu, 22 Jun 2006 12:08:16 +0900	[thread overview]
Message-ID: <449A09A0.70009@ak.jp.nec.com> (raw)
In-Reply-To: <449A0967.2020701@ak.jp.nec.com>

     [PACCT] two phase process accounting

     The pacct facility need an i/o operation when an accounting record
     is generated. There is a possibility to wake OOM killer up.
     If OOM killer is activated, it kills some processes to make them
     release process memory regions.
     But acct_process() is called in the killed processes context
     before calling exit_mm(), so those processes cannot release
     own memory. In the results, any processes stop in this point and
     it finally cause a system stall.

     ---- in kernel/exit.c : do_exit() ------------
             group_dead = atomic_dec_and_test(&tsk->signal->live);
             if (group_dead) {
                     hrtimer_cancel(&tsk->signal->real_timer);
                     exit_itimers(tsk->signal);
                     acct_process(code);
             }
                :
            - snip -
                :
             exit_mm(tsk);
     ----------------------------------------------

     This patch separates generating an accounting record facility
     into two-phase.
     In the first one, acct_collect() calculate vitual memory size
     of the process and stores it into pacct_struct before exit_mm().
     Then, acct_process() generates an accounting record and write
     it into medium.

     Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com>

diff --git a/include/linux/acct.h b/include/linux/acct.h
index 9a66401..6753687 100644
--- a/include/linux/acct.h
+++ b/include/linux/acct.h
@@ -121,16 +121,20 @@ struct acct_v3
  #ifdef CONFIG_BSD_PROCESS_ACCT
  struct vfsmount;
  struct super_block;
  extern void acct_auto_close_mnt(struct vfsmount *m);
  extern void acct_auto_close(struct super_block *sb);
+extern void acct_init_pacct(struct pacct_struct *pacct);
+extern void acct_collect();
  extern void acct_process(long exitcode);
  extern void acct_update_integrals(struct task_struct *tsk);
  extern void acct_clear_integrals(struct task_struct *tsk);
  #else
  #define acct_auto_close_mnt(x)	do { } while (0)
  #define acct_auto_close(x)	do { } while (0)
+#define acct_init_pacct(x)	do { } while (0)
+#define acct_collect()		do { } while (0)
  #define acct_process(x)		do { } while (0)
  #define acct_update_integrals(x)		do { } while (0)
  #define acct_clear_integrals(task)	do { } while (0)
  #endif

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 29b7d4f..918fdda 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -356,10 +356,14 @@ struct sighand_struct {
  	atomic_t		count;
  	struct k_sigaction	action[_NSIG];
  	spinlock_t		siglock;
  };

+struct pacct_struct {
+	unsigned long		ac_mem;
+};
+
  /*
   * NOTE! "signal_struct" does not have it's own
   * locking, because a shared signal_struct always
   * implies a shared sighand_struct, so locking
   * sighand_struct is always a proper superset of
@@ -447,10 +451,13 @@ struct signal_struct {
  	 * thing in threads created with CLONE_THREAD */
  #ifdef CONFIG_KEYS
  	struct key *session_keyring;	/* keyring inherited over fork */
  	struct key *process_keyring;	/* keyring private to this process */
  #endif
+#ifdef CONFIG_BSD_PROCESS_ACCT
+	struct pacct_struct pacct;	/* per-process accounting information */
+#endif
  };

  /* Context switch must be unlocked if interrupts are to be enabled */
  #ifdef __ARCH_WANT_INTERRUPTS_ON_CTXSW
  # define __ARCH_WANT_UNLOCKED_CTXSW
diff --git a/kernel/acct.c b/kernel/acct.c
index b327f4d..f1a4e12 100644
--- a/kernel/acct.c
+++ b/kernel/acct.c
@@ -419,13 +419,13 @@ static u32 encode_float(u64 value)
  /*
   *  do_acct_process does all actual work. Caller holds the reference to file.
   */
  static void do_acct_process(long exitcode, struct file *file)
  {
+	struct pacct_struct *pacct = &current->signal->pacct;
  	acct_t ac;
  	mm_segment_t fs;
-	unsigned long vsize;
  	unsigned long flim;
  	u64 elapsed;
  	u64 run_time;
  	struct timespec uptime;
  	unsigned long jiffies;
@@ -503,24 +503,13 @@ static void do_acct_process(long exitcod
  		ac.ac_flag |= ASU;
  	if (current->flags & PF_DUMPCORE)
  		ac.ac_flag |= ACORE;
  	if (current->flags & PF_SIGNALED)
  		ac.ac_flag |= AXSIG;
-
-	vsize = 0;
-	if (current->mm) {
-		struct vm_area_struct *vma;
-		down_read(&current->mm->mmap_sem);
-		vma = current->mm->mmap;
-		while (vma) {
-			vsize += vma->vm_end - vma->vm_start;
-			vma = vma->vm_next;
-		}
-		up_read(&current->mm->mmap_sem);
-	}
-	vsize = vsize / 1024;
-	ac.ac_mem = encode_comp_t(vsize);
+	spin_lock(&current->sighand->siglock);
+	ac.ac_mem = encode_comp_t(pacct->ac_mem);
+	spin_unlock(&current->sighand->siglock);
  	ac.ac_io = encode_comp_t(0 /* current->io_usage */);	/* %% */
  	ac.ac_rw = encode_comp_t(ac.ac_io / 1024);
  	ac.ac_minflt = encode_comp_t(current->signal->min_flt +
  				     current->min_flt);
  	ac.ac_majflt = encode_comp_t(current->signal->maj_flt +
@@ -544,10 +533,42 @@ static void do_acct_process(long exitcod
  	current->signal->rlim[RLIMIT_FSIZE].rlim_cur = flim;
  	set_fs(fs);
  }

  /**
+ * acct_init_pacct - initialize a new pacct_struct
+ */
+void acct_init_pacct(struct pacct_struct *pacct)
+{
+	memset(pacct, 0, sizeof(struct pacct_struct));
+}
+
+/**
+ * acct_collect - collect accounting information into pacct_struct
+ */
+void acct_collect(void)
+{
+	struct pacct_struct *pacct = &current->signal->pacct;
+	unsigned long vsize = 0;
+
+	if (current->mm) {
+		struct vm_area_struct *vma;
+		down_read(&current->mm->mmap_sem);
+		vma = current->mm->mmap;
+		while (vma) {
+			vsize += vma->vm_end - vma->vm_start;
+			vma = vma->vm_next;
+		}
+		up_read(&current->mm->mmap_sem);
+	}
+
+	spin_lock(&current->sighand->siglock);
+	pacct->ac_mem = vsize / 1024;
+	spin_unlock(&current->sighand->siglock);
+}
+
+/**
   * acct_process - now just a wrapper around do_acct_process
   * @exitcode: task exit code
   *
   * handles process accounting for an exiting task
   */
diff --git a/kernel/exit.c b/kernel/exit.c
index e06d0c1..54bdbd9 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -893,11 +893,11 @@ fastcall NORET_TYPE void do_exit(long co
  	}
  	group_dead = atomic_dec_and_test(&tsk->signal->live);
  	if (group_dead) {
   		hrtimer_cancel(&tsk->signal->real_timer);
  		exit_itimers(tsk->signal);
-		acct_process(code);
+		acct_collect();
  	}
  	if (unlikely(tsk->robust_list))
  		exit_robust_list(tsk);
  #ifdef CONFIG_COMPAT
  	if (unlikely(tsk->compat_robust_list))
@@ -905,10 +905,12 @@ fastcall NORET_TYPE void do_exit(long co
  #endif
  	if (unlikely(tsk->audit_context))
  		audit_free(tsk);
  	exit_mm(tsk);

+	if (group_dead)
+		acct_process(code);
  	exit_sem(tsk);
  	__exit_files(tsk);
  	__exit_fs(tsk);
  	exit_namespace(tsk);
  	exit_thread();
diff --git a/kernel/fork.c b/kernel/fork.c
index ac8100e..d6c812c 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -869,10 +869,11 @@ static inline int copy_signal(unsigned l
  		 * of the whole CPU time limit.
  		 */
  		tsk->it_prof_expires =
  			secs_to_cputime(sig->rlim[RLIMIT_CPU].rlim_cur);
  	}
+	acct_init_pacct(&sig->pacct);

  	return 0;
  }

  void __cleanup_signal(struct signal_struct *sig)


-- 
Open Source Software Promotion Center, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

  reply	other threads:[~2006-06-22  3:08 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-06-20  6:24 Add pacct_struct to fix some pacct bugs KaiGai Kohei
2006-06-20  6:42 ` Andrew Morton
2006-06-20  7:27   ` KaiGai Kohei
2006-06-22  3:07     ` KaiGai Kohei
2006-06-22  3:08       ` KaiGai Kohei [this message]
2006-06-22  3:09       ` KaiGai Kohei
2006-06-22  3:09       ` KaiGai Kohei
2006-06-22  3:35       ` Andrew Morton
2006-06-22  4:05         ` KaiGai Kohei
2006-06-22  4:14         ` Randy.Dunlap

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=449A09A0.70009@ak.jp.nec.com \
    --to=kaigai@ak.jp.nec.com \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox