| Message ID | 1461323137-21045-1-git-send-email-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On pe, 2016-04-22 at 12:05 +0100, Matthew Auld wrote: > If we are not in FULL_48BIT_PPGTT mode then we really shouldn't > continue on with our allocations, given that the call to free_pdp would > bail early without freeing everything, thus leaking memory. > > v2: > (Joonas Lahtinen) > - tidy up with goto teardown path > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 0d666b3..f996f4b 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -567,13 +567,16 @@ static struct > i915_page_directory_pointer *alloc_pdp(struct drm_device *dev) > { > struct i915_page_directory_pointer *pdp; > - int ret = -ENOMEM; > + int ret = -EINVAL; > > - WARN_ON(!USES_FULL_48BIT_PPGTT(dev)); > + if (WARN_ON(!USES_FULL_48BIT_PPGTT(dev))) > + goto fail_pdp; > > pdp = kzalloc(sizeof(*pdp), GFP_KERNEL); > - if (!pdp) > - return ERR_PTR(-ENOMEM); > + if (!pdp) { > + ret = -ENOMEM; > + goto fail_pdp; > + } > > ret = __pdp_init(dev, pdp); > if (ret) > @@ -589,7 +592,7 @@ fail_page_m: > __pdp_fini(pdp); > fail_bitmap: > kfree(pdp); > - > +fail_pdp: According to the Kernel Coding Style Chapter 7: "Centralized exiting of functions", the naming of this label should likely just be "fail:", as it does not free anything. The label should describe what is done under the label, not why the label is entered (which may vary as function grows). With that, Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Regards, Joonas > return ERR_PTR(ret); > } >
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 0d666b3..f996f4b 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -567,13 +567,16 @@ static struct i915_page_directory_pointer *alloc_pdp(struct drm_device *dev) { struct i915_page_directory_pointer *pdp; - int ret = -ENOMEM; + int ret = -EINVAL; - WARN_ON(!USES_FULL_48BIT_PPGTT(dev)); + if (WARN_ON(!USES_FULL_48BIT_PPGTT(dev))) + goto fail_pdp; pdp = kzalloc(sizeof(*pdp), GFP_KERNEL); - if (!pdp) - return ERR_PTR(-ENOMEM); + if (!pdp) { + ret = -ENOMEM; + goto fail_pdp; + } ret = __pdp_init(dev, pdp); if (ret) @@ -589,7 +592,7 @@ fail_page_m: __pdp_fini(pdp); fail_bitmap: kfree(pdp); - +fail_pdp: return ERR_PTR(ret); }
If we are not in FULL_48BIT_PPGTT mode then we really shouldn't continue on with our allocations, given that the call to free_pdp would bail early without freeing everything, thus leaking memory. v2: (Joonas Lahtinen) - tidy up with goto teardown path Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Signed-off-by: Matthew Auld <matthew.auld@intel.com> --- drivers/gpu/drm/i915/i915_gem_gtt.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)