Re: [PATCH] drm/ttm: fix two regressions since move_notify changes

[Ben Skeggs <skeggsb@gmail.com>]

On Wed, 2012-01-25 at 08:24 +0000, Dave Airlie wrote:
> On Wed, Jan 25, 2012 at 5:34 AM, Ben Skeggs <skeggsb@gmail.com> wrote:
> > From: Ben Skeggs <bskeggs@redhat.com>
> >
> > 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.