On Mon, Dec 11, 2017 at 11:39 AM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > Teach lockdep to track the device's internal mmapping separately > from the generic lockclass over all other inodes. Since this is device > private we wish to allow a different locking hierarchy than is typified > by the requirement for the mmap_rwsem being the outermost lock for > handling pagefaults. By giving the internal mmap_rwsem a distinct > lockclass, lockdep can identify it and learn/enforce its distinct locking > requirements. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104209 > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> I think both the commit message and comment are a bit too fluffy - the critical bit is that we're biting ourselves on gtt mmaps from usersptr, and that's explicitly not allowed exactly because it would deadlock. I'm also not sure it's a good idea to implement this in generic code, since this is a very i915 specific issue, and other drivers (who might be a lot less sloppy here) will now no longer get reports about this deadlock. Aside from that I'm not really sure why you think the bugzilla link is a false positive: The mapping->rwsem is the one for the gtt in both cases I think. -Daniel > --- > drivers/gpu/drm/drm_drv.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 9acc1e157813..21ad06c3d684 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -393,6 +393,7 @@ static struct file_system_type drm_fs_type = { > > static struct inode *drm_fs_inode_new(void) > { > + static struct lock_class_key lockclass; > struct inode *inode; > int r; > > @@ -403,8 +404,22 @@ static struct inode *drm_fs_inode_new(void) > } > > inode = alloc_anon_inode(drm_fs_mnt->mnt_sb); > - if (IS_ERR(inode)) > + if (IS_ERR(inode)) { > simple_release_fs(&drm_fs_mnt, &drm_fs_cnt); > + return inode; > + } > + > + /* > + * Teach lockdep to track the device's internal mmapping separately > + * from all other inodes. Since this is device private we wish to > + * allow a different locking hierarchy than is typified by the > + * requirement for the mmap_rwsem being the outermost lock for > + * handling pagefaults. By giving the internal mmap_rwsem a distinct > + * lockclass, lockdep can identify it and thereby learn and enforce its > + * distinct locking requirements. > + */ > + lockdep_set_class_and_name(&inode->i_mapping->i_mmap_rwsem, > + &lockclass, "drm_fs_inode"); > > return inode; > } > -- > 2.15.1 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
