Re: [PATCH] drm/i915: Fix failure paths around initial fbdev allocation

[Lukas Wunner <lukas@wunner.de>]

Hi,

On Thu, Jul 02, 2015 at 02:59:24PM +0100, Tvrtko Ursulin wrote:
> On 07/02/2015 02:37 PM, Ville Syrjälä wrote:
> >On Tue, Jun 30, 2015 at 10:06:27AM +0100, Tvrtko Ursulin wrote:
> >>Ville, you suggested some changes earlier;
> >>"""
> >>I suggest removing the unref from __intel_framebuffer_create().
> >>"""
> >>Do you view that as an improvement in code clarity/organisation,
> >>or you think my version is actually wrong for some reason?
> >I find it rather unexpected that the function drops the passed
> >reference on error. My usual rule is: do nothing on error, if possible.
> I just don't have time at the moment to evaluate the other call sites etc.
> So from my point of view it boils down to whether this patch improves things
> without making anything worse. If it does R-B would be cool. If it doesn't
> then it will have to make for free time to appear. Or someone is free to
> take it over.

I think Ville's suggestion is merited, right now the unref on failure
happens at the bottom of the call chain in __intel_framebuffer_create
while the ref happened further up in the call chain. This should really
be consolidated in the same place. That a double unref managed to sneak
into intelfb_alloc proves this is error prone.

I've just submitted a v3 which is functionally equivalent to Tvrtko's v2
but takes up Ville's suggestion. This consists of two patches now, the
first one being equivalent to Tvrtko's patch sans fix for the double unref,
the second one fixing the double unref by moving the unref further up the
call chain. Of course I've tested this, works for me.

Now you can decide for yourself which version you like better. :-)

Thanks,

Lukas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx