Messages in this thread | | | Subject | Re: [PATCH] drm/tegra: Fix crash caused by reference count imbalance | From | Jon Hunter <> | Date | Wed, 18 May 2016 10:18:52 +0100 |
| |
On 17/05/16 18:36, Daniel Vetter wrote: > On Tue, May 17, 2016 at 7:29 PM, Jon Hunter <jonathanh@nvidia.com> wrote: >>>> @@ -764,6 +769,9 @@ tegra_dsi_connector_duplicate_state(struct drm_connector *connector) >>>> if (!copy) >>>> return NULL; >>>> >>>> + if (copy->base.crtc) >>>> + drm_connector_reference(connector); >>>> + >>> >>> Please use __drm_atomic_helper_connector_duplicate_state instead of >>> open-coding it. >> >> Unfortunately, tegra is allocating and duplicating memory for the entire >> tegra_dsi_state structure (of which drm_connector_state is a member) in >> this function and so I was not able to do that. However, may be Thierry >> can comment on whether that is completely necessary and if we can move >> to using __drm_atomic_helper_connector_duplicate_state() instead. > > Check out how other drivers are using this helper - it is explicitly > for the case where you duplicate the entire struct, and it just > initializes the core part from drm. You can then add your own fixup > code afterwards. It also doesn't matter whether you do kmalloc or > kcalloc or kmemdup - it does a memcpy of its own to make sure state > gets copied.
I had a look but I don't see anyone using the __drm_atomic_helper_connector_duplicate_state() helper, I only see that drivers are using drm_atomic_helper_connector_duplicate_state() directly.
Yes I understand that this helper is doing an explicit copy of the entire drm_connector_state struct and yes I could do something like the following ...
static struct drm_connector_state * tegra_dsi_connector_duplicate_state(struct drm_connector *connector) { struct tegra_dsi_state *state = to_dsi_state(connector->state); struct tegra_dsi_state *copy; copy = kmemdup(state, sizeof(*state), GFP_KERNEL); if (!copy) return NULL;
__drm_atomic_helper_connector_duplicate_state(connector, ©->base); return ©->base; }
... however, this means that I am copying the drm_connector_state twice and this is what I was trying to avoid. Sorry if I am misunderstanding you here, but I don't see how I can avoid the 2nd copy if I use __drm_atomic_helper_connector_duplicate_state().
Cheers Jon
-- nvpublic
| |