From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Hellstrom Subject: Re: [PATCH 0/9] [RFC] fair-lru eviction Date: Wed, 19 May 2010 23:08:52 +0200 Message-ID: <4BF45364.1050204@shipmail.org> References: <1274217111-3882-1-git-send-email-daniel.vetter@ffwll.ch> <4BF44145.1020006@shipmail.org> <20100519201341.GD3537@viiv.ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <20100519201341.GD3537@viiv.ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: Daniel Vetter , intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On 05/19/2010 10:13 PM, Daniel Vetter wrote: > On Wed, May 19, 2010 at 09:51:33PM +0200, Thomas Hellstr=F6m 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 >> = > 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 > =