On Thu, Jan 12, 2017 at 01:11:50PM +0000, Tvrtko Ursulin wrote:
>
> On 11/01/2017 21:09, Chris Wilson wrote:
> >+struct i915_gem_context *
> >+mock_context(struct drm_i915_private *i915,
> >+ const char *name)
> >+{
> >+ struct i915_gem_context *ctx;
> >+ int ret;
> >+
> >+ ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> >+ if (!ctx)
> >+ return NULL;
> >+
> >+ kref_init(&ctx->ref);
> >+ INIT_LIST_HEAD(&ctx->link);
> >+ ctx->name = name ? kstrdup(name, GFP_KERNEL) : NULL;
>
> Care or not whether allocation worked?
Not really, ctx->name is a hint. But not checking is just asking for
trouble later.
> >+static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
> >+{
> >+ const unsigned long sz = roundup_pow_of_two(sizeof(struct intel_ring));
>
> You certainly like your longs. Never mind. :)
We have a habit of growing our structs. Consider this future proofing
;)
> >+ struct intel_ring *ring;
> >+
> >+ ring = kzalloc(sizeof(*ring) + sz, GFP_TEMPORARY);
>
> Why GFP_TEMPORARY, the mocked context & co are not?
Depends on the phase I was writing the patch. The first tests were fast,
and so GFP_TEMPORARY seemed appropriate, and I used it everywhere I
remembered. With some tests being deliberately slow and others
deliberately using all memory, GFP_TEMPORARY seems more like misuse.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
