From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Skeggs Subject: Re: [PATCH] drm/ttm: fix two regressions since move_notify changes Date: Wed, 25 Jan 2012 18:38:44 +1000 Message-ID: <1327480724.5189.10.camel@nisroch> References: <20120124223319.GA10002@homer.localdomain> <1327469662-6248-1-git-send-email-skeggsb@gmail.com> Reply-To: skeggsb@gmail.com Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-yx0-f177.google.com (mail-yx0-f177.google.com [209.85.213.177]) by gabe.freedesktop.org (Postfix) with ESMTP id E13E79E75A for ; Wed, 25 Jan 2012 00:35:25 -0800 (PST) Received: by yenm4 with SMTP id m4so1647161yen.36 for ; Wed, 25 Jan 2012 00:35:25 -0800 (PST) In-Reply-To: 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: Dave Airlie Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Wed, 2012-01-25 at 08:24 +0000, Dave Airlie wrote: > On Wed, Jan 25, 2012 at 5:34 AM, Ben Skeggs wrote: > > From: Ben Skeggs > > > > Both changes in dc97b3409a790d2a21aac6e5cdb99558b5944119 cause serious > > regressions in the nouveau driver. > > > > move_notify() was originally able to presume that bo->mem is the old node, > > and new_mem is the new node. The above commit moves the call to > > move_notify() to after move() has been done, which means that now, sometimes, > > new_mem isn't the new node at all, bo->mem is, and new_mem points at a > > stale, possibly-just-been-killed-by-move node. > > > > This is clearly not a good situation. This patch reverts this change, and > > replaces it with a cleanup in the move() failure path instead. > > > > The second issue is that the call to move_notify() from cleanup_memtype_use() > > causes the TTM ghost objects to get passed into the driver. This is clearly > > bad as the driver knows nothing about these "fake" TTM BOs, and ends up > > accessing uninitialised memory. > > btw we've had this check in radeon for a long time. > radeon_ttm_bo_is_radeon_bo > > not sure how you haven't gotten a ghost object in there up until now. I don't believe there was any possible way a ghost object could've ended up in move_notify() until recently. I did notice radeon had checks a long time ago, and always wondered why it bothered when the ghost object is only ever hooked onto the delayed delete list and not really touched by the driver again.. Ben. > > Dave.