[Devel] [PATCH 2/3] mm: vmscan: get rid of DEFAULT_SEEKS and document shrink_slab logic

[Andrew Morton]

On Wed, 5 Feb 2014 11:16:49 +0400 Vladimir Davydov <vdavydov at parallels.com> wrote:

> > So why did I originally make DEFAULT_SEEKS=2?  Because I figured that to
> > recreate (say) an inode would require a seek to the inode data then a
> > seek back.  Is it legitimate to include the
> > seek-back-to-what-you-were-doing-before seek in the cost of an inode
> > reclaim?  I guess so...
> 
> Hmm, that explains this 2. Since we typically don't need to "seek back"
> when recreating a cache page, as they are usually read in bunches by
> readahead, the number of seeks to bring back a user page is 1, while the
> number of seeks to recreate an average inode is 2, right?

Sounds right to me.

> Then to scan inodes and user pages so that they would generate
> approximately the same number of seeks, we should calculate the number
> of objects to scan as follows:
> 
> nr_objects_to_scan = nr_pages_scanned / lru_pages *
>                                         nr_freeable_objects /
> shrinker->seeks
> 
> where shrinker->seeks = DEFAULT_SEEKS = 2 for inodes.

hm, I wonder if we should take the size of the object into account. 
Should we be maximizing (memory-reclaimed / seeks-to-reestablish-it).

> But currently we
> have four times that. I can explain why we should multiply this by 2 -
> we do not count pages moving from active to inactive lrus in
> nr_pages_scanned, and 2*nr_pages_scanned can be a good approximation for
> that - but I have no idea why we multiply it by 4...

I don't understand this code at all:

	total_scan = nr;
	delta = (4 * nr_pages_scanned) / shrinker->seeks;
	delta *= freeable;
	do_div(delta, lru_pages + 1);
	total_scan += delta;

If it actually makes any sense, it sorely sorely needs documentation.

David, you touched it last.  Any hints?