From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH 15/17] drm/tegra: Fix potential bug on driver unload Date: Tue, 4 Nov 2014 13:30:34 +0100 Message-ID: <20141104123031.GA23240@ulmo.nvidia.com> References: <1415006868-318-1-git-send-email-thierry.reding@gmail.com> <1415006868-318-15-git-send-email-thierry.reding@gmail.com> <5458B1A2.1020309@samsung.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1599249999==" Return-path: Received: from mail-pd0-f171.google.com (mail-pd0-f171.google.com [209.85.192.171]) by gabe.freedesktop.org (Postfix) with ESMTP id 634476E5EB for ; Tue, 4 Nov 2014 04:30:40 -0800 (PST) Received: by mail-pd0-f171.google.com with SMTP id r10so13608714pdi.30 for ; Tue, 04 Nov 2014 04:30:40 -0800 (PST) In-Reply-To: <5458B1A2.1020309@samsung.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Andrzej Hajda Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1599249999== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="XsQoSWH+UP9D9v3l" Content-Disposition: inline --XsQoSWH+UP9D9v3l Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Nov 04, 2014 at 11:59:46AM +0100, Andrzej Hajda wrote: > Hi Thierry, >=20 > Just passing by. >=20 > On 11/03/2014 10:27 AM, Thierry Reding wrote: > > From: Thierry Reding > >=20 > > The HDMI hotplug signal may toggle after the DRM driver has been > > unloaded. Make sure not to call into DRM if that's the case. > >=20 > > Signed-off-by: Thierry Reding > > --- > > drivers/gpu/drm/tegra/output.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > >=20 > > diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/out= put.c > > index 6b393cfbb5e7..def74914dd72 100644 > > --- a/drivers/gpu/drm/tegra/output.c > > +++ b/drivers/gpu/drm/tegra/output.c > > @@ -181,7 +181,8 @@ static irqreturn_t hpd_irq(int irq, void *data) > > { > > struct tegra_output *output =3D data; > > =20 > > - drm_helper_hpd_irq_event(output->connector.dev); > > + if (output->connector.dev) > > + drm_helper_hpd_irq_event(output->connector.dev); > > =20 > > return IRQ_HANDLED; > > } > >=20 >=20 > Since output->connector.dev is not synchronized between irq and other > code this patch do not solves the issue, it just decreases chances of > the disaster. You're right. I guess in addition to this we could call enable_irq() when the connector is bound to the DRM device and disable_irq() when it is unbound. Actually, that should even allow drm_helper_hpd_irq_event() to be called unconditionally because .dev could not be NULL in that case. Upon closer inspection it seems like the majority of drivers would fall prey to this particular race. Thierry --XsQoSWH+UP9D9v3l Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJUWMbnAAoJEN0jrNd/PrOhKK0QALkCwgmNDTIvUV/CHSGyeW5k E3l3Ao/Uqao73PpTRBNjqyw8hecHz8jMfuQgy5uRSxyD6fsQd86RVpaIicvSnwjn yrm3adRJ7JcE6ScgByZZlD/R+359BU1HLzUd1sXdqQUABevlEcHQtKDJRV/k1z5o svbtUW/kw+pP2lOIK6v/5aK1zAg95zK2j5Rqx6fgW1TwWqt01eqdAVEQOqBgSDvi q3U4K3iQ+WUuwX4775qd4/H56KV+qiSGVQzn4WRRcelfP33brFmfgasQUlMRPvsy m/J1Hp1FuhdueJ1nk30xwoe9yswzrIyITA3kU/6LInfc+LVmQr0C4zGixYH7zmzo DDJ3LXbr/+LgsdLl/vKdCDx8TevEmGsTTfWGDkkCDUbjs/FMgvz0lo1RSw2X0+Q8 pcfpnH0hLcHymnYKkmJ0eaHECy+33RQ+IQMsTWXvujOxJBvKnXkLazIXmDCNWV1c F0VxuWfHOCsDWFz/8rdtWQ7V4ipfqhBQ9VfJcY5WwaxQze73sOf80ugzKVGh7pYw Wu3Dxv5XCPPmbdkmJDy03T70Mms5OmlADNYez2C+3LyV+s7kFJibVhhdHdkj0k5j joiOMz6ruAts9kao4C2DCI+SN0Yj4r2nh7JKzoEEwU6+zOyvSKiMyfK7rdNtytO6 wY/49ZRG8IuSko6ud/1E =O5+r -----END PGP SIGNATURE----- --XsQoSWH+UP9D9v3l-- --===============1599249999== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK --===============1599249999==--