I had reasons to look closer at the MSM DRM driver error handling and
realised that it had suffered from a fair amount of bit rot over the
years.
Unfortunately, I started fixing this in my 6.2 branch and failed to
notice two partial and, as it turned out, broken attempts to address
this that are now in 6.3-rc1.
Instead of trying to salvage this incrementally, I'm reverting the two
broken commits so that clean and backportable fixes can be added in
their place.
Included are also two related cleanups.
Johan
Johan Hovold (10):
Revert "drm/msm: Add missing check and destroy for
alloc_ordered_workqueue"
Revert "drm/msm: Fix failure paths in msm_drm_init()"
drm/msm: fix NULL-deref on snapshot tear down
drm/msm: fix NULL-deref on irq uninstall
drm/msm: fix drm device leak on bind errors
drm/msm: fix vram leak on bind errors
drm/msm: fix missing wq allocation error handling
drm/msm: fix workqueue leak on bind errors
drm/msm: use drmm_mode_config_init()
drm/msm: move include directive
drivers/gpu/drm/msm/disp/msm_disp_snapshot.c | 3 -
drivers/gpu/drm/msm/msm_drv.c | 67 +++++++++++++-------
2 files changed, 44 insertions(+), 26 deletions(-)
--
2.39.2
This reverts commit 643b7d0869cc7f1f7a5ac7ca6bd25d88f54e31d0.
A recent patch that tried to fix up the msm_drm_init() paths with
respect to the workqueue but only ended up making things worse:
First, the newly added calls to msm_drm_uninit() on early errors would
trigger NULL-pointer dereferences, for example, as the kms pointer would
not have been initialised. (Note that these paths were also modified by
a second broken error handling patch which in effect cancelled out this
part when merged.)
Second, the newly added allocation sanity check would still leak the
previously allocated drm device.
Instead of trying to salvage what was badly broken (and clearly not
tested), let's revert the bad commit so that clean and backportable
fixes can be added in its place.
Fixes: 643b7d0869cc ("drm/msm: Add missing check and destroy for alloc_ordered_workqueue")
Cc: Jiasheng Jiang <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/gpu/drm/msm/msm_drv.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index aca48c868c14..b7f5a78eadd4 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -420,8 +420,6 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
priv->dev = ddev;
priv->wq = alloc_ordered_workqueue("msm", 0);
- if (!priv->wq)
- return -ENOMEM;
INIT_LIST_HEAD(&priv->objects);
mutex_init(&priv->obj_lock);
--
2.39.2
This reverts commit 8636500300a01740d92b345c680b036b94555b1b.
A recent commit tried to address a drm device leak in the early
msm_drm_uninit() error paths but ended up making things worse.
Specifically, it moved the drm device reference put in msm_drm_uninit()
to msm_drm_init() which means that the drm would now be leaked on normal
unbind.
For reasons that were never spelled out, it also added kms NULL pointer
checks to a couple of helper functions that had nothing to do with the
paths modified by the patch.
Instead of trying to salvage this incrementally, let's revert the bad
commit so that clean and backportable fixes can be added in its place.
Fixes: 8636500300a0 ("drm/msm: Fix failure paths in msm_drm_init()")
Cc: Akhil P Oommen <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/gpu/drm/msm/disp/msm_disp_snapshot.c | 3 ---
drivers/gpu/drm/msm/msm_drv.c | 11 ++++-------
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/msm_disp_snapshot.c b/drivers/gpu/drm/msm/disp/msm_disp_snapshot.c
index b73031cd48e4..e75b97127c0d 100644
--- a/drivers/gpu/drm/msm/disp/msm_disp_snapshot.c
+++ b/drivers/gpu/drm/msm/disp/msm_disp_snapshot.c
@@ -129,9 +129,6 @@ void msm_disp_snapshot_destroy(struct drm_device *drm_dev)
}
priv = drm_dev->dev_private;
- if (!priv->kms)
- return;
-
kms = priv->kms;
if (kms->dump_worker)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index b7f5a78eadd4..9ded384acba4 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -150,9 +150,6 @@ static void msm_irq_uninstall(struct drm_device *dev)
struct msm_drm_private *priv = dev->dev_private;
struct msm_kms *kms = priv->kms;
- if (!priv->kms)
- return;
-
kms->funcs->irq_uninstall(kms);
if (kms->irq_requested)
free_irq(kms->irq, dev);
@@ -270,6 +267,8 @@ static int msm_drm_uninit(struct device *dev)
component_unbind_all(dev, ddev);
ddev->dev_private = NULL;
+ drm_dev_put(ddev);
+
destroy_workqueue(priv->wq);
return 0;
@@ -442,12 +441,12 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
ret = msm_init_vram(ddev);
if (ret)
- goto err_drm_dev_put;
+ return ret;
/* Bind all our sub-components: */
ret = component_bind_all(dev, ddev);
if (ret)
- goto err_drm_dev_put;
+ return ret;
dma_set_max_seg_size(dev, UINT_MAX);
@@ -542,8 +541,6 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
err_msm_uninit:
msm_drm_uninit(dev);
-err_drm_dev_put:
- drm_dev_put(ddev);
return ret;
}
--
2.39.2
Make sure to destroy the workqueue also in case of early errors during
bind (e.g. a subcomponent failing to bind).
Since commit c3b790ea07a1 ("drm: Manage drm_mode_config_init with
drmm_") the mode config will be freed when the drm device is released
also when using the legacy interface, but add an explicit cleanup for
consistency and to facilitate backporting.
Fixes: 060530f1ea67 ("drm/msm: use componentised device support")
Cc: [email protected] # 3.15
Cc: Rob Clark <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/gpu/drm/msm/msm_drv.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index ac3b77dbfacc..73c597565f99 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -458,7 +458,7 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
ret = msm_init_vram(ddev);
if (ret)
- goto err_put_dev;
+ goto err_cleanup_mode_config;
/* Bind all our sub-components: */
ret = component_bind_all(dev, ddev);
@@ -563,6 +563,9 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
err_deinit_vram:
msm_deinit_vram(ddev);
+err_cleanup_mode_config:
+ drm_mode_config_cleanup(ddev);
+ destroy_workqueue(priv->wq);
err_put_dev:
drm_dev_put(ddev);
--
2.39.2
Make sure to release the VRAM buffer also in a case a subcomponent fails
to bind.
Fixes: d863f0c7b536 ("drm/msm: Call msm_init_vram before binding the gpu")
Cc: [email protected] # 5.11
Cc: Craig Tatlor <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/gpu/drm/msm/msm_drv.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 89634159ad75..41cc6cd690cd 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -51,6 +51,8 @@
#define MSM_VERSION_MINOR 10
#define MSM_VERSION_PATCHLEVEL 0
+static void msm_deinit_vram(struct drm_device *ddev);
+
static const struct drm_mode_config_funcs mode_config_funcs = {
.fb_create = msm_framebuffer_create,
.output_poll_changed = drm_fb_helper_output_poll_changed,
@@ -260,12 +262,7 @@ static int msm_drm_uninit(struct device *dev)
if (kms && kms->funcs)
kms->funcs->destroy(kms);
- if (priv->vram.paddr) {
- unsigned long attrs = DMA_ATTR_NO_KERNEL_MAPPING;
- drm_mm_takedown(&priv->vram.mm);
- dma_free_attrs(dev, priv->vram.size, NULL,
- priv->vram.paddr, attrs);
- }
+ msm_deinit_vram(ddev);
component_unbind_all(dev, ddev);
@@ -403,6 +400,19 @@ static int msm_init_vram(struct drm_device *dev)
return ret;
}
+static void msm_deinit_vram(struct drm_device *ddev)
+{
+ struct msm_drm_private *priv = ddev->dev_private;
+ unsigned long attrs = DMA_ATTR_NO_KERNEL_MAPPING;
+
+ if (!priv->vram.paddr)
+ return;
+
+ drm_mm_takedown(&priv->vram.mm);
+ dma_free_attrs(ddev->dev, priv->vram.size, NULL, priv->vram.paddr,
+ attrs);
+}
+
static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
{
struct msm_drm_private *priv = dev_get_drvdata(dev);
@@ -449,7 +459,7 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
/* Bind all our sub-components: */
ret = component_bind_all(dev, ddev);
if (ret)
- goto err_put_dev;
+ goto err_deinit_vram;
dma_set_max_seg_size(dev, UINT_MAX);
@@ -547,6 +557,8 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
return ret;
+err_deinit_vram:
+ msm_deinit_vram(ddev);
err_put_dev:
drm_dev_put(ddev);
--
2.39.2
On Mon, 06 Mar 2023 18:07:13 +0800, Johan Hovold wrote:
> This reverts commit 643b7d0869cc7f1f7a5ac7ca6bd25d88f54e31d0.
The commit not only adds the allocation sanity check, but also adds the
destroy_workqueue to release the allocated priv->wq.
Therefore, revert the commit will cause memory leak.
> A recent patch that tried to fix up the msm_drm_init() paths with
> respect to the workqueue but only ended up making things worse:
>
> First, the newly added calls to msm_drm_uninit() on early errors would
> trigger NULL-pointer dereferences, for example, as the kms pointer would
> not have been initialised. (Note that these paths were also modified by
> a second broken error handling patch which in effect cancelled out this
> part when merged.)
There is a check for the kms pointer to avoid NULL-pointer dereference in
the msm_drm_uninit().
> Second, the newly added allocation sanity check would still leak the
> previously allocated drm device.
The ddev is allocated by drm_dev_alloc which support automatic cleanup.
Thanks,
Jiang
On Wed, Mar 08, 2023 at 10:10:24AM +0800, Jiasheng Jiang wrote:
> On Mon, 06 Mar 2023 18:07:13 +0800, Johan Hovold wrote:
> > This reverts commit 643b7d0869cc7f1f7a5ac7ca6bd25d88f54e31d0.
>
> The commit not only adds the allocation sanity check, but also adds the
> destroy_workqueue to release the allocated priv->wq.
> Therefore, revert the commit will cause memory leak.
No, reverting this commit does not cause any memory leaks (look at at
diff again).
The original patch called msm_drm_uninit() in some early error paths,
but that was just completely broken as that function must not be called
before the subcomponents have been bound and also triggered a bunch of
other NULL-pointer dereferences.
That bit was however removed when the change was merged with a second
branch that also touched these error paths. In the end, the leaked wq is
still there and this commit only added broken error handling for the wq
allocation failing (as it does not free the drm device).
> > A recent patch that tried to fix up the msm_drm_init() paths with
> > respect to the workqueue but only ended up making things worse:
> >
> > First, the newly added calls to msm_drm_uninit() on early errors would
> > trigger NULL-pointer dereferences, for example, as the kms pointer would
> > not have been initialised. (Note that these paths were also modified by
> > a second broken error handling patch which in effect cancelled out this
> > part when merged.)
>
> There is a check for the kms pointer to avoid NULL-pointer dereference in
> the msm_drm_uninit().
No, there were further places in msm_drm_uninit() which did not have any
such checks when you submitted your patch. Some of the missing checks
were added by a separate patch, but that would not in itself have been
sufficient as with your patch you'd still end up trying to unbind the
subcomponents before they are bound, which will lead to further crashes.
> > Second, the newly added allocation sanity check would still leak the
> > previously allocated drm device.
>
> The ddev is allocated by drm_dev_alloc which support automatic cleanup.
We don't have automatic garbage collection in the kernel. You still need
to release the reference to the device for it to be freed.
Johan
On Mon, Mar 06, 2023 at 11:07:12AM +0100, Johan Hovold wrote:
> I had reasons to look closer at the MSM DRM driver error handling and
> realised that it had suffered from a fair amount of bit rot over the
> years.
>
> Unfortunately, I started fixing this in my 6.2 branch and failed to
> notice two partial and, as it turned out, broken attempts to address
> this that are now in 6.3-rc1.
>
> Instead of trying to salvage this incrementally, I'm reverting the two
> broken commits so that clean and backportable fixes can be added in
> their place.
>
> Included are also two related cleanups.
Any further comments to these patches (except for 9/10, which should be
dropped)?
As the patches being reverted here were first added in 6.3-rc1 there is
still time to get this into 6.3-rc (e.g. before AUTOSEL starts trying to
backport them).
Johan
> Johan Hovold (10):
> Revert "drm/msm: Add missing check and destroy for
> alloc_ordered_workqueue"
> Revert "drm/msm: Fix failure paths in msm_drm_init()"
> drm/msm: fix NULL-deref on snapshot tear down
> drm/msm: fix NULL-deref on irq uninstall
> drm/msm: fix drm device leak on bind errors
> drm/msm: fix vram leak on bind errors
> drm/msm: fix missing wq allocation error handling
> drm/msm: fix workqueue leak on bind errors
> drm/msm: use drmm_mode_config_init()
> drm/msm: move include directive
>
> drivers/gpu/drm/msm/disp/msm_disp_snapshot.c | 3 -
> drivers/gpu/drm/msm/msm_drv.c | 67 +++++++++++++-------
> 2 files changed, 44 insertions(+), 26 deletions(-)
On 21/03/2023 15:02, Johan Hovold wrote:
> On Mon, Mar 06, 2023 at 11:07:12AM +0100, Johan Hovold wrote:
>> I had reasons to look closer at the MSM DRM driver error handling and
>> realised that it had suffered from a fair amount of bit rot over the
>> years.
>>
>> Unfortunately, I started fixing this in my 6.2 branch and failed to
>> notice two partial and, as it turned out, broken attempts to address
>> this that are now in 6.3-rc1.
>>
>> Instead of trying to salvage this incrementally, I'm reverting the two
>> broken commits so that clean and backportable fixes can be added in
>> their place.
>>
>> Included are also two related cleanups.
>
> Any further comments to these patches (except for 9/10, which should be
> dropped)?
>
> As the patches being reverted here were first added in 6.3-rc1 there is
> still time to get this into 6.3-rc (e.g. before AUTOSEL starts trying to
> backport them).
I will take a look at the patches. Additional question, as you have been
looking into this area. We have plenty of code which is only called
under the `if (kms)` condition. Could you hopefully move these parts to
separate functions, so that the error handling is also simpler? If not,
I'll put this to my todo list, but it might take some time before I have
time for that.
>
> Johan
>
>> Johan Hovold (10):
>> Revert "drm/msm: Add missing check and destroy for
>> alloc_ordered_workqueue"
>> Revert "drm/msm: Fix failure paths in msm_drm_init()"
>> drm/msm: fix NULL-deref on snapshot tear down
>> drm/msm: fix NULL-deref on irq uninstall
>> drm/msm: fix drm device leak on bind errors
>> drm/msm: fix vram leak on bind errors
>> drm/msm: fix missing wq allocation error handling
>> drm/msm: fix workqueue leak on bind errors
>> drm/msm: use drmm_mode_config_init()
>> drm/msm: move include directive
>>
>> drivers/gpu/drm/msm/disp/msm_disp_snapshot.c | 3 -
>> drivers/gpu/drm/msm/msm_drv.c | 67 +++++++++++++-------
>> 2 files changed, 44 insertions(+), 26 deletions(-)
--
With best wishes
Dmitry
On Tue, Mar 21, 2023 at 05:21:56PM +0200, Dmitry Baryshkov wrote:
> On 21/03/2023 15:02, Johan Hovold wrote:
> > On Mon, Mar 06, 2023 at 11:07:12AM +0100, Johan Hovold wrote:
> >> I had reasons to look closer at the MSM DRM driver error handling and
> >> realised that it had suffered from a fair amount of bit rot over the
> >> years.
> >>
> >> Unfortunately, I started fixing this in my 6.2 branch and failed to
> >> notice two partial and, as it turned out, broken attempts to address
> >> this that are now in 6.3-rc1.
> >>
> >> Instead of trying to salvage this incrementally, I'm reverting the two
> >> broken commits so that clean and backportable fixes can be added in
> >> their place.
> >>
> >> Included are also two related cleanups.
> >
> > Any further comments to these patches (except for 9/10, which should be
> > dropped)?
> >
> > As the patches being reverted here were first added in 6.3-rc1 there is
> > still time to get this into 6.3-rc (e.g. before AUTOSEL starts trying to
> > backport them).
>
> I will take a look at the patches. Additional question, as you have been
> looking into this area. We have plenty of code which is only called
> under the `if (kms)` condition. Could you hopefully move these parts to
> separate functions, so that the error handling is also simpler? If not,
> I'll put this to my todo list, but it might take some time before I have
> time for that.
There's definitely room for cleaning up the bind/unbind paths further,
but for this series I focus on correctness while maintaining symmetry
(e.g. if an allocation was done under if (kms), then the release should
be done under the same).
I don't think I will have time to look at this further for a few weeks
either, but I'll add it to my list of future work as well and I'll check
in with you before actually working on it.
Johan
On Mon, 6 Mar 2023 at 12:09, Johan Hovold <[email protected]> wrote:
>
> Make sure to release the VRAM buffer also in a case a subcomponent fails
> to bind.
>
> Fixes: d863f0c7b536 ("drm/msm: Call msm_init_vram before binding the gpu")
> Cc: [email protected] # 5.11
> Cc: Craig Tatlor <[email protected]>
> Signed-off-by: Johan Hovold <[email protected]>
> ---
> drivers/gpu/drm/msm/msm_drv.c | 26 +++++++++++++++++++-------
> 1 file changed, 19 insertions(+), 7 deletions(-)
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On 06/03/2023 12:07, Johan Hovold wrote:
> This reverts commit 643b7d0869cc7f1f7a5ac7ca6bd25d88f54e31d0.
>
> A recent patch that tried to fix up the msm_drm_init() paths with
> respect to the workqueue but only ended up making things worse:
>
> First, the newly added calls to msm_drm_uninit() on early errors would
> trigger NULL-pointer dereferences, for example, as the kms pointer would
> not have been initialised. (Note that these paths were also modified by
> a second broken error handling patch which in effect cancelled out this
> part when merged.)
>
> Second, the newly added allocation sanity check would still leak the
> previously allocated drm device.
>
> Instead of trying to salvage what was badly broken (and clearly not
> tested), let's revert the bad commit so that clean and backportable
> fixes can be added in its place.
>
> Fixes: 643b7d0869cc ("drm/msm: Add missing check and destroy for alloc_ordered_workqueue")
> Cc: Jiasheng Jiang <[email protected]>
> Signed-off-by: Johan Hovold <[email protected]>
> ---
> drivers/gpu/drm/msm/msm_drv.c | 2 --
> 1 file changed, 2 deletions(-)
>
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On 06/03/2023 12:07, Johan Hovold wrote:
> This reverts commit 8636500300a01740d92b345c680b036b94555b1b.
>
> A recent commit tried to address a drm device leak in the early
> msm_drm_uninit() error paths but ended up making things worse.
>
> Specifically, it moved the drm device reference put in msm_drm_uninit()
> to msm_drm_init() which means that the drm would now be leaked on normal
> unbind.
>
> For reasons that were never spelled out, it also added kms NULL pointer
> checks to a couple of helper functions that had nothing to do with the
> paths modified by the patch.
>
> Instead of trying to salvage this incrementally, let's revert the bad
> commit so that clean and backportable fixes can be added in its place.
>
> Fixes: 8636500300a0 ("drm/msm: Fix failure paths in msm_drm_init()")
> Cc: Akhil P Oommen <[email protected]>
> Signed-off-by: Johan Hovold <[email protected]>
Ok, let's do it this way
Reviewed-by: Dmitry Baryshkov <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/msm_disp_snapshot.c | 3 ---
> drivers/gpu/drm/msm/msm_drv.c | 11 ++++-------
> 2 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/msm_disp_snapshot.c b/drivers/gpu/drm/msm/disp/msm_disp_snapshot.c
> index b73031cd48e4..e75b97127c0d 100644
> --- a/drivers/gpu/drm/msm/disp/msm_disp_snapshot.c
> +++ b/drivers/gpu/drm/msm/disp/msm_disp_snapshot.c
> @@ -129,9 +129,6 @@ void msm_disp_snapshot_destroy(struct drm_device *drm_dev)
> }
>
> priv = drm_dev->dev_private;
> - if (!priv->kms)
> - return;
> -
> kms = priv->kms;
>
> if (kms->dump_worker)
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index b7f5a78eadd4..9ded384acba4 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -150,9 +150,6 @@ static void msm_irq_uninstall(struct drm_device *dev)
> struct msm_drm_private *priv = dev->dev_private;
> struct msm_kms *kms = priv->kms;
>
> - if (!priv->kms)
> - return;
> -
> kms->funcs->irq_uninstall(kms);
> if (kms->irq_requested)
> free_irq(kms->irq, dev);
> @@ -270,6 +267,8 @@ static int msm_drm_uninit(struct device *dev)
> component_unbind_all(dev, ddev);
>
> ddev->dev_private = NULL;
> + drm_dev_put(ddev);
> +
> destroy_workqueue(priv->wq);
>
> return 0;
> @@ -442,12 +441,12 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
>
> ret = msm_init_vram(ddev);
> if (ret)
> - goto err_drm_dev_put;
> + return ret;
>
> /* Bind all our sub-components: */
> ret = component_bind_all(dev, ddev);
> if (ret)
> - goto err_drm_dev_put;
> + return ret;
>
> dma_set_max_seg_size(dev, UINT_MAX);
>
> @@ -542,8 +541,6 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
>
> err_msm_uninit:
> msm_drm_uninit(dev);
> -err_drm_dev_put:
> - drm_dev_put(ddev);
> return ret;
> }
>
--
With best wishes
Dmitry
On 06/03/2023 12:07, Johan Hovold wrote:
> Make sure to destroy the workqueue also in case of early errors during
> bind (e.g. a subcomponent failing to bind).
>
> Since commit c3b790ea07a1 ("drm: Manage drm_mode_config_init with
> drmm_") the mode config will be freed when the drm device is released
> also when using the legacy interface, but add an explicit cleanup for
> consistency and to facilitate backporting.
>
> Fixes: 060530f1ea67 ("drm/msm: use componentised device support")
> Cc: [email protected] # 3.15
> Cc: Rob Clark <[email protected]>
> Signed-off-by: Johan Hovold <[email protected]>
> ---
> drivers/gpu/drm/msm/msm_drv.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On Mon, 06 Mar 2023 11:07:12 +0100, Johan Hovold wrote:
> I had reasons to look closer at the MSM DRM driver error handling and
> realised that it had suffered from a fair amount of bit rot over the
> years.
>
> Unfortunately, I started fixing this in my 6.2 branch and failed to
> notice two partial and, as it turned out, broken attempts to address
> this that are now in 6.3-rc1.
>
> [...]
Applied, thanks!
[01/10] Revert "drm/msm: Add missing check and destroy for alloc_ordered_workqueue"
https://gitlab.freedesktop.org/lumag/msm/-/commit/ebf761d6a02f
[02/10] Revert "drm/msm: Fix failure paths in msm_drm_init()"
https://gitlab.freedesktop.org/lumag/msm/-/commit/35a08e19a1c6
[03/10] drm/msm: fix NULL-deref on snapshot tear down
https://gitlab.freedesktop.org/lumag/msm/-/commit/d3234fe12b3b
[04/10] drm/msm: fix NULL-deref on irq uninstall
https://gitlab.freedesktop.org/lumag/msm/-/commit/0b5ffe5be6fd
[05/10] drm/msm: fix drm device leak on bind errors
https://gitlab.freedesktop.org/lumag/msm/-/commit/6a44f0dbd141
[06/10] drm/msm: fix vram leak on bind errors
https://gitlab.freedesktop.org/lumag/msm/-/commit/e6091a855649
[07/10] drm/msm: fix missing wq allocation error handling
https://gitlab.freedesktop.org/lumag/msm/-/commit/9c6027d5a3f4
[08/10] drm/msm: fix workqueue leak on bind errors
https://gitlab.freedesktop.org/lumag/msm/-/commit/023691129696
[10/10] drm/msm: move include directive
https://gitlab.freedesktop.org/lumag/msm/-/commit/110fd0d5b032
Best regards,
--
Dmitry Baryshkov <[email protected]>