On 08/13/2014 02:40 PM, David Herrmann wrote:
Hi
On Wed, Aug 13, 2014 at 2:35 PM, Thomas Hellstrom thellstrom@vmware.com wrote:
On 08/13/2014 12:42 PM, Daniel Vetter wrote:
On Wed, Aug 13, 2014 at 11:06:25AM +0200, Thomas Hellstrom wrote:
On 08/13/2014 05:52 AM, Jérôme Glisse wrote:
From: Jérôme Glisse jglisse@redhat.com
When experiencing memory pressure we want to minimize pool size so that memory we just shrinked is not added back again just as the next thing.
This will divide by 2 the maximum pool size for each device each time the pool have to shrink. The limit is bumped again is next allocation happen after one second since the last shrink. The one second delay is obviously an arbitrary choice.
Jérôme,
I don't like this patch. It adds extra complexity and its usefulness is highly questionable. There are a number of caches in the system, and if all of them added some sort of voluntary shrink heuristics like this, we'd end up with impossible-to-debug unpredictable performance issues.
We should let the memory subsystem decide when to reclaim pages from caches and what caches to reclaim them from.
Yeah, artificially limiting your cache from growing when your shrinker gets called will just break the equal-memory pressure the core mm uses to rebalance between all caches when workload changes. In i915 we let everything grow without artificial bounds and only rely upon the shrinker callbacks to ensure we don't consume more than our fair share of available memory overall. -Daniel
Now when you bring i915 memory usage up, Daniel, I can't refrain from bringing up the old user-space unreclaimable kernel memory issue, for which gem open is a good example ;) Each time user-space opens a gem handle, some un-reclaimable kernel memory is allocated, for which there is no accounting, so theoretically I think a user can bring a system to unusability this way.
Typically there are various limits on unreclaimable objects like this, like open file descriptors, and IIRC the kernel even has an internal limit on the number of struct files you initialize, based on the available system memory, so dma-buf / prime should already have some sort of protection.
gem->filp points to a fresh shmem file, which itself is limited like dmabuf. That should suffice, right?
Thanks David
I'm thinking of situations where you have a gem name and open a new handle. It allocates a new unaccounted idr object. Admittedly you'd have to open a hell of a lot of new handles to stress the system, but that's an example of the situation I'm thinking of. Similarly perhaps if you create a gem handle from a prime file-descriptor but I haven't looked at that code in detail.
Thanks
/Thomas