Hello, On Wednesday, 4 April 2018 19:18:42 EEST Daniel Vetter wrote: > On Wed, Apr 4, 2018 at 3:37 PM, Lucas Stach <l.stach@xxxxxxxxxxxxxx> wrote: > > Am Montag, den 02.04.2018, 21:50 +0300 schrieb Laurent Pinchart: > >> The __omap_gem_get_pages() function is a wrapper around > >> omap_gem_attach_pages() that returns the omap_obj->pages pointer through > >> a function argument. Some callers don't need the pages pointer, and all > >> of them can access omap_obj->pages directly. To simplify the code merge > >> the __omap_gem_get_pages() wrapper with omap_gem_attach_pages() and > >> update the callers accordingly. > >> > >> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > >> > >> --- > >> > >> drivers/gpu/drm/omapdrm/omap_gem.c | 62 ++++++++++++-------------------- > >> 1 file changed, 23 insertions(+), 39 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c > >> b/drivers/gpu/drm/omapdrm/omap_gem.c index 6cfcf60cffe3..13fea207343e > >> 100644 > >> --- a/drivers/gpu/drm/omapdrm/omap_gem.c > >> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c > >> @@ -222,7 +222,10 @@ static void omap_gem_evict(struct drm_gem_object > >> *obj) > >> * Page Management > >> */ > >> > >> -/** ensure backing pages are allocated */ > >> +/* > >> + * Ensure backing pages are allocated. Must be called with the > >> omap_obj.lock > >> + * held. > >> + */ > > > > Drive-by comment: I always find it hard to validate those comment-only > > lock prerequisites, especially if callstacks get deeper. > > > > What we do in etnaviv is to make those lock comments executable by > > using lockdep_assert_held() to validate the locking assumptions. This > > makes sure that if you ever manage to violate the locking in a code > > rework, a lockdep enabled debug build will explode right at the spot. > > +1 on this. I've gone as far as removing all the locking related > comments in core drm code because most of it was misleading or > outright wrong. The runtime checks have a much higher chance of > actually being correct :-) I agree, I'll fix that in the next version. I plan to keep the comment though, as I find it easier to read when glancing at the function, but I'll add a corresponding lockdep_assert_held(). [snip] -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel