From: Dave Gordon <david.s.gordon@intel.com>
To: intel-gfx@lists.freedesktop.org, Chris Wilson <chris@chris-wilson.co.uk>
Subject: Re: [PATCH] drm/i915: Propagate error from drm_gem_object_init()
Date: Fri, 22 Apr 2016 12:59:43 +0100 [thread overview]
Message-ID: <571A122F.7000109@intel.com> (raw)
In-Reply-To: <1461322677-20728-1-git-send-email-matthew.auld@intel.com>
On 22/04/16 11:57, Matthew Auld wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
>
> Propagate the real error from drm_gem_object_init(). Note this also
> fixes some confusion in the error return from i915_gem_alloc_object...
>
> v2:
> (Matthew Auld)
> - updated new users of gem_alloc_object from latest drm-nightly
> - replaced occurrences of IS_ERR_OR_NULL() with IS_ERR()
> v3:
> (Joonas Lahtinen)
> - fix double "From:" in commit message
> - add goto teardown path
>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 21 +++++++++++++--------
> drivers/gpu/drm/i915/i915_gem_batch_pool.c | 4 ++--
> drivers/gpu/drm/i915/i915_gem_context.c | 4 ++--
> drivers/gpu/drm/i915/i915_gem_render_state.c | 7 +++++--
> drivers/gpu/drm/i915/i915_guc_submission.c | 2 +-
> drivers/gpu/drm/i915/intel_display.c | 4 ++--
> drivers/gpu/drm/i915/intel_fbdev.c | 4 ++--
> drivers/gpu/drm/i915/intel_lrc.c | 10 ++++++----
> drivers/gpu/drm/i915/intel_overlay.c | 2 +-
> drivers/gpu/drm/i915/intel_ringbuffer.c | 19 ++++++++++---------
> 10 files changed, 44 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 261a3ef..c6c17dd 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -382,8 +382,8 @@ i915_gem_create(struct drm_file *file,
>
> /* Allocate the new object */
> obj = i915_gem_alloc_object(dev, size);
> - if (obj == NULL)
> - return -ENOMEM;
> + if (IS_ERR(obj))
> + return PTR_ERR(obj);
>
> ret = drm_gem_handle_create(file, &obj->base, &handle);
> /* drop reference from allocate - handle holds it now */
> @@ -4498,15 +4498,15 @@ struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
> struct drm_i915_gem_object *obj;
> struct address_space *mapping;
> gfp_t mask;
> + int ret;
>
> obj = i915_gem_object_alloc(dev);
> if (obj == NULL)
> - return NULL;
> + return ERR_PTR(-ENOMEM);
The two changes above looked really really confusing, where one tests
the returned pointer and returns it if it's an ERR_PTR, and the other
tests for NULL and returns ERR_PTR(-ENOMEM).
Then I realised one was i915_gem_alloc_object() and the other was
i915_gem_object_alloc()!
Can we please get rid of one or the other? Since we generally use
subsystem_class_action naming, I'd suggest keeping (the low-level
memory-allocator) i915_gem_object_alloc(), and renaming the high-level
i915_gem_alloc_object() to i915_gem_object_create() or similar.
> - if (drm_gem_object_init(dev, &obj->base, size) != 0) {
> - i915_gem_object_free(obj);
> - return NULL;
> - }
> + ret = drm_gem_object_init(dev, &obj->base, size);
> + if (ret)
> + goto fail;
>
> mask = GFP_HIGHUSER | __GFP_RECLAIMABLE;
> if (IS_CRESTLINE(dev) || IS_BROADWATER(dev)) {
> @@ -4543,6 +4543,11 @@ struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
> trace_i915_gem_object_create(obj);
Oh and BTW i915_gem_alloc_object() already calls itself
i915_gem_object_create() in trace messages!
.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2016-04-22 11:59 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-22 10:57 [PATCH] drm/i915: Propagate error from drm_gem_object_init() Matthew Auld
2016-04-22 11:59 ` Dave Gordon [this message]
2016-04-22 13:44 ` Joonas Lahtinen
2016-04-24 6:57 ` ✗ Fi.CI.BAT: failure for " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2016-04-25 12:32 [PATCH] " Matthew Auld
2015-09-23 11:32 [PATCH 5/5] drm/i915: Propagating correct error codes to the userspace Chris Wilson
2015-09-23 13:14 ` [PATCH] drm/i915: Propagate error from drm_gem_object_init() Chris Wilson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=571A122F.7000109@intel.com \
--to=david.s.gordon@intel.com \
--cc=chris@chris-wilson.co.uk \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.