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
