Re: [PATCH 2.6.14-rc2] fix incorrect mm->hiwater_vm and mm->hiwater_rss

[Hugh Dickins <hugh@veritas.com>]

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.

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