Hi Chris,
thank you for the quick response (on a weekend no less).
On Sat, Jul 04, 2015 at 01:31:48PM +0100, Chris Wilson wrote:
> > - return intel_framebuffer_create(dev, &mode_cmd, obj);
> > + fb = intel_framebuffer_create(dev, &mode_cmd, obj);
> > + if (IS_ERR(fb))
> > + drm_gem_object_unreference(&obj->base);
>
> This needs to be drm_gem_object_unreference_unlocked().
You're absolutely right, thanks for pointing this out.
I'm posting a rectified v4 right now.
> It's much simpler if you just document this as consuming the
> obj reference.
Yes but I believe that is what Ville took exception to.
If you guys all agree that documenting this is sufficient then
you can just merge Tvrtko's v2. The rationale of the v3 + v4
I've submitted is to offer an alternative in the hope of pushing
this forward.
> If you want to fix it, you have to move the struct_mutex into
> the caller i.e. eliminate intel_framebuffer_create() and call
> __intel_framebuffer_create().
Hm, I don't understand. The (locking) intel_framebuffer_create
is used by intel_framebuffer_create_for_mode as well as
intel_user_framebuffer_create.
The (non-locking) __intel_framebuffer_create is used by
intelfb_alloc. So it seems both are needed. Daniel added
__intel_framebuffer_create with a8bb6818270c ("drm/i915:
Fix error path leak in fbdev fb allocation"). Incidentally
this is also the commit that introduced the double unref. :-)
We could eliminate the (non-locking) __intel_framebuffer_create
however by briefly unlocking struct_mutex in intelfb_alloc after
i915_gem_alloc_object and then relocking before
intel_pin_and_fence_fb_obj (this is on top of Tvrtko's patch
which moves the locking from intelfb_create to intelfb_alloc).
Best regards,
Lukas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
