On Wed, May 06, 2015 at 10:22:22AM -0400, Rob Clark wrote:
> Hey Russell,
>
> I just noticed a splat triggered by component_unbind_all() called from
> ->bind().. any opinions about whether component stuff should handle
> that case, or whether I should rearrange my error cleanup path to not
> component_unbind_all() in this case?
Essentially, if component_bind_all() returned an error, you should not
call component_unbind_all() - component_bind_all() has the effect that
it's either successful and all components have been bound, or when it
fails, no components are bound.
component_unbind_all() assumes that the components were previously
successfully bound.
When I look at the MSM code, I can see why this happens, and it's
basically because of broken error cleanup handling caused by this
commit:
commit 5bf9c0b614542d69fb9a8681a0411715cc3e8ba8
Author: Rob Clark <robdclark@xxxxxxxxx>
Date: Tue Mar 3 15:04:24 2015 -0500
drm/msm: split out vram initialization
We'll want to extend this a bit to handle also a reserved-memory
("stolen") region, so that drm/msm can take-over bootloader splash
screen. First split it out into it's own fxn to reduce noise in
the following patch.
Signed-off-by: Rob Clark <robdclark@xxxxxxxxx>
Let's look at the clean-up paths:
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv) {
dev_err(dev->dev, "failed to allocate private data\n");
return -ENOMEM;
returns immediately.
ret = msm_init_vram(dev);
if (ret)
goto fail;
calls msm_unload.
/* Bind all our sub-components: */
ret = component_bind_all(dev->dev, dev);
if (ret)
return ret;
doesn't call msm_unload.
Why would the second not need to run any cleanup if failing at
msm_init_vram() does? This is totally mad.
I think you need to fix this driver to have a sane error cleanup
implementation...
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
