| Message ID | 1436967532-13251-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 15, 2015 at 03:38:51PM +0200, Daniel Vetter wrote: > Since David Herrmann's mmap vma manager rework we don't need to grab > dev->struct_mutex any more to prevent races when looking up the mmap > offset. Drop it and instead don't forget to use the unref_unlocked > variant (since the drm core still cares). > > While at it also fix a leak when this ioctl is called on an imported > buffer. I don't see where the leak's fixed, but other than that this looks good to me. Shall I pick this up into the drm/tegra tree? Thierry > Cc: Thierry Reding <thierry.reding@gmail.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/tegra/gem.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c > index 01e16e146bfe..827838e64d6e 100644 > --- a/drivers/gpu/drm/tegra/gem.c > +++ b/drivers/gpu/drm/tegra/gem.c > @@ -408,12 +408,9 @@ int tegra_bo_dumb_map_offset(struct drm_file *file, struct drm_device *drm, > struct drm_gem_object *gem; > struct tegra_bo *bo; > > - mutex_lock(&drm->struct_mutex); > - > gem = drm_gem_object_lookup(drm, file, handle); > if (!gem) { > dev_err(drm->dev, "failed to lookup GEM object\n"); > - mutex_unlock(&drm->struct_mutex); > return -EINVAL; > } > > @@ -423,8 +420,6 @@ int tegra_bo_dumb_map_offset(struct drm_file *file, struct drm_device *drm, > > drm_gem_object_unreference(gem); > > - mutex_unlock(&drm->struct_mutex); > - > return 0; > } > > -- > 2.1.4 >
On Mon, Aug 10, 2015 at 12:30:21PM +0200, Thierry Reding wrote: > On Wed, Jul 15, 2015 at 03:38:51PM +0200, Daniel Vetter wrote: > > Since David Herrmann's mmap vma manager rework we don't need to grab > > dev->struct_mutex any more to prevent races when looking up the mmap > > offset. Drop it and instead don't forget to use the unref_unlocked > > variant (since the drm core still cares). > > > > While at it also fix a leak when this ioctl is called on an imported > > buffer. > > I don't see where the leak's fixed, but other than that this looks good > to me. Shall I pick this up into the drm/tegra tree? Copypaste in the commit message from armada, doesn't apply to tegra. Do you also plan to pick up "drm/tegra: Use drm_gem_object_reference_unlocked" directly? And thanks for all the review. -Daniel > > Thierry > > > Cc: Thierry Reding <thierry.reding@gmail.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > --- > > drivers/gpu/drm/tegra/gem.c | 5 ----- > > 1 file changed, 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c > > index 01e16e146bfe..827838e64d6e 100644 > > --- a/drivers/gpu/drm/tegra/gem.c > > +++ b/drivers/gpu/drm/tegra/gem.c > > @@ -408,12 +408,9 @@ int tegra_bo_dumb_map_offset(struct drm_file *file, struct drm_device *drm, > > struct drm_gem_object *gem; > > struct tegra_bo *bo; > > > > - mutex_lock(&drm->struct_mutex); > > - > > gem = drm_gem_object_lookup(drm, file, handle); > > if (!gem) { > > dev_err(drm->dev, "failed to lookup GEM object\n"); > > - mutex_unlock(&drm->struct_mutex); > > return -EINVAL; > > } > > > > @@ -423,8 +420,6 @@ int tegra_bo_dumb_map_offset(struct drm_file *file, struct drm_device *drm, > > > > drm_gem_object_unreference(gem); > > > > - mutex_unlock(&drm->struct_mutex); > > - > > return 0; > > } > > > > -- > > 2.1.4 > >
On Mon, Aug 10, 2015 at 01:31:42PM +0200, Daniel Vetter wrote: > On Mon, Aug 10, 2015 at 12:30:21PM +0200, Thierry Reding wrote: > > On Wed, Jul 15, 2015 at 03:38:51PM +0200, Daniel Vetter wrote: > > > Since David Herrmann's mmap vma manager rework we don't need to grab > > > dev->struct_mutex any more to prevent races when looking up the mmap > > > offset. Drop it and instead don't forget to use the unref_unlocked > > > variant (since the drm core still cares). > > > > > > While at it also fix a leak when this ioctl is called on an imported > > > buffer. > > > > I don't see where the leak's fixed, but other than that this looks good > > to me. Shall I pick this up into the drm/tegra tree? > > Copypaste in the commit message from armada, doesn't apply to tegra. Do > you also plan to pick up "drm/tegra: Use > drm_gem_object_reference_unlocked" directly? I don't mind much either way. I don't think they'll conflict with anything, but since you already said that they're all independent, I don't see a reason why I shouldn't pull them into drm/tegra. If you prefer to keep them together, that's fine with me too. Thanks, Thierry
diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c index 01e16e146bfe..827838e64d6e 100644 --- a/drivers/gpu/drm/tegra/gem.c +++ b/drivers/gpu/drm/tegra/gem.c @@ -408,12 +408,9 @@ int tegra_bo_dumb_map_offset(struct drm_file *file, struct drm_device *drm, struct drm_gem_object *gem; struct tegra_bo *bo; - mutex_lock(&drm->struct_mutex); - gem = drm_gem_object_lookup(drm, file, handle); if (!gem) { dev_err(drm->dev, "failed to lookup GEM object\n"); - mutex_unlock(&drm->struct_mutex); return -EINVAL; } @@ -423,8 +420,6 @@ int tegra_bo_dumb_map_offset(struct drm_file *file, struct drm_device *drm, drm_gem_object_unreference(gem); - mutex_unlock(&drm->struct_mutex); - return 0; }
Since David Herrmann's mmap vma manager rework we don't need to grab dev->struct_mutex any more to prevent races when looking up the mmap offset. Drop it and instead don't forget to use the unref_unlocked variant (since the drm core still cares). While at it also fix a leak when this ioctl is called on an imported buffer. Cc: Thierry Reding <thierry.reding@gmail.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/tegra/gem.c | 5 ----- 1 file changed, 5 deletions(-)