On 05/19/2010 10:13 PM, Daniel Vetter wrote:
On Wed, May 19, 2010 at 09:51:33PM +0200, Thomas Hellström wrote:
Daniel,
TTM releases the spinlock protecting the drm_mm manager in between
evictions to be able to wait (without holding locks) for bo idle.
That means that the lru list may have changed between the first
eviction and the next. In practice, drivers either don't allow
simultaneous bo validation or should disallow it if thrashing is
likely to occur, so the likelyhood of the lru list changing in
between evictions should be small but it needs to be taken into
account.
Well, in my opinion ttm locking optimize for the wrong case: Usually there
should be a little bit of room free to fully pipeline everything. And if
it there is no room free anymore, performance is gonna dip anyway, so we
might as well block.
Yes, the driver is free to block in such cases if it wants to. It's a
matter of taking a command submission rwsem in either read- or write
mode. However, a validation sequence of one DRI client shouldn't
unnecessarily block other renderers whose render targets / sources are
already in memory, but that's only a special case. TTM tries to make
sure and encourage driver writers make sure no CPU locks are held while
waiting for a GPU, which may turn out to be optimizing for the wrong
case in a single-client-single-gpu environment, but turn out to be a
good choice in other situations.
In any case, the code must take into account that the lru may be
modified when the spinlock is released, which I believe you have
addressed below.
I think the linux vm with the split between
asynchronous background writeback and synchronous writeback in case of low
amounts of free memory could be used as inspiration.
Whatever, I think ttm could use this fair eviction stuff even with the
current locking scheme: Do all the accounting under the spinlock, i.e.
- scanning the lru
- building up the eviction list
- mark the memory blocks as free (with drm_mm_put_block)
- reserve the complete free hole (needs a new function in drm_mm, but
range-restricted allocations are not yet implemented yet, anyway) with a
temporary object.
Then do the effective eviction outside the spinlock. iirc ttm already uses
such shadow (dunno what they're really called) objects for buffer moves.
Nevertheless, the drm_mm.c cleanup is
Acked-by: Thomas Hellstrom<thellstrom@xxxxxxxxxx>
Does that include the drm_mm_node->private pointer removal? Jerome naked
that one (but I've objected). Just to clarify.
IIRC, Jerome's range validation patches was using that member at some
stage. I think if Jerome needs the pointer it should stay.
Thanks,
Thomas
I'll leave it to the Intel guys to comment on the fair eviction stuff.
/Thomas
Thanks, Daniel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
