[Intel-gfx] [PATCH] drm/i915: Revert shrinker changes from "Track unbound pages"

[Daniel Vetter]

On Thu, Jan 10, 2013 at 1:23 AM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> On Wed,  9 Jan 2013 18:57:09 +0100, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
>> This partially reverts
>>
>> commit 6c085a728cf000ac1865d66f8c9b52935558b328
>> Author: Chris Wilson <chris at chris-wilson.co.uk>
>> Date:   Mon Aug 20 11:40:46 2012 +0200
>>
>>     drm/i915: Track unbound pages
>>
>> Closer inspection of that patch revealed a bunch of unrelated changes
>> in the shrinker:
>> - The shrinker count is now in pages instead of objects.
>> - For counting the shrinkable objects the old code only looked at the
>>   inactive list, the new code looks at all bounds objects (including
>>   pinned ones). That is obviously in addition to the new unbound list.
>> - The shrinker cound is no longer scaled with
>>   sysctl_vfs_cache_pressure.
>
> I made a mistake and copied the wrong code in my original
> implementation:
>
> vfs_cache_pressure
> ------------------
>
> Controls the tendency of the kernel to reclaim the memory which is used for
> caching of directory and inode objects.
>
> At the default value of vfs_cache_pressure=100 the kernel will attempt to
> reclaim dentries and inodes at a "fair" rate with respect to pagecache and
> swapcache reclaim.  Decreasing vfs_cache_pressure causes the kernel to prefer
> to retain dentry and inode caches. When vfs_cache_pressure=0, the kernel will
> never reclaim dentries and inodes due to memory pressure and this can easily
> lead to out-of-memory conditions. Increasing vfs_cache_pressure beyond 100
> causes the kernel to prefer to reclaim dentries and inodes.
>
> --------------------
>
> It's not an inode or a directory and our pages directly akin to the page
> and buffer caches, so I should have never used vfs_cache_pressure and
> you have not justified why you are adding it back.

Yeah, I agree that the vfs_cache_pressure tuning is rather bogus, but
I'd prefer to revert all the shrinker related changes for now. We can
remedy this in next with a quick patch easily - burned child applies
;-)

>> - When actually shrinking objects, the old code first dropped
>>   purgeable objects, then normal (inactive) objects. Only then did it,
>>   in a last-ditch effort idle the gpu and evict everything. The new
>>   code omits the intermediate step of evicting normal inactive
>>   objects.
>
> This is the crux of the papering, so just do this and call it what it is.

Agreed, I'll clarify the commit message a bit and stress that the
referenced bugs aren't really fixed, but at least the regressions
should be resolved.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch