enhanced accounting data collection

PATCH In review Jay Lan 2 participants 8 messages Oct 22, 2004 – Nov 5, 2004

Subsystems (8)

Memory management VFS MM core Scheduler +4 more subsystems

lore

Share & tools

lore

Jay Lan

author

2004-10-22 01:36 UTC #0 lore raw

cover enhanced accounting data collection

These two patches are the one we submitted to SuSE for Sles9 SP1. They are clean of CSA specific code.

In earlier round of discussion, all partipants favored a common layer of accounting data collection. I believe these two patches are the super set that meets the needs of people who need enhanced BSD accounting.

This patchset consists of two parts: acct_io and acct_mm, as we identified improved data collection in the area of IO and MM are useful to our customers.

It is intended to offer common data collection method for various accounting packages including BSD accouting, ELSA, CSA, and any other acct packages that favor a common layer of data collection.

'acct_mm' defines a few macros that are no-op unless CONFIG_BSD_PROCESS_ACCT config flag is set on.

Andrew, please consider including these two patches. Please let me know how i can help!

Best Regards,
---
Jay Lan - Linux System Software
Silicon Graphics Inc., Mountain View, CA

Jay Lan

author

2004-10-22 01:39 UTC #1 lore raw

PATCH 1/2 enhanced accounting data collection

1/2: acct_io

Enahanced I/O accounting data collection.

Signed-off-by Jay Lan <[email protected]>

fs/read_write.c +23 -2

Index: linux/fs/read_write.c
===================================================================
@@ -216,8 +216,11 @@
 				ret = file->f_op->read(file, buf, count, pos);
 			else
 				ret = do_sync_read(file, buf, count, pos);
-			if (ret > 0)
+			if (ret > 0) {
 				dnotify_parent(file->f_dentry, DN_ACCESS);
+				current->rchar += ret;
+			}
+			current->syscr++;
 		}
 	}

@@ -260,8 +263,11 @@
 				ret = file->f_op->write(file, buf, count, pos);
 			else
 				ret = do_sync_write(file, buf, count, pos);
-			if (ret > 0)
+			if (ret > 0) {
 				dnotify_parent(file->f_dentry, DN_MODIFY);
+				current->wchar += ret;
+			}
+			current->syscw++;
 		}
 	}

@@ -540,6 +546,10 @@
 		fput_light(file, fput_needed);
 	}
 
+	if (ret > 0) {
+		current->rchar += ret;
+	}
+	current->syscr++;
 	return ret;
 }

@@ -558,6 +568,10 @@
 		fput_light(file, fput_needed);
 	}
 
+	if (ret > 0) {
+		current->wchar += ret;
+	}
+	current->syscw++;
 	return ret;
 }

@@ -636,6 +650,13 @@
 
 	retval = in_file->f_op->sendfile(in_file, ppos, count, file_send_actor, out_file);
 
+	if (retval > 0) {
+		current->rchar += retval;
+		current->wchar += retval;
+	}
+	current->syscr++;
+	current->syscw++;
+
 	if (*ppos > max)
 		retval = -EOVERFLOW;

include/linux/sched.h +3 -0

Index: linux/include/linux/sched.h
===================================================================
@@ -591,6 +591,9 @@
 	struct rw_semaphore pagg_sem;
 #endif
 
+/* i/o counters(bytes read/written, #syscalls */
+	unsigned long rchar, wchar, syscr, syscw;
+
 };
 
 static inline pid_t process_group(struct task_struct *tsk)

kernel/fork.c +1 -0

Index: linux/kernel/fork.c
===================================================================
@@ -995,6 +995,7 @@
 	p->real_timer.data = (unsigned long) p;
 
 	p->utime = p->stime = 0;
+	p->rchar = p->wchar = p->syscr = p->syscw = 0;
 	p->lock_depth = -1;		/* -1 = no lock */
 	p->start_time = get_jiffies_64();
 	p->security = NULL;

Jay Lan

author

2004-10-22 02:06 UTC #4 lore raw

PATCH 2/2 enhanced accounting data collection

In reply to Jay Lan

2/2: acct_mm

Enhanced MM accounting data collection.

