Re: [PATCH 00/10] drm/msm: fix bind error handling

From: Johan Hovold <johan@kernel.org>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Rob Clark <robdclark@gmail.com>,
	Abhinav Kumar <quic_abhinavk@quicinc.com>,
	Sean Paul <sean@poorly.run>, David Airlie <airlied@gmail.com>,
	Daniel Vetter <daniel@ffwll.ch>,
	linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
	freedreno@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 00/10] drm/msm: fix bind error handling
Date: Wed, 22 Mar 2023 08:54:53 +0100	[thread overview]
Message-ID: <ZBq0TaDON8iJ3/Rh@hovoldconsulting.com> (raw)
In-Reply-To: <6b2f4d0d-ba39-dd2b-e41e-b5accb4df1a2@linaro.org>

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