Re: [PATCH 05/13] drm/ttm: overhaul memory accounting

[Jerome Glisse <j.glisse@gmail.com>]

On Thu, Nov 10, 2011 at 09:05:22PM +0100, Thomas Hellstrom wrote:
> On 11/10/2011 07:05 PM, Jerome Glisse wrote:
> >On Thu, Nov 10, 2011 at 11:27:33AM +0100, Thomas Hellstrom wrote:
> >>On 11/09/2011 09:22 PM, j.glisse@gmail.com wrote:
> >>>From: Jerome Glisse<jglisse@redhat.com>
> >>>
> >>>This is an overhaul of the ttm memory accounting. This tries to keep
> >>>the same global behavior while removing the whole zone concept. It
> >>>keeps a distrinction for dma32 so that we make sure that ttm don't
> >>>starve the dma32 zone.
> >>>
> >>>There is 3 threshold for memory allocation :
> >>>- max_mem is the maximum memory the whole ttm infrastructure is
> >>>   going to allow allocation for (exception of system process see
> >>>   below)
> >>>- emer_mem is the maximum memory allowed for system process, this
> >>>   limit is>   to max_mem
> >>>- swap_limit is the threshold at which point ttm will start to
> >>>   try to swap object because ttm is getting close the max_mem
> >>>   limit
> >>>- swap_dma32_limit is the threshold at which point ttm will start
> >>>   swap object to try to reduce the pressure on the dma32 zone. Note
> >>>   that we don't specificly target object to swap to it might very
> >>>   well free more memory from highmem rather than from dma32
> >>>
> >>>Accounting is done through used_mem&   used_dma32_mem, which sum give
> >>>the total amount of memory actually accounted by ttm.
> >>>
> >>>Idea is that allocation will fail if (used_mem + used_dma32_mem)>
> >>>max_mem and if swapping fail to make enough room.
> >>>
> >>>The used_dma32_mem can be updated as a later stage, allowing to
> >>>perform accounting test before allocating a whole batch of pages.
> >>>
> >>Jerome, you're removing a fair amount of functionality here, without
> >>justifying
> >>why it could be removed.
> >All this code was overkill.
> 
> [1] I don't agree, and since it's well tested, thought throught and
> working, I see no obvious reason to alter it,
> within the context of this patch series unless it's absolutely
> required for the functionality.

Well one thing i can tell is that it doesn't work on radeon, i pushed
a test to libdrm and here it's the oom that starts doing its beating.
Anyway i won't alter it. Was just trying to make it works, ie be useful
while also being simpler.

> >>Consider a low-end system with 1G of kernel memory and 10G of
> >>highmem. How do we avoid putting stress on the kernel memory? I also
> >>wouldn't be too surprised if DMA32 zones appear in HIGHMEM systems
> >>in the future making the current zone concept good to keep.
> >Right now kernel memory is accounted against all zone so it decrease
> >not only the kernel zone but also the dma32&  highmem if present.
> 
> Do you mean that the code is incorrect? In that case, did you
> consider the fact
> that zones may overlap? (Although I admit the name "highmem" might
> be misleading. Should be "total").

Yeah i am well aware that zone overlap :)

> >Note also that kernel zone in current code == dma32 zone.
> 
> Last time I looked, the highmem split was typically at slightly less
> than 1GB, depending on the hardware and desired setup. I admit that
> was some time ago, but has that really changed? On all archs?
> Furthermore, on !Highmem systems, All pages are in the kernel zone.

I was bit too much focused on my system where 1G of ram is wonderland
and 512M is the average. Thx to AMD i got a system with 8G i should
use it more.

Cheers,
Jerome