Signed-off-by Jay Lan <[email protected]>

fs/exec.c +3 -0

Index: linux/fs/exec.c
===================================================================
@@ -47,6 +47,7 @@
 #include <linux/syscalls.h>
 #include <linux/rmap.h>
 #include <linux/pagg.h>
+#include <linux/acct.h>
 
 #include <asm/uaccess.h>
 #include <asm/mmu_context.h>

@@ -1163,6 +1164,8 @@
 
 		/* execve success */
 		security_bprm_free(bprm);
+		acct_update_integrals();
+		update_mem_hiwater();
 		kfree(bprm);
 		return retval;
 	}

include/linux/sched.h +19 -0

Index: linux/include/linux/sched.h
===================================================================
@@ -249,6 +249,8 @@
 	struct kioctx		*ioctx_list;
 
 	struct kioctx		default_kioctx;
+
+	unsigned long hiwater_rss, hiwater_vm;
 };
 
 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
 
 };

@@ -817,6 +823,19 @@
 /* Remove the current tasks stale references to the old mm_struct */
 extern void mm_release(struct task_struct *, struct mm_struct *);
 
+/* 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;
+		}
+	}
+}
+
 extern int  copy_thread(int, unsigned long, unsigned long, unsigned long, struct task_struct *, struct pt_regs *);
 extern void flush_thread(void);
 extern void exit_thread(void);

kernel/exit.c +2 -0

Index: linux/kernel/exit.c
===================================================================
@@ -808,6 +808,8 @@
 		ptrace_notify((PTRACE_EVENT_EXIT << 8) | SIGTRAP);
 	}
 
+	acct_update_integrals();
+	update_mem_hiwater();
 	acct_process(code);
 	__exit_mm(tsk);

kernel/fork.c +6 -1

Index: linux/kernel/fork.c
===================================================================
@@ -39,7 +39,7 @@
 #include <linux/profile.h>
 #include <linux/rmap.h>
 #include <linux/pagg.h>
-
+#include <linux/acct.h>
 #include <asm/pgtable.h>
 #include <asm/pgalloc.h>
 #include <asm/uaccess.h>

@@ -609,6 +609,9 @@
 	if (retval)
 		goto free_pt;
 
+	mm->hiwater_rss = mm->rss;
+	mm->hiwater_vm = mm->total_vm;	
+
 good_mm:
 	tsk->mm = mm;
 	tsk->active_mm = mm;

@@ -996,6 +999,8 @@
 
 	p->utime = p->stime = 0;
 	p->rchar = p->wchar = p->syscr = p->syscw = 0;
+	acct_clear_integrals(p);
+
 	p->lock_depth = -1;		/* -1 = no lock */
 	p->start_time = get_jiffies_64();
 	p->security = NULL;

mm/memory.c +14 -2

Index: linux/mm/memory.c
===================================================================
@@ -44,6 +44,7 @@
 #include <linux/highmem.h>
 #include <linux/pagemap.h>
 #include <linux/rmap.h>
+#include <linux/acct.h>
 #include <linux/module.h>
 #include <linux/init.h>

@@ -605,6 +606,7 @@
 	tlb = tlb_gather_mmu(mm, 0);
 	unmap_vmas(&tlb, mm, vma, address, end, &nr_accounted, details);
 	tlb_finish_mmu(tlb, address, end);
+	acct_update_integrals();
 	spin_unlock(&mm->page_table_lock);
 }

@@ -1095,9 +1097,11 @@
 	spin_lock(&mm->page_table_lock);
 	page_table = pte_offset_map(pmd, address);
 	if (likely(pte_same(*page_table, pte))) {
-		if (PageReserved(old_page))
+		if (PageReserved(old_page)) {
 			++mm->rss;
-		else
+			acct_update_integrals();
+			update_mem_hiwater();
+		} else
 			page_remove_rmap(old_page);
 		break_cow(vma, new_page, address, page_table);
 		lru_cache_add_active(new_page);

@@ -1379,6 +1383,9 @@
 		remove_exclusive_swap_page(page);
 
 	mm->rss++;
+	acct_update_integrals();
+	update_mem_hiwater();
+
 	pte = mk_pte(page, vma->vm_page_prot);
 	if (write_access && can_share_swap_page(page)) {
 		pte = maybe_mkwrite(pte_mkdirty(pte), vma);

@@ -1444,6 +1451,8 @@
 			goto out;
 		}
 		mm->rss++;
+		acct_update_integrals();
+		update_mem_hiwater();
 		entry = maybe_mkwrite(pte_mkdirty(mk_pte(page,
 							 vma->vm_page_prot)),
 				      vma);

@@ -1553,6 +1562,9 @@
 	if (pte_none(*page_table)) {
 		if (!PageReserved(new_page))
 			++mm->rss;
+		acct_update_integrals();
+		update_mem_hiwater();
+
 		flush_icache_page(vma, new_page);
 		entry = mk_pte(new_page, vma->vm_page_prot);
 		if (write_access)

mm/mmap.c +7 -0

Index: linux/mm/mmap.c
===================================================================
@@ -20,6 +20,7 @@
 #include <linux/hugetlb.h>
 #include <linux/profile.h>
 #include <linux/module.h>
+#include <linux/acct.h>
 #include <linux/mount.h>
 #include <linux/mempolicy.h>
 #include <linux/rmap.h>

@@ -1014,6 +1015,8 @@
 		down_write(&mm->mmap_sem);
 	}
 	__vm_stat_account(mm, vm_flags, file, len >> PAGE_SHIFT);
+	acct_update_integrals();
+	update_mem_hiwater();
 	return addr;
 
 unmap_and_free_vma:

@@ -1360,6 +1363,8 @@
 	if (vma->vm_flags & VM_LOCKED)
 		vma->vm_mm->locked_vm += grow;
 	__vm_stat_account(vma->vm_mm, vma->vm_flags, vma->vm_file, grow);
+	acct_update_integrals();
+	update_mem_hiwater();
 	anon_vma_unlock(vma);
 	return 0;
 }

@@ -1816,6 +1821,8 @@
 		mm->locked_vm += len >> PAGE_SHIFT;
 		make_pages_present(addr, addr + len);
 	}
+	acct_update_integrals();
+	update_mem_hiwater();
 	return addr;
 }

mm/mremap.c +6 -0

Index: linux/mm/mremap.c
===================================================================
@@ -16,6 +16,7 @@
 #include <linux/fs.h>
 #include <linux/highmem.h>
 #include <linux/security.h>
+#include <linux/acct.h>
 
 #include <asm/uaccess.h>
 #include <asm/cacheflush.h>

@@ -232,6 +233,9 @@
 					   new_addr + new_len);
 	}
 
+	acct_update_integrals();
+	update_mem_hiwater();
+
 	return new_addr;
 }

@@ -368,6 +372,8 @@
 				make_pages_present(addr + old_len,
 						   addr + new_len);
 			}
+			acct_update_integrals();
+			update_mem_hiwater();
 			ret = addr;
 			goto out;
 		}

mm/rmap.c +3 -0

Index: linux/mm/rmap.c
===================================================================
@@ -50,6 +50,7 @@
 #include <linux/swapops.h>
 #include <linux/slab.h>
 #include <linux/init.h>
+#include <linux/acct.h>
 #include <linux/rmap.h>
 #include <linux/rcupdate.h>

@@ -581,6 +582,7 @@
 	}
 
 	mm->rss--;
+	acct_update_integrals();
 	page_remove_rmap(page);
 	page_cache_release(page);

@@ -680,6 +682,7 @@
 
 		page_remove_rmap(page);
 		page_cache_release(page);
+		acct_update_integrals();
 		mm->rss--;
 		(*mapcount)--;
 	}

mm/swapfile.c +3 -0

Index: linux/mm/swapfile.c
===================================================================
@@ -24,6 +24,7 @@
 #include <linux/module.h>
 #include <linux/rmap.h>
 #include <linux/security.h>
+#include <linux/acct.h>
 #include <linux/backing-dev.h>
 
 #include <asm/pgtable.h>

@@ -435,6 +436,8 @@
 	set_pte(dir, pte_mkold(mk_pte(page, vma->vm_page_prot)));
 	page_add_anon_rmap(page, vma, address);
 	swap_free(entry);
+	acct_update_integrals();
+	update_mem_hiwater();
 }
 
 /* vma->vm_mm->page_table_lock is held */

include/linux/acct.h +25 -0

Index: linux/include/linux/acct.h
===================================================================
@@ -120,9 +120,34 @@
 struct super_block;
 extern void acct_auto_close(struct super_block *sb);
 extern void acct_process(long exitcode);
+
+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;
+	}
+}
+
+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;
+	}
+}
+
 #else
 #define acct_auto_close(x)	do { } while (0)
 #define acct_process(x)		do { } while (0)
+
+#define acct_update_integrals()		do { } while (0)
+#define acct_clear_integrals(task)	do { } while (0)
 #endif
 
 /*

Andrew Morton

2004-10-22 02:20 UTC #2 lore raw

In reply to Jay Lan

Quoted 5 lines · Jay Lan

1/2: acct_io
Enahanced I/O accounting data collection.

It's nice to use `diff -p' so people can see what functions you're hitting.

Review

Quoted 1 line

(quoted diff) +1 -0
+			current->syscr++;

Should these metrics be per-thread or per-heavyweight process?

fs/read_write.c line 222

Quoted 3 lines

fs/read_write.c +3 -0

+	if (ret > 0) {
+		current->rchar += ret;
+	}

It's conventional to omit the braces if there is only one statement in the block.

include/linux/sched.h line 591

Quoted 5 lines

===================================================================
include/linux/sched.h +0 -0
@@ -591,6 +591,9 @@
 	struct rw_semaphore pagg_sem;

There is no `pagg_sem' in the kernel, so this will spit a reject.

include/linux/sched.h line 596

Quoted 5 lines

include/linux/sched.h +3 -0

 #endif
 
+/* i/o counters(bytes read/written, #syscalls */
+	unsigned long rchar, wchar, syscr, syscw;
+

These will overflow super-quick. Shouldn't they be 64-bit?

kernel/fork.c line 998

Quoted 7 lines

kernel/fork.c +1 -0

@@ -995,6 +995,7 @@
 	p->real_timer.data = (unsigned long) p;
 
 	p->utime = p->stime = 0;
+	p->rchar = p->wchar = p->syscr = p->syscw = 0;

We generally prefer

	p->rchar = 0;
	p->wchar = 0;
	etc.

yes, the code which is there has already sinned - feel free to clean it up while you're there ;)

Andrew Morton

2004-10-22 02:29 UTC #5 lore raw

In reply to Jay Lan

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

Quoted 11 lines · Jay Lan

2/2: acct_mm
Enhanced MM accounting data collection.
Index: linux/include/linux/sched.h
===================================================================

include/linux/sched.h line 254

Quoted 6 lines

include/linux/sched.h +2 -0

@@ -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 */

include/linux/sched.h line 601

Quoted 10 lines

include/linux/sched.h +4 -0

 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.

include/linux/sched.h line 837

Quoted 12 lines

include/linux/sched.h +12 -0

+/* 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.

include/linux/sched.h

Quoted 12 lines

include/linux/sched.h +12 -0

+
+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.

include/linux/sched.h

Quoted 8 lines

include/linux/sched.h +8 -0

+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'?

Andrew Morton

2004-10-22 02:29 UTC #7 lore raw

In reply to Jay Lan

Quoted 2 lines · Jay Lan

These two patches are the one we submitted to SuSE for Sles9 SP1.

Did suse accept them?

Quoted 6 lines

They are clean of CSA specific code.
In earlier round of discussion, all partipants favored a common layer of accounting data collection. I believe these two patches are the super set that meets the needs of people who need enhanced BSD accounting.

OK, well I shall assume that all participants are reading lse-tech, or that readers of lse-tech will pass on the appropriate heads-up. Because if I don't hear from anyone, this all goes in.

Quoted 8 lines

This patchset consists of two parts: acct_io and acct_mm, as we identified improved data collection in the area of IO and MM are useful to our customers.
It is intended to offer common data collection method for various accounting packages including BSD accouting, ELSA, CSA, and any other acct packages that favor a common layer of data collection.

Quoted 5 lines

'acct_mm' defines a few macros that are no-op unless CONFIG_BSD_PROCESS_ACCT config flag is set on.
Andrew, please consider including these two patches. Please let me know how i can help!

These patches are really, really badly documented. This is by no means unusual, but I labour on. A nice description of what you set out to do and how you did it would be a nice thing to present.

I have a few comments on the present implementation and shall follow up to those patches.

Jay Lan

author

2004-11-04 23:49 UTC #3 lore raw

In reply to Andrew Morton

Quoted 9 lines · Andrew Morton

Quoted 4 lines · Jay Lan
1/2: acct_io
Enahanced I/O accounting data collection.
It's nice to use `diff -p' so people can see what functions you're hitting.

Will be done that way next time.

Quoted 6 lines

Review
Quoted 3 lines
(quoted diff) +1 -0
+			current->syscr++;
Should these metrics be per-thread or per-heavyweight process?

Our customers found it useful per process.

Quoted 9 lines

fs/read_write.c line 222
Quoted 5 lines
fs/read_write.c +3 -0
+	if (ret > 0) {
+		current->rchar += ret;
+	}
It's conventional to omit the braces if there is only one statement in the block.

Fixed. Also a few other places in the same siutation.

Quoted 10 lines

include/linux/sched.h line 591
Quoted 7 lines
===================================================================
include/linux/sched.h +0 -0
@@ -591,6 +591,9 @@
 	struct rw_semaphore pagg_sem;
There is no `pagg_sem' in the kernel, so this will spit a reject.

Fixed. That was from pagg, but i should not assume that.

Quoted 10 lines

include/linux/sched.h line 596
Quoted 7 lines
include/linux/sched.h +3 -0
 #endif
 
+/* i/o counters(bytes read/written, #syscalls */
+	unsigned long rchar, wchar, syscr, syscw;
+
These will overflow super-quick. Shouldn't they be 64-bit?

You are correct. Thanks!

Quoted 13 lines

kernel/fork.c line 998

Quoted 9 lines

kernel/fork.c +1 -0

@@ -995,6 +995,7 @@
 	p->real_timer.data = (unsigned long) p;
 
 	p->utime = p->stime = 0;
+	p->rchar = p->wchar = p->syscr = p->syscw = 0;

We generally prefer

Quoted 6 lines

	p->rchar = 0;
	p->wchar = 0;
	etc.
yes, the code which is there has already sinned - feel free to clean it up while you're there ;)

Will be corrected.

Thanks,
- jay

Quoted 12 lines

-------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/lse-tech

Jay Lan

author

2004-11-05 00:07 UTC #6 lore raw

In reply to Andrew Morton

Quoted 6 lines · Andrew Morton

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 :)

Quoted 16 lines

Quoted 13 lines · Jay Lan
2/2: acct_mm
Enhanced MM accounting data collection.
Index: linux/include/linux/sched.h
===================================================================
include/linux/sched.h +0 -0
@@ -249,6 +249,8 @@
 	struct kioctx		*ioctx_list;
 

Quoted 8 lines

include/linux/sched.h line 254
Quoted 4 lines
include/linux/sched.h +2 -0
 	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.

Quoted 16 lines

include/linux/sched.h line 601
Quoted 12 lines
include/linux/sched.h +4 -0
 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.

Quoted 2 lines

It's not clear to me what, semantically, these fields represent. That's something which is appropriate for the supporting changelog entry.

Certainly!

Quoted 39 lines

Quoted 4 lines
include/linux/sched.h +2 -0
+/* Update highwater values */
+static inline void update_mem_hiwater(void)
include/linux/sched.h line 837
Quoted 10 lines
include/linux/sched.h +10 -0
+{
+	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.
include/linux/sched.h
Quoted 12 lines
include/linux/sched.h +12 -0
+
+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.

Quoted 1 line

This function is too big to inline.

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

Quoted 13 lines

include/linux/sched.h
Quoted 10 lines
include/linux/sched.h +8 -0
+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

Quoted 12 lines

-------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/lse-tech