[PATCH 2/3] drm: make DRM_UNLOCKED ioctls with their own mutex

Ilija Hadzic ihadzic at research.bell-labs.com
Wed Oct 26 09:11:14 PDT 2011



On Wed, 26 Oct 2011, Daniel Vetter wrote:

>
> Just to check before I dig into reviewing this: Have you check all the
> other users of these data structure that these functions touch whether
> they don't accidentally rely on the global lock being taken?

I did and thought it was safe. I re-checked this morning prompted by 
your note and of course there is one (easily fixable) thing that I missed.
Here is the full "report":

drm_getclient is safe: the only thing that should be protected there is 
dev->filelist and that is all protected in other functions using 
struct_mutex.

drm_getstats is safe: actually I think there is no need for any mutex there
because the loop runs through quasi-static (set at load time only) data, 
and the actual count access is done with atomic_read()

drm_getmap has one problem which I can fix (and it's the hardest to 
follow): the structure that should be protected there is the dev->maplist. 
There are three functions that may potentially be an issue:

    drm_getsarea doesn't grab any lock before touching dev->maplist (so
    global mutex won't help here either). However, drivers seem to call
    it only at initialization time so it probably doesn't matter

    drm_master_destroy doesn't grab any lock either, but it is called
    from drm_master_put which is protected with dev->struct_mutex
    everywhere else in drm module. I also see a couple of calls
    to drm_master_destroy from vmwgfx driver that look unlocked
    to me, but drm_global_mutex won't help here either

    drm_getsareactx uses struct_mutex, but it apparently releases it
    too early (that's the problem I missed initially). If it's released
    after it's done with dev->maplist, we should be fine.

I'll redo this patch with a fix to drm_getsareactx and that should do it.
I would still appreciate another pair of eyes to make sure I didn't miss 
anything else

thanks,

-- Ilija








More information about the dri-devel mailing list