All of lore.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:09:22 +0900	[thread overview]
Message-ID: <449A09E2.1030607@ak.jp.nec.com> (raw)
In-Reply-To: <449A0967.2020701@ak.jp.nec.com>

     [PACCT] avoidance to refer the last thread as a representation of the process

     When pacct facility generate an 'ac_flag' field in accounting record,
     it refers a task_struct of the thread which died last in the process.
     But any other task_structs are ignored.

     Therefore, pacct facility drops ASU flag even if root-privilege
     operations are used by any other threads except the last one.
     In addition, AFORK flag is always set when the thread of group-leader
     didn't die last, although this process has called execve() after fork().

     We have a same matter in ac_exitcode. The recorded ac_exitcode is
     an exit code of the last thread in the process. There is a possibility
     this exitcode is not the group leader's one.

     ---- in kernel/acct.c : do_acct_process() ----
             ac.ac_flag = 0;
             if (current->flags & PF_FORKNOEXEC)
                     ac.ac_flag |= AFORK;
             if (current->flags & PF_SUPERPRIV)
                     ac.ac_flag |= ASU;
             if (current->flags & PF_DUMPCORE)
                     ac.ac_flag |= ACORE;
             if (current->flags & PF_SIGNALED)
                     ac.ac_flag |= AXSIG;
                       :
                    - snip -
                       :
             ac.ac_exitcode = exitcode;
     ----------------------------------------------

     This patch fixes those matters.
     - The exit code of group leader is recorded as ac_exitcode.
     - ASU, ACORE, AXSIG flag are marked if any task_struct satisfy
       the conditions.
     - AFORK flag is marked if only group leader thread satisfy
       the condition.

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

diff --git a/include/linux/acct.h b/include/linux/acct.h
index 6753687..879d441 100644
--- a/include/linux/acct.h
+++ b/include/linux/acct.h
@@ -122,20 +122,20 @@ struct acct_v3
  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_collect(long exitcode, int group_dead);
+extern void acct_process(void);
  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_collect(x,y)	do { } while (0)
+#define acct_process()		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 918fdda..6905ac0 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -357,10 +357,12 @@ struct sighand_struct {
  	struct k_sigaction	action[_NSIG];
  	spinlock_t		siglock;
  };

  struct pacct_struct {
+	int			ac_flag;
+	long			ac_exitcode;
  	unsigned long		ac_mem;
  };

  /*
   * NOTE! "signal_struct" does not have it's own
diff --git a/kernel/acct.c b/kernel/acct.c
index f1a4e12..7111fe7 100644
--- a/kernel/acct.c
+++ b/kernel/acct.c
@@ -73,11 +73,11 @@ int acct_parm[3] = {4, 2, 30};
  #define ACCT_TIMEOUT	(acct_parm[2])	/* foo second timeout between checks */

  /*
   * External references and all of the globals.
   */
-static void do_acct_process(long, struct file *);
+static void do_acct_process(struct file *);

  /*
   * This structure is used so that all the data protected by lock
   * can be placed in the same cache line as the lock.  This primes
   * the cache line to have the data after getting the lock.
@@ -194,11 +194,11 @@ static void acct_file_reopen(struct file
  		add_timer(&acct_globals.timer);
  	}
  	if (old_acct) {
  		mnt_unpin(old_acct->f_vfsmnt);
  		spin_unlock(&acct_globals.lock);
-		do_acct_process(0, old_acct);
+		do_acct_process(old_acct);
  		filp_close(old_acct, NULL);
  		spin_lock(&acct_globals.lock);
  	}
  }

@@ -417,11 +417,11 @@ 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)
+static void do_acct_process(struct file *file)
  {
  	struct pacct_struct *pacct = &current->signal->pacct;
  	acct_t ac;
  	mm_segment_t fs;
  	unsigned long flim;
@@ -494,30 +494,22 @@ static void do_acct_process(long exitcod
  	read_lock(&tasklist_lock);	/* pin current->signal */
  	ac.ac_tty = current->signal->tty ?
  		old_encode_dev(tty_devnum(current->signal->tty)) : 0;
  	read_unlock(&tasklist_lock);

-	ac.ac_flag = 0;
-	if (current->flags & PF_FORKNOEXEC)
-		ac.ac_flag |= AFORK;
-	if (current->flags & PF_SUPERPRIV)
-		ac.ac_flag |= ASU;
-	if (current->flags & PF_DUMPCORE)
-		ac.ac_flag |= ACORE;
-	if (current->flags & PF_SIGNALED)
-		ac.ac_flag |= AXSIG;
  	spin_lock(&current->sighand->siglock);
+	ac.ac_flag = pacct->ac_flag;
  	ac.ac_mem = encode_comp_t(pacct->ac_mem);
+	ac.ac_exitcode = pacct->ac_exitcode;
  	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 +
  				     current->maj_flt);
  	ac.ac_swaps = encode_comp_t(0);
-	ac.ac_exitcode = exitcode;

  	/*
           * Kernel segment override to datasegment and write it
           * to the accounting file.
           */
@@ -542,17 +534,19 @@ void acct_init_pacct(struct pacct_struct
  	memset(pacct, 0, sizeof(struct pacct_struct));
  }

  /**
   * acct_collect - collect accounting information into pacct_struct
+ * @exitcode: task exit code
+ * @group_dead: not 0, if this thread is the last one in the process.
   */
-void acct_collect(void)
+void acct_collect(long exitcode, int group_dead)
  {
  	struct pacct_struct *pacct = &current->signal->pacct;
  	unsigned long vsize = 0;

-	if (current->mm) {
+	if (group_dead && 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;
@@ -560,21 +554,33 @@ void acct_collect(void)
  		}
  		up_read(&current->mm->mmap_sem);
  	}

  	spin_lock(&current->sighand->siglock);
-	pacct->ac_mem = vsize / 1024;
+	if (group_dead)
+		pacct->ac_mem = vsize / 1024;
+	if (thread_group_leader(current)) {
+		pacct->ac_exitcode = exitcode;
+		if (current->flags & PF_FORKNOEXEC)
+			pacct->ac_flag |= AFORK;
+	}
+	if (current->flags & PF_SUPERPRIV)
+		pacct->ac_flag |= ASU;
+	if (current->flags & PF_DUMPCORE)
+		pacct->ac_flag |= ACORE;
+	if (current->flags & PF_SIGNALED)
+		pacct->ac_flag |= AXSIG;
  	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
   */
-void acct_process(long exitcode)
+void acct_process()
  {
  	struct file *file = NULL;

  	/*
  	 * accelerate the common fastpath:
@@ -589,11 +595,11 @@ void acct_process(long exitcode)
  		return;
  	}
  	get_file(file);
  	spin_unlock(&acct_globals.lock);

-	do_acct_process(exitcode, file);
+	do_acct_process(file);
  	fput(file);
  }


  /**
diff --git a/kernel/exit.c b/kernel/exit.c
index 54bdbd9..9d395c7 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -893,12 +893,12 @@ 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_collect();
  	}
+	acct_collect(code, group_dead);
  	if (unlikely(tsk->robust_list))
  		exit_robust_list(tsk);
  #ifdef CONFIG_COMPAT
  	if (unlikely(tsk->compat_robust_list))
  		compat_exit_robust_list(tsk);
@@ -906,11 +906,11 @@ fastcall NORET_TYPE void do_exit(long co
  	if (unlikely(tsk->audit_context))
  		audit_free(tsk);
  	exit_mm(tsk);

  	if (group_dead)
-		acct_process(code);
+		acct_process();
  	exit_sem(tsk);
  	__exit_files(tsk);
  	__exit_fs(tsk);
  	exit_namespace(tsk);
  	exit_thread();

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

  parent reply	other threads:[~2006-06-22  3:09 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
2006-06-22  3:09       ` KaiGai Kohei [this message]
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=449A09E2.1030607@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.