drm: optimize drm_framebuffer_remove

Now that all framebuffer usage is properly refcounted, we are no
longer required to hold the modeset locks while dropping the last
reference. Hence implemented a fastpath which avoids the potential
stalls associated with grabbing mode_config.lock for the case where
there's no other reference around.

Explain in a big comment why it is safe. Also update kerneldocs with
the new locking rules around drm_framebuffer_remove.

Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This commit is contained in:
Daniel Vetter 2012-12-11 16:51:35 +01:00
parent 2fd5eabab0
commit b62584e366

View File

@ -517,7 +517,11 @@ EXPORT_SYMBOL(drm_framebuffer_cleanup);
*
* Scans all the CRTCs and planes in @dev's mode_config. If they're
* using @fb, removes it, setting it to NULL. Then drops the reference to the
* passed-in framebuffer.
* passed-in framebuffer. Might take the modeset locks.
*
* Note that this function optimizes the cleanup away if the caller holds the
* last reference to the framebuffer. It is also guaranteed to not take the
* modeset locks in this case.
*/
void drm_framebuffer_remove(struct drm_framebuffer *fb)
{
@ -527,9 +531,25 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
struct drm_mode_set set;
int ret;
WARN_ON(!drm_modeset_is_locked(dev));
WARN_ON(!list_empty(&fb->filp_head));
/*
* drm ABI mandates that we remove any deleted framebuffers from active
* useage. But since most sane clients only remove framebuffers they no
* longer need, try to optimize this away.
*
* Since we're holding a reference ourselves, observing a refcount of 1
* means that we're the last holder and can skip it. Also, the refcount
* can never increase from 1 again, so we don't need any barriers or
* locks.
*
* Note that userspace could try to race with use and instate a new
* usage _after_ we've cleared all current ones. End result will be an
* in-use fb with fb-id == 0. Userspace is allowed to shoot its own foot
* in this manner.
*/
if (atomic_read(&fb->refcount.refcount) > 1) {
drm_modeset_lock_all(dev);
/* remove from any CRTC */
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
if (crtc->fb == fb) {
@ -555,6 +575,8 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
plane->crtc = NULL;
}
}
drm_modeset_unlock_all(dev);
}
drm_framebuffer_unreference(fb);
}
@ -2538,9 +2560,7 @@ int drm_mode_rmfb(struct drm_device *dev,
mutex_unlock(&dev->mode_config.fb_lock);
mutex_unlock(&file_priv->fbs_lock);
drm_modeset_lock_all(dev);
drm_framebuffer_remove(fb);
drm_modeset_unlock_all(dev);
return 0;
@ -2691,9 +2711,7 @@ void drm_fb_release(struct drm_file *priv)
list_del_init(&fb->filp_head);
/* This will also drop the fpriv->fbs reference. */
drm_modeset_lock_all(dev);
drm_framebuffer_remove(fb);
drm_modeset_unlock_all(dev);
}
mutex_unlock(&priv->fbs_lock);
}