[PATCH] 2.5.7-pre1 Code cleanup for BSD accounting.

Clean up BSD accounting locking code..

kernel/acct.c

@@ -89,18 +89,12 @@ struct acct_glbs {
 
 static struct acct_glbs acct_globals __cacheline_aligned = {SPIN_LOCK_UNLOCKED};
 
-#define	acct_lock	acct_globals.lock
-#define	acct_active	acct_globals.active
-#define	acct_needcheck	acct_globals.needcheck
-#define	acct_file	acct_globals.file
-#define	acct_timer	acct_globals.timer
-
 /*
  * Called whenever the timer says to check the free space.
  */
 static void acct_timeout(unsigned long unused)
 {
-	acct_needcheck = 1;
+	acct_globals.needcheck = 1;
 }
 
 /*
@@ -112,11 +106,11 @@ static int check_free_space(struct file *file)
 	int res;
 	int act;
 
-	spin_lock(&acct_lock);
-	res = acct_active;
-	if (!file || !acct_needcheck)
+	spin_lock(&acct_globals.lock);
+	res = acct_globals.active;
+	if (!file || !acct_globals.needcheck)
 		goto out;
-	spin_unlock(&acct_lock);
+	spin_unlock(&acct_globals.lock);
 
 	/* May block */
 	if (vfs_statfs(file->f_dentry->d_inode->i_sb, &sbuf))
@@ -130,108 +124,73 @@ static int check_free_space(struct file *file)
 		act = 0;
 
 	/*
-	 * If some joker switched acct_file under us we'ld better be
+	 * If some joker switched acct_globals.file under us we'ld better be
 	 * silent and _not_ touch anything.
 	 */
-	spin_lock(&acct_lock);
-	if (file != acct_file) {
+	spin_lock(&acct_globals.lock);
+	if (file != acct_globals.file) {
 		if (act)
 			res = act>0;
 		goto out;
 	}
 
-	if (acct_active) {
+	if (acct_globals.active) {
 		if (act < 0) {
-			acct_active = 0;
+			acct_globals.active = 0;
 			printk(KERN_INFO "Process accounting paused\n");
 		}
 	} else {
 		if (act > 0) {
-			acct_active = 1;
+			acct_globals.active = 1;
 			printk(KERN_INFO "Process accounting resumed\n");
 		}
 	}
 
-	del_timer(&acct_timer);
-	acct_needcheck = 0;
-	acct_timer.expires = jiffies + ACCT_TIMEOUT*HZ;
-	add_timer(&acct_timer);
-	res = acct_active;
+	del_timer(&acct_globals.timer);
+	acct_globals.needcheck = 0;
+	acct_globals.timer.expires = jiffies + ACCT_TIMEOUT*HZ;
+	add_timer(&acct_globals.timer);
+	res = acct_globals.active;
 out:
-	spin_unlock(&acct_lock);
+	spin_unlock(&acct_globals.lock);
 	return res;
 }
 
 /*
- *  acct_common() is the main routine that implements process accounting.
- *  It takes the name of the file where accounting records should be
- *  written. If the filename is NULL, accounting will be shutdown.
+ * Close the old accouting file (if currently open) and then replace
+ * it with file (if non-NULL).
+ *
+ * NOTE: acct_globals.lock MUST be held on entry and exit.
  */
-long acct_common(const char *name, int locked)
+void acct_file_reopen(struct file *file)
 {
-	struct file *file = NULL, *old_acct = NULL;
-	char *tmp;
-	int error;
-
-	/*
-	 * Should only have locked set when name is NULL (enforce this).
-	 */
-	BUG_ON(locked && name);
-
-	if (!capable(CAP_SYS_PACCT))
-		return -EPERM;
-
-	if (name) {
-		tmp = getname(name);
-		error = PTR_ERR(tmp);
-		if (IS_ERR(tmp))
-			goto out;
-		/* Difference from BSD - they don't do O_APPEND */
-		file = filp_open(tmp, O_WRONLY|O_APPEND, 0);
-		putname(tmp);
-		if (IS_ERR(file)) {
-			error = PTR_ERR(file);
-			goto out;
-		}
-		error = -EACCES;
-		if (!S_ISREG(file->f_dentry->d_inode->i_mode)) 
-			goto out_err;
+	struct file *old_acct = NULL;
 
-		error = -EIO;
-		if (!file->f_op->write) 
-			goto out_err;
-	}
+	BUG_ON(!spin_is_locked(&acct_globals.lock));
 
-	error = 0;
-	if (!locked)
-		spin_lock(&acct_lock);
-	if (acct_file) {
-		old_acct = acct_file;
-		del_timer(&acct_timer);
-		acct_active = 0;
-		acct_needcheck = 0;
-		acct_file = NULL;
+	if (acct_globals.file) {
+		old_acct = acct_globals.file;
+		del_timer(&acct_globals.timer);
+		acct_globals.active = 0;
+		acct_globals.needcheck = 0;
+		acct_globals.file = NULL;
 	}
-	if (name) {
-		acct_file = file;
-		acct_needcheck = 0;
-		acct_active = 1;
+	if (file) {
+		acct_globals.file = file;
+		acct_globals.needcheck = 0;
+		acct_globals.active = 1;
 		/* It's been deleted if it was used before so this is safe */
-		init_timer(&acct_timer);
-		acct_timer.function = acct_timeout;
-		acct_timer.expires = jiffies + ACCT_TIMEOUT*HZ;
-		add_timer(&acct_timer);
+		init_timer(&acct_globals.timer);
+		acct_globals.timer.function = acct_timeout;
+		acct_globals.timer.expires = jiffies + ACCT_TIMEOUT*HZ;
+		add_timer(&acct_globals.timer);
 	}
-	spin_unlock(&acct_lock);
 	if (old_acct) {
-		do_acct_process(0,old_acct);
+		spin_unlock(&acct_globals.lock);
+		do_acct_process(0, old_acct);
 		filp_close(old_acct, NULL);
+		spin_lock(&acct_globals.lock);
 	}
-out:
-	return error;
-out_err:
-	filp_close(file, NULL);
-	goto out;
 }
 
 /*
@@ -242,17 +201,53 @@ long acct_common(const char *name, int locked)
  */
 asmlinkage long sys_acct(const char *name)
 {
-	return (acct_common(name, 0));
+	struct file *file = NULL;
+	char *tmp;
+
+	if (!capable(CAP_SYS_PACCT))
+		return -EPERM;
+
+	if (name) {
+		tmp = getname(name);
+		if (IS_ERR(tmp)) {
+			return (PTR_ERR(tmp));
+		}
+		/* Difference from BSD - they don't do O_APPEND */
+		file = filp_open(tmp, O_WRONLY|O_APPEND, 0);
+		putname(tmp);
+		if (IS_ERR(file)) {
+			return (PTR_ERR(file));
+		}
+		if (!S_ISREG(file->f_dentry->d_inode->i_mode)) {
+			filp_close(file, NULL);
+			return (-EACCES);
+		}
+
+		if (!file->f_op->write) {
+			filp_close(file, NULL);
+			return (-EIO);
+		}
+	}
+
+	spin_lock(&acct_globals.lock);
+	acct_file_reopen(file);
+	spin_unlock(&acct_globals.lock);
+
+	return (0);
 }
 
+/*
+ * If the accouting is turned on for a file in the filesystem pointed
+ * to by sb, turn accouting off.
+ */
 void acct_auto_close(struct super_block *sb)
 {
-	spin_lock(&acct_lock);
-	if (acct_file && acct_file->f_dentry->d_inode->i_sb == sb) {
-		(void) acct_common(NULL, 1);
-	} else {
-		spin_unlock(&acct_lock);
+	spin_lock(&acct_globals.lock);
+	if (acct_globals.file &&
+	    acct_globals.file->f_dentry->d_inode->i_sb == sb) {
+		acct_file_reopen((struct file *)NULL);
 	}
+	spin_unlock(&acct_globals.lock);
 }
 
 /*
@@ -329,7 +324,8 @@ static void do_acct_process(long exitcode, struct file *file)
 	strncpy(ac.ac_comm, current->comm, ACCT_COMM);
 	ac.ac_comm[ACCT_COMM - 1] = '\0';
 
-	ac.ac_btime = CT_TO_SECS(current->start_time) + (xtime.tv_sec - (jiffies / HZ));
+	ac.ac_btime = CT_TO_SECS(current->start_time) +
+		(xtime.tv_sec - (jiffies / HZ));
 	ac.ac_etime = encode_comp_t(jiffies - current->start_time);
 	ac.ac_utime = encode_comp_t(current->times.tms_utime);
 	ac.ac_stime = encode_comp_t(current->times.tms_stime);
@@ -390,15 +386,15 @@ static void do_acct_process(long exitcode, struct file *file)
 int acct_process(long exitcode)
 {
 	struct file *file = NULL;
-	spin_lock(&acct_lock);
-	if (acct_file) {
-		file = acct_file;
+	spin_lock(&acct_globals.lock);
+	if (acct_globals.file) {
+		file = acct_globals.file;
 		get_file(file);
-		spin_unlock(&acct_lock);
+		spin_unlock(&acct_globals.lock);
 		do_acct_process(exitcode, file);
 		fput(file);
 	} else
-		spin_unlock(&acct_lock);
+		spin_unlock(&acct_globals.lock);
 	return 0;
 }