lkml.org 
[lkml]   [2005]   [Sep]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2.6.14-rc2] fix incorrect mm->hiwater_vm and mm->hiwater_rss
Jay Lan wrote:
> Hugh Dickins wrote:
>
>> On Wed, 21 Sep 2005, Frank van Maarseveen wrote:
>>
>>> What about calling
>>>
>>> static inline void grow_total_vm(struct mm_struct *mm, unsigned long
>>> increase)
>>> {
>>> mm->total_vm += increase;
>>> if (mm->total_vm > mm->hiwater_vm)
>>> mm->hiwater_vm = mm->total_vm;
>>> }
>>>
>>> whenever total_vm is increased and possibly doing something similar
>>> for rss at
>>> different places? If it is not on the fast path then it's not
>>> necessary to
>>> #ifdef the thing anywhere.
>>
>>
>>
>> I think there's a good argument for separating hiwater_vm and hiwater_rss
>> completely (and you don't seem to be interested in hiwater_rss yourself).
>>
>> hiwater_rss is on some fast paths: well, I don't see them as fast paths
>> myself (the page faults), but they are of exceptional concern to
>> Christoph,
>> and the less we have to mess with struct mm at those points the
>> happier he
>> is. I guess hiwater_rss should remain updated from the timer tick for
>> now.
>>
>> But I think you're right that hiwater_vm is best updated where total_vm
>> is: I'm not sure if it covers all cases completely (I think there's one
>> or two places which don't bother to call __vm_stat_account because they
>> believe it won't change anything), but in principle it would make lots of
>> sense to do it in the __vm_stat_account which typically follows adjusting
>> total_vm, as you did, and if possible nowhere else; rather than adding
>> your inline above.

Just looked at the __vm_stat_account() code. It is enclosed inside
#ifdef CONFIG_PROC_FS.

If that is necessary, i can not put hiwater_vm update code in there. The
system accounting code should not be dependent on a config flag that has
nothing to do with system accounting.

Thanks,
- jay

>
>
> 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?
>
> - jay
>
>
>>
>> Would you be satisfied with that, Christoph?
>>
>> I should warn you that I'll shortly (shortly meaning in days rather
>> than hours) be sending Andrew a patch which will remove the "__" from
>> __vm_stat_account, since the old vm_stat_account is now hardly used.
>> I'm also rearranging the rss,anon_rss accounting. Maybe come back
>> to the hiwaters later on?
>>
>> Hugh
>
>

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

\
 
 \ /
  Last update: 2005-09-27 23:50    [W:0.069 / U:0.408 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site