From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Skeggs Subject: Re: [PATCH] nouveau: need to handle failed allocation Date: Fri, 29 Jan 2016 13:18:26 +1000 Message-ID: <56AADA02.9040900@gmail.com> References: <1454026152-5644-1-git-send-email-wuninsu@gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1466818102==" Return-path: Received: from mail-pa0-f46.google.com (mail-pa0-f46.google.com [209.85.220.46]) by gabe.freedesktop.org (Postfix) with ESMTPS id F0F3E6E8F5 for ; Thu, 28 Jan 2016 19:17:28 -0800 (PST) Received: by mail-pa0-f46.google.com with SMTP id yy13so33566723pab.3 for ; Thu, 28 Jan 2016 19:17:28 -0800 (PST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Insu Yun , Ilia Mirkin Cc: Yeongjin Jang , Taesoo Kim , Daniel Vetter , "Yun, Insu" , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , Thierry Reding , Changwoo Min List-Id: dri-devel@lists.freedesktop.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --===============1466818102== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="hGaaMwL66b2AxoJNxuN9EDlKqV0db6etK" This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --hGaaMwL66b2AxoJNxuN9EDlKqV0db6etK Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 01/29/2016 10:12 AM, Insu Yun wrote: >=20 > On Thu, Jan 28, 2016 at 7:08 PM, Ilia Mirkin > wrote: >=20 > On Thu, Jan 28, 2016 at 7:09 PM, Insu Yun > wrote: > > drm_property_create_range can be failed in memory pressure. > > So, it needs to be handled. > > > > Signed-off-by: Insu Yun > > > --- > > drivers/gpu/drm/nouveau/nouveau_display.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/= gpu/drm/nouveau/nouveau_display.c > > index 24be27d..26b4902 100644 > > --- a/drivers/gpu/drm/nouveau/nouveau_display.c > > +++ b/drivers/gpu/drm/nouveau/nouveau_display.c > > @@ -443,6 +443,12 @@ nouveau_display_create_properties(struct drm= _device *dev) > > /* -100..+100 */ > > disp->color_vibrance_property =3D > > drm_property_create_range(dev, 0, "color vibrance= ", 0, 200); > > + > > + if (!disp->underscan_hborder_property || > > + !disp->underscan_vborder_property || > > + !disp->vibrant_hue_property || > > + !disp->color_vibrance_property) > > + return; >=20 > Aren't we at the end of the function anyways? >=20 >=20 > Sorry. it is not perfect patch=20 > I found this by my static analyzer. > I have limited knowledge about this driver. > I don't want to mass up your driver. > What I want to do is to tell you there is a bug. > I think we need to return error to caller. I'm not so sure we do. We check for valid pointers for these when they're actually used, so no OOPS will occur. Worst case, the driver still loads correctly with some missing properties. Ben. > =20 >=20 >=20 > > } > > > > int > > -- > > 1.9.1 > > >=20 >=20 >=20 >=20 > --=20 > Regards > Insu Yun --hGaaMwL66b2AxoJNxuN9EDlKqV0db6etK Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJWqtoGAAoJEHYLnGJQkpH75Y8QAK6NgYkeO6wnla9LjI6OZ8i8 ZZQtUsab3jpwo1pEj5D2vqmC1Est7hST3ssMTWf25DinDbA15kmcUNMarbui5TWW OPIauIDhWzXe8eD9okLUmhM63Kdmc0z65bevOupx+/kLy7ohzSb0E7alUilpbQgo hGY25oDTEsABYS5pR+OEULpGKYSJI2Bm8ZiqXznKjXZHchfb2GxE8oGO/YbV188Y eOwAH9o0bqZclrR1Y7GNYrY3xy1o/KQ48Kn/LxrYW8H/JZ96IMFcDp45Iiv5XySs MY1+Zm6tpZHvWri8/tFLUE03Qx0VBCiorGPji01IuwbiMml78TT2zgs6zGwe40UM wTZHCUC4oBQf2qhfhf4CBQ6dPeP4tLBriwH9/8axz7ey8J/9bN2jUeZsEqKAy7KY diFp9F5QTGTCHKN2VHNUZZKl1J0mTCZGaUdz73Cp/VJTSYG/xM8wu0a1cMUPYamA +BeFYBpIx6nJjdejZueNBYpi2gw10AWoIzKRz7Ff/LtPUNN7dsDogr676N4+NymH 2XlVNmhunuU+ClQkN9o6YXKbhu1mqUba/IfcVGE0JTLs+hak3bdiyZwx40mbTfxy gKF0+m3NQudhPapY01LTQ5+XDYBv1/GHPFOcx5Gy3Ui08PD3tXDmNOkNY6AvIE6u 5dRyxrvAu0DCMhKat3nT =N4/U -----END PGP SIGNATURE----- --hGaaMwL66b2AxoJNxuN9EDlKqV0db6etK-- --===============1466818102== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK --===============1466818102==-- From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752747AbcA2DRa (ORCPT ); Thu, 28 Jan 2016 22:17:30 -0500 Received: from mail-pa0-f46.google.com ([209.85.220.46]:35192 "EHLO mail-pa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751228AbcA2DR3 (ORCPT ); Thu, 28 Jan 2016 22:17:29 -0500 Subject: Re: [PATCH] nouveau: need to handle failed allocation To: Insu Yun , Ilia Mirkin References: <1454026152-5644-1-git-send-email-wuninsu@gmail.com> Cc: David Airlie , Daniel Vetter , =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , maarten.lankhorst@linux.intel.com, Thierry Reding , Alexandre Courbot , "dri-devel@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" , Taesoo Kim , Yeongjin Jang , "Yun, Insu" , Changwoo Min From: Ben Skeggs Message-ID: <56AADA02.9040900@gmail.com> Date: Fri, 29 Jan 2016 13:18:26 +1000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="hGaaMwL66b2AxoJNxuN9EDlKqV0db6etK" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --hGaaMwL66b2AxoJNxuN9EDlKqV0db6etK Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 01/29/2016 10:12 AM, Insu Yun wrote: >=20 > On Thu, Jan 28, 2016 at 7:08 PM, Ilia Mirkin > wrote: >=20 > On Thu, Jan 28, 2016 at 7:09 PM, Insu Yun > wrote: > > drm_property_create_range can be failed in memory pressure. > > So, it needs to be handled. > > > > Signed-off-by: Insu Yun > > > --- > > drivers/gpu/drm/nouveau/nouveau_display.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/= gpu/drm/nouveau/nouveau_display.c > > index 24be27d..26b4902 100644 > > --- a/drivers/gpu/drm/nouveau/nouveau_display.c > > +++ b/drivers/gpu/drm/nouveau/nouveau_display.c > > @@ -443,6 +443,12 @@ nouveau_display_create_properties(struct drm= _device *dev) > > /* -100..+100 */ > > disp->color_vibrance_property =3D > > drm_property_create_range(dev, 0, "color vibrance= ", 0, 200); > > + > > + if (!disp->underscan_hborder_property || > > + !disp->underscan_vborder_property || > > + !disp->vibrant_hue_property || > > + !disp->color_vibrance_property) > > + return; >=20 > Aren't we at the end of the function anyways? >=20 >=20 > Sorry. it is not perfect patch=20 > I found this by my static analyzer. > I have limited knowledge about this driver. > I don't want to mass up your driver. > What I want to do is to tell you there is a bug. > I think we need to return error to caller. I'm not so sure we do. We check for valid pointers for these when they're actually used, so no OOPS will occur. Worst case, the driver still loads correctly with some missing properties. Ben. > =20 >=20 >=20 > > } > > > > int > > -- > > 1.9.1 > > >=20 >=20 >=20 >=20 > --=20 > Regards > Insu Yun --hGaaMwL66b2AxoJNxuN9EDlKqV0db6etK Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJWqtoGAAoJEHYLnGJQkpH75Y8QAK6NgYkeO6wnla9LjI6OZ8i8 ZZQtUsab3jpwo1pEj5D2vqmC1Est7hST3ssMTWf25DinDbA15kmcUNMarbui5TWW OPIauIDhWzXe8eD9okLUmhM63Kdmc0z65bevOupx+/kLy7ohzSb0E7alUilpbQgo hGY25oDTEsABYS5pR+OEULpGKYSJI2Bm8ZiqXznKjXZHchfb2GxE8oGO/YbV188Y eOwAH9o0bqZclrR1Y7GNYrY3xy1o/KQ48Kn/LxrYW8H/JZ96IMFcDp45Iiv5XySs MY1+Zm6tpZHvWri8/tFLUE03Qx0VBCiorGPji01IuwbiMml78TT2zgs6zGwe40UM wTZHCUC4oBQf2qhfhf4CBQ6dPeP4tLBriwH9/8axz7ey8J/9bN2jUeZsEqKAy7KY diFp9F5QTGTCHKN2VHNUZZKl1J0mTCZGaUdz73Cp/VJTSYG/xM8wu0a1cMUPYamA +BeFYBpIx6nJjdejZueNBYpi2gw10AWoIzKRz7Ff/LtPUNN7dsDogr676N4+NymH 2XlVNmhunuU+ClQkN9o6YXKbhu1mqUba/IfcVGE0JTLs+hak3bdiyZwx40mbTfxy gKF0+m3NQudhPapY01LTQ5+XDYBv1/GHPFOcx5Gy3Ui08PD3tXDmNOkNY6AvIE6u 5dRyxrvAu0DCMhKat3nT =N4/U -----END PGP SIGNATURE----- --hGaaMwL66b2AxoJNxuN9EDlKqV0db6etK--