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@xxxxxxxxxxxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Signed-off-by: Matthew Auld <matthew.auld@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxx>
Regards, Joonas
> return ERR_PTR(ret);
> }
>
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
