Skip to content

Commit 9f0ba53

Browse files
committed
drm/gem: support BO freeing without dev->struct_mutex
Finally all the core gem and a lot of drivers are entirely free of dev->struct_mutex depencies, and we can start to have an entirely lockless unref path. To make sure that no one who touches the core code accidentally breaks existing drivers which still require dev->struct_mutex I've made the might_lock check unconditional. While at it de-inline the ref/unref functions, they've become a bit too big. v2: Make it not leak like a sieve. v3: Review from Lucas: - drop != NULL in pointer checks. - fixup copypasted kerneldoc to actually match the functions. v4: Add __drm_gem_object_unreference as a fastpath helper for drivers who abolished dev->struct_mutex, requested by Chris. v5: Fix silly mistake in drm_gem_object_unreference_unlocked caught by intel-gfx CI - I checked for gem_free_object instead of gem_free_object_unlocked ... Cc: Chris Wilson <[email protected]> Cc: Alex Deucher <[email protected]> Cc: Lucas Stach <[email protected]> Reviewed-by: Lucas Stach <[email protected]> (v3) Reviewed-by: Chris Wilson <[email protected]> (v4) Reviewed-by: Alex Deucher <[email protected]> Signed-off-by: Daniel Vetter <[email protected]> Signed-off-by: Daniel Vetter <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
1 parent 8636d45 commit 9f0ba53

File tree

3 files changed

+80
-37
lines changed

3 files changed

+80
-37
lines changed

drivers/gpu/drm/drm_gem.c

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -806,11 +806,63 @@ drm_gem_object_free(struct kref *kref)
806806

807807
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
808808

809-
if (dev->driver->gem_free_object != NULL)
809+
if (dev->driver->gem_free_object_unlocked)
810+
dev->driver->gem_free_object_unlocked(obj);
811+
else if (dev->driver->gem_free_object)
810812
dev->driver->gem_free_object(obj);
811813
}
812814
EXPORT_SYMBOL(drm_gem_object_free);
813815

816+
/**
817+
* drm_gem_object_unreference_unlocked - release a GEM BO reference
818+
* @obj: GEM buffer object
819+
*
820+
* This releases a reference to @obj. Callers must not hold the
821+
* dev->struct_mutex lock when calling this function.
822+
*
823+
* See also __drm_gem_object_unreference().
824+
*/
825+
void
826+
drm_gem_object_unreference_unlocked(struct drm_gem_object *obj)
827+
{
828+
struct drm_device *dev;
829+
830+
if (!obj)
831+
return;
832+
833+
dev = obj->dev;
834+
might_lock(&dev->struct_mutex);
835+
836+
if (dev->driver->gem_free_object_unlocked)
837+
kref_put(&obj->refcount, drm_gem_object_free);
838+
else if (kref_put_mutex(&obj->refcount, drm_gem_object_free,
839+
&dev->struct_mutex))
840+
mutex_unlock(&dev->struct_mutex);
841+
}
842+
EXPORT_SYMBOL(drm_gem_object_unreference_unlocked);
843+
844+
/**
845+
* drm_gem_object_unreference - release a GEM BO reference
846+
* @obj: GEM buffer object
847+
*
848+
* This releases a reference to @obj. Callers must hold the dev->struct_mutex
849+
* lock when calling this function, even when the driver doesn't use
850+
* dev->struct_mutex for anything.
851+
*
852+
* For drivers not encumbered with legacy locking use
853+
* drm_gem_object_unreference_unlocked() instead.
854+
*/
855+
void
856+
drm_gem_object_unreference(struct drm_gem_object *obj)
857+
{
858+
if (obj) {
859+
WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex));
860+
861+
kref_put(&obj->refcount, drm_gem_object_free);
862+
}
863+
}
864+
EXPORT_SYMBOL(drm_gem_object_unreference);
865+
814866
/**
815867
* drm_gem_vm_open - vma->ops->open implementation for GEM
816868
* @vma: VM area structure

include/drm/drmP.h

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -580,12 +580,21 @@ struct drm_driver {
580580
void (*debugfs_cleanup)(struct drm_minor *minor);
581581

582582
/**
583-
* Driver-specific constructor for drm_gem_objects, to set up
584-
* obj->driver_private.
583+
* @gem_free_object: deconstructor for drm_gem_objects
585584
*
586-
* Returns 0 on success.
585+
* This is deprecated and should not be used by new drivers. Use
586+
* @gem_free_object_unlocked instead.
587587
*/
588588
void (*gem_free_object) (struct drm_gem_object *obj);
589+
590+
/**
591+
* @gem_free_object_unlocked: deconstructor for drm_gem_objects
592+
*
593+
* This is for drivers which are not encumbered with dev->struct_mutex
594+
* legacy locking schemes. Use this hook instead of @gem_free_object.
595+
*/
596+
void (*gem_free_object_unlocked) (struct drm_gem_object *obj);
597+
589598
int (*gem_open_object) (struct drm_gem_object *, struct drm_file *);
590599
void (*gem_close_object) (struct drm_gem_object *, struct drm_file *);
591600

include/drm/drm_gem.h

Lines changed: 15 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -200,47 +200,29 @@ drm_gem_object_reference(struct drm_gem_object *obj)
200200
}
201201

202202
/**
203-
* drm_gem_object_unreference - release a GEM BO reference
203+
* __drm_gem_object_unreference - raw function to release a GEM BO reference
204204
* @obj: GEM buffer object
205205
*
206-
* This releases a reference to @obj. Callers must hold the dev->struct_mutex
207-
* lock when calling this function, even when the driver doesn't use
208-
* dev->struct_mutex for anything.
206+
* This function is meant to be used by drivers which are not encumbered with
207+
* dev->struct_mutex legacy locking and which are using the
208+
* gem_free_object_unlocked callback. It avoids all the locking checks and
209+
* locking overhead of drm_gem_object_unreference() and
210+
* drm_gem_object_unreference_unlocked().
209211
*
210-
* For drivers not encumbered with legacy locking use
211-
* drm_gem_object_unreference_unlocked() instead.
212+
* Drivers should never call this directly in their code. Instead they should
213+
* wrap it up into a driver_gem_object_unreference(struct driver_gem_object
214+
* *obj) wrapper function, and use that. Shared code should never call this, to
215+
* avoid breaking drivers by accident which still depend upon dev->struct_mutex
216+
* locking.
212217
*/
213218
static inline void
214-
drm_gem_object_unreference(struct drm_gem_object *obj)
219+
__drm_gem_object_unreference(struct drm_gem_object *obj)
215220
{
216-
if (obj != NULL) {
217-
WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex));
218-
219-
kref_put(&obj->refcount, drm_gem_object_free);
220-
}
221+
kref_put(&obj->refcount, drm_gem_object_free);
221222
}
222223

223-
/**
224-
* drm_gem_object_unreference_unlocked - release a GEM BO reference
225-
* @obj: GEM buffer object
226-
*
227-
* This releases a reference to @obj. Callers must not hold the
228-
* dev->struct_mutex lock when calling this function.
229-
*/
230-
static inline void
231-
drm_gem_object_unreference_unlocked(struct drm_gem_object *obj)
232-
{
233-
struct drm_device *dev;
234-
235-
if (!obj)
236-
return;
237-
238-
dev = obj->dev;
239-
if (kref_put_mutex(&obj->refcount, drm_gem_object_free, &dev->struct_mutex))
240-
mutex_unlock(&dev->struct_mutex);
241-
else
242-
might_lock(&dev->struct_mutex);
243-
}
224+
void drm_gem_object_unreference_unlocked(struct drm_gem_object *obj);
225+
void drm_gem_object_unreference(struct drm_gem_object *obj);
244226

245227
int drm_gem_handle_create(struct drm_file *file_priv,
246228
struct drm_gem_object *obj,

0 commit comments

Comments
 (0)