Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1166265AbdDXHrs (ORCPT ); Mon, 24 Apr 2017 03:47:48 -0400 Received: from pegasos-out.vodafone.de ([80.84.1.38]:45302 "EHLO pegasos-out.vodafone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1166227AbdDXHrk (ORCPT ); Mon, 24 Apr 2017 03:47:40 -0400 X-Spam-Flag: NO X-Spam-Score: -0.045 Authentication-Results: rohrpostix2.prod.vfnet.de (amavisd-new); dkim=pass header.i=@vodafone.de X-DKIM: OpenDKIM Filter v2.6.8 pegasos-out.vodafone.de 02E0B5642B4 Subject: Re: [PATCH 1/1] drm/radeon: check return value of radeon_ring_lock To: Pan Bian , Alex Deucher , =?UTF-8?Q?Christian_K=c3=b6nig?= , David Airlie References: <1492954603-16489-1-git-send-email-bianpan201603@163.com> Cc: Pan Bian , dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org From: =?UTF-8?Q?Christian_K=c3=b6nig?= Message-ID: <325dd89e-aa25-3346-d622-c49528f4e68d@vodafone.de> Date: Mon, 24 Apr 2017 09:47:35 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <1492954603-16489-1-git-send-email-bianpan201603@163.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1899 Lines: 51 Am 23.04.2017 um 15:36 schrieb Pan Bian: > From: Pan Bian > > Function radeon_ring_lock() returns an errno on failure, and its return > value should be validated. However, in functions r420_cp_errata_init() > and r420_cp_errata_fini(), its return value is not checked. This patch > adds the checks. > > Signed-off-by: Pan Bian > --- > drivers/gpu/drm/radeon/r420.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/r420.c b/drivers/gpu/drm/radeon/r420.c > index 2828605..a8c2b37 100644 > --- a/drivers/gpu/drm/radeon/r420.c > +++ b/drivers/gpu/drm/radeon/r420.c > @@ -215,7 +215,8 @@ static void r420_cp_errata_init(struct radeon_device *rdev) > * of the CP init, apparently. > */ > radeon_scratch_get(rdev, &rdev->config.r300.resync_scratch); > - radeon_ring_lock(rdev, ring, 8); > + if (radeon_ring_lock(rdev, ring, 8)) > + return; Nice that somebody wants to clean that up, but just returning here is not a good idea. Additional to that radeon_ring_lock() can only fail if we try to allocate to many dw (impossible with only 8) or the hardware is crashed and then it doesn't matter anyway. I suggest to just add a WARN_ON() here. Regards, Christian. > radeon_ring_write(ring, PACKET0(R300_CP_RESYNC_ADDR, 1)); > radeon_ring_write(ring, rdev->config.r300.resync_scratch); > radeon_ring_write(ring, 0xDEADBEEF); > @@ -229,7 +230,8 @@ static void r420_cp_errata_fini(struct radeon_device *rdev) > /* Catch the RESYNC we dispatched all the way back, > * at the very beginning of the CP init. > */ > - radeon_ring_lock(rdev, ring, 8); > + if (radeon_ring_lock(rdev, ring, 8)) > + return; > radeon_ring_write(ring, PACKET0(R300_RB3D_DSTCACHE_CTLSTAT, 0)); > radeon_ring_write(ring, R300_RB3D_DC_FINISH); > radeon_ring_unlock_commit(rdev, ring, false);