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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
