Messages in this thread Patch in this message | | | Date | Wed, 28 Sep 2005 10:31:05 -0700 | From | Jay Lan <> | Subject | Re: [PATCH 2.6.14-rc2] fix incorrect mm->hiwater_vm and mm->hiwater_rss |
| |
Hugh Dickins wrote: >On Tue, 27 Sep 2005, Jay Lan wrote: > >>While in the work on separating hiwater_vm from hiwater_rss, i noticed >>that __vm_stat_account() was not called in these functions where >>total_vm was updated: >>mm/mmap.c do_brk >>mm/nommu.c do_mmap_pgoff >>mm/nommu.c do_munmap >>arch/ppc64/kernel/vdso.c arch_setup_additional_pages >>arch/x86_64/ia32/syscall32.c syscall32_setup_pages >> >>Frank tried to touch the latter two in his proposed patch. >>Does it make sense we add __vm_stat_account() calls to the above >>routines? >> > >Probably not. Partly because of the PROCFS issue you noticed after. >And partly because that's a long list, whereas the evidence is that >__vm_stat_account is nowadays being called in the places that need it. > >What, you couldn't find the call to __vm_stat_account next to >updating total_vm in do_mmap_pgoff :-? And I don't see that total_vm >is updated in do_munmap, that and the vm_stat_account have to be done >at a lower, per-vma level. Haven't looked at the others yet. >
There are two do_mmap_pgoff, one in mm/mmap.c and the other in mm/nommu.c. It was the mm/nommu.c version that does not have a call to __vm_stat_account.
I am not concerned about do_munmap any more from updating hiwater_vm's concern since the total_vm gets decreased, not increased.
>I think, better leave hiwater_vm to me for now. Since I've got that > Sounds good to me.
I agreed with you (and Frank) that it makes sense to split hiwater_vm and hiwater_rss update. >patchset in the next -mm, it's maybe easier for me to put something >together on top of that. And it seems quite possible (haven't checked >yet) that actually we should be moving the update of total_vm in to the >core from out all over, rather than splattering hiwater_vm updates about > >I'll take a look in the next couple of days. > >Hugh >
In case you are going to turn hiwater_vm update into a macro and hide it in a config flag, here is what i have that might be useful.
Thanks, - jay
Index: linux/init/Kconfig =================================================================== --- linux.orig/init/Kconfig 2005-09-22 14:14:54.105262867 -0700 +++ linux/init/Kconfig 2005-09-22 14:57:19.326784560 -0700 @@ -162,6 +162,19 @@ config BSD_PROCESS_ACCT_V3 for processing it. A preliminary version of these tools is available at <http://www.physik3.uni-rostock.de/tim/kernel/utils/acct/>. +config ENHANCED_SYS_ACCT + bool "Enhanced System Accounting support" + depends on BSD_PROCESS_ACCT + default n + help + If you say Y here, some extra system accounting data will be + collected and updated to task struct as well as task->mm struct. + These information are used and presented in enhanced system + accounting packages such as Comprehensive System Accounting (CSA). + This activity may force cachelines into shared from exclusive + on a processor. The CSA project can be viewed at + <http://oss.sgi.com/projects/csa/>. + config SYSCTL bool "Sysctl support" ---help--- Index: linux/mm/memory.c =================================================================== --- linux.orig/mm/memory.c 2005-09-22 14:15:07.465757425 -0700 +++ linux/mm/memory.c 2005-09-27 16:55:28.438702812 -0700 @@ -2206,21 +2206,21 @@ unsigned long vmalloc_to_pfn(void * vmal EXPORT_SYMBOL(vmalloc_to_pfn); +#ifdef CONFIG_ENHANCED_SYS_ACCT /* - * update_mem_hiwater - * - update per process rss and vm high water data + * update_rss_hiwater + * - update per process rss high water data */ -void update_mem_hiwater(struct task_struct *tsk) +void update_rss_hiwater(struct mm_struct *mm) { - if (tsk->mm) { - unsigned long rss = get_mm_counter(tsk->mm, rss); + if (likely(mm)) { + unsigned long rss = get_mm_counter(mm, rss); - if (tsk->mm->hiwater_rss < rss) - tsk->mm->hiwater_rss = rss; - if (tsk->mm->hiwater_vm < tsk->mm->total_vm) - tsk->mm->hiwater_vm = tsk->mm->total_vm; + if (mm->hiwater_rss < rss) + mm->hiwater_rss = rss; } } +#endif #if !defined(__HAVE_ARCH_GATE_AREA) Index: linux/include/linux/mm.h =================================================================== --- linux.orig/include/linux/mm.h 2005-08-28 16:41:01.000000000 -0700 +++ linux/include/linux/mm.h 2005-09-27 17:33:49.372846334 -0700 @@ -949,7 +949,18 @@ static inline void vm_stat_unaccount(str } /* update per process rss and vm hiwater data */ -extern void update_mem_hiwater(struct task_struct *tsk); +#ifdef CONFIG_ENHANCED_SYS_ACCT +static inline void update_hiwater_vm(struct mm_struct *mm) +{ + if (mm->hiwater_vm < mm->total_vm) + mm->hiwater_vm = mm->total_vm; +} + +extern void update_hiwater_rss(struct mm_struct *mm); +#else +#define update_hiwater_vm(x) do { } while (0) +#define update_hiwater_rss(x) do { } while (0) +#endif #ifndef CONFIG_DEBUG_PAGEALLOC static inline void - 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/
| |