From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Hellstrom Subject: Re: [PATCH 3/6] drm/ttm: call ttm_bo_cleanup_refs with reservation and lru lock held Date: Wed, 28 Nov 2012 14:21:00 +0100 Message-ID: <50B60FBC.6090907@vmware.com> References: <1354101944-10455-1-git-send-email-maarten.lankhorst@canonical.com> <1354101944-10455-3-git-send-email-maarten.lankhorst@canonical.com> <50B5FB82.9020904@vmware.com> <50B6006A.6030107@canonical.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from smtp-outbound-1.vmware.com (smtp-outbound-1.vmware.com [208.91.2.12]) by gabe.freedesktop.org (Postfix) with ESMTP id DC52BE5C77 for ; Wed, 28 Nov 2012 05:21:05 -0800 (PST) In-Reply-To: <50B6006A.6030107@canonical.com> 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: Maarten Lankhorst Cc: Maarten Lankhorst , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On 11/28/2012 01:15 PM, Maarten Lankhorst wrote: > Op 28-11-12 12:54, Thomas Hellstrom schreef: >> On 11/28/2012 12:25 PM, Maarten Lankhorst wrote: >>> By removing the unlocking of lru and retaking it immediately, a race is >>> removed where the bo is taken off the swap list or the lru list between >>> the unlock and relock. As such the cleanup_refs code can be simplified, >>> it will attempt to call ttm_bo_wait non-blockingly, and if it fails >>> it will drop the locks and perform a blocking wait, or return an error >>> if no_wait_gpu was set. >>> >>> The need for looping is also eliminated, since swapout and evict_mem_first >>> will always follow the destruction path, so no new fence is allowed >>> to be attached. As far as I can see this may already have been the case, >>> but the unlocking / relocking required a complicated loop to deal with >>> re-reservation. >>> >>> The downside is that ttm_bo_cleanup_memtype_use is no longer called with >>> reservation held, so drivers must be aware that move_notify with a null >>> parameter doesn't require a reservation. >> Why can't we unreserve *after* ttm_bo_cleanup_memtype_use? That's not >> immediately clear from this patch. > Because we would hold the reservation while waiting and with the object still > on swap and lru lists still, that would defeat the whole purpose of keeping > the object on multiple lists, plus break current code that assumes bo's on the > those lists can always be reserved. > > the if (ret && !no_wait_gpu) path has to drop the reservation and lru lock, and > isn't guaranteed to be able to retake it. Maybe it could be guaranteed now, but > I'm sure that would not be the case if the reservations were shared across > devices. The evict path removes the BO from the LRU lists, drops the LRU lock but hangs on to the reservation, and in case the wait goes wrong, re-adds the bo to the LRU lists and returns an error. Isn't it possible to do the same in the !no_wait_gpu case? /Thomas