From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julia Lawall Date: Thu, 21 Mar 2019 08:26:44 +0000 Subject: Re: [PATCH] drm/amd/powerplay: Fix double unlock bug in smu_sys_set_pp_table() Message-Id: List-Id: References: <20190321062822.GD21489@kadam> <20190321082329.GV2227@kadam> In-Reply-To: <20190321082329.GV2227@kadam> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Dan Carpenter Cc: "Zhou, David(ChunMing)" , "Gui, Jack" , David Airlie , "Wang, Kevin(Yang)" , "kernel-janitors-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org" , Julia Lawall , "Huang, Ray" , Daniel Vetter , "Deucher, Alexander" , "Gao, Likun" , "Koenig, Christian" On Thu, 21 Mar 2019, Dan Carpenter wrote: > On Thu, Mar 21, 2019 at 09:20:38AM +0100, Julia Lawall wrote: > > > > > > On Thu, 21 Mar 2019, Huang, Ray wrote: > > > > > > -----Original Message----- > > > > From: Dan Carpenter [mailto:dan.carpenter@oracle.com] > > > > Sent: Thursday, March 21, 2019 2:28 PM > > > > To: Deucher, Alexander ; Wang, Kevin(Yang) > > > > > > > > Cc: Koenig, Christian ; Zhou, David(ChunMing) > > > > ; David Airlie ; Daniel Vetter > > > > ; Huang, Ray ; Gao, Likun > > > > ; Gui, Jack ; amd- > > > > gfx@lists.freedesktop.org; kernel-janitors@vger.kernel.org > > > > Subject: [PATCH] drm/amd/powerplay: Fix double unlock bug in > > > > smu_sys_set_pp_table() > > > > > > > > We already unlocked a few lines earlier so this code unlocks twice on the > > > > success path. > > > > > > > > Fixes: 5809d7420f97 ("drm/amd/powerplay: implement sysfs of pp_table for > > > > smu11 (v2)") > > > > Signed-off-by: Dan Carpenter > > > > > > Nice catch! Thanks, Dan. > > > Kevin, could you please verify this patch. > > > Reviewed-by: Huang Rui > > > > > > > --- > > > > I'm not sure what this bug looks like at runtime, but it's slightly weird that no > > > > one noticed. This is from static analysis and I haven't tested it myself. > > > > > > > > drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c > > > > b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c > > > > index 00b7c885772b..7e8c74da6a74 100644 > > > > --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c > > > > +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c > > > > @@ -187,6 +187,8 @@ int smu_sys_set_pp_table(struct smu_context *smu, > > > > void *buf, size_t size) > > > > if (ret) > > > > pr_info("smu reset failed, ret = %d\n", ret); > > > > > > > > + return ret; > > > > Why not return 0? > > It's not necessarily zero. Oops, I was looking at the invisble goto after the pr_info :) julia From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julia Lawall Subject: Re: [PATCH] drm/amd/powerplay: Fix double unlock bug in smu_sys_set_pp_table() Date: Thu, 21 Mar 2019 09:26:44 +0100 (CET) Message-ID: References: <20190321062822.GD21489@kadam> <20190321082329.GV2227@kadam> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20190321082329.GV2227@kadam> List-Id: Discussion list for AMD gfx List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Sender: "amd-gfx" To: Dan Carpenter Cc: "Zhou, David(ChunMing)" , "Gui, Jack" , David Airlie , "Wang, Kevin(Yang)" , "kernel-janitors-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org" , Julia Lawall , "Huang, Ray" , Daniel Vetter , "Deucher, Alexander" , "Gao, Likun" , "Koenig, Christian" CgpPbiBUaHUsIDIxIE1hciAyMDE5LCBEYW4gQ2FycGVudGVyIHdyb3RlOgoKPiBPbiBUaHUsIE1h ciAyMSwgMjAxOSBhdCAwOToyMDozOEFNICswMTAwLCBKdWxpYSBMYXdhbGwgd3JvdGU6Cj4gPgo+ ID4KPiA+IE9uIFRodSwgMjEgTWFyIDIwMTksIEh1YW5nLCBSYXkgd3JvdGU6Cj4gPgo+ID4gPiA+ IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tCj4gPiA+ID4gRnJvbTogRGFuIENhcnBlbnRlciBb bWFpbHRvOmRhbi5jYXJwZW50ZXJAb3JhY2xlLmNvbV0KPiA+ID4gPiBTZW50OiBUaHVyc2RheSwg TWFyY2ggMjEsIDIwMTkgMjoyOCBQTQo+ID4gPiA+IFRvOiBEZXVjaGVyLCBBbGV4YW5kZXIgPEFs ZXhhbmRlci5EZXVjaGVyQGFtZC5jb20+OyBXYW5nLCBLZXZpbihZYW5nKQo+ID4gPiA+IDxLZXZp bjEuV2FuZ0BhbWQuY29tPgo+ID4gPiA+IENjOiBLb2VuaWcsIENocmlzdGlhbiA8Q2hyaXN0aWFu LktvZW5pZ0BhbWQuY29tPjsgWmhvdSwgRGF2aWQoQ2h1bk1pbmcpCj4gPiA+ID4gPERhdmlkMS5a aG91QGFtZC5jb20+OyBEYXZpZCBBaXJsaWUgPGFpcmxpZWRAbGludXguaWU+OyBEYW5pZWwgVmV0 dGVyCj4gPiA+ID4gPGRhbmllbEBmZndsbC5jaD47IEh1YW5nLCBSYXkgPFJheS5IdWFuZ0BhbWQu Y29tPjsgR2FvLCBMaWt1bgo+ID4gPiA+IDxMaWt1bi5HYW9AYW1kLmNvbT47IEd1aSwgSmFjayA8 SmFjay5HdWlAYW1kLmNvbT47IGFtZC0KPiA+ID4gPiBnZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3Jn OyBrZXJuZWwtamFuaXRvcnNAdmdlci5rZXJuZWwub3JnCj4gPiA+ID4gU3ViamVjdDogW1BBVENI XSBkcm0vYW1kL3Bvd2VycGxheTogRml4IGRvdWJsZSB1bmxvY2sgYnVnIGluCj4gPiA+ID4gc211 X3N5c19zZXRfcHBfdGFibGUoKQo+ID4gPiA+Cj4gPiA+ID4gV2UgYWxyZWFkeSB1bmxvY2tlZCBh IGZldyBsaW5lcyBlYXJsaWVyIHNvIHRoaXMgY29kZSB1bmxvY2tzIHR3aWNlIG9uIHRoZQo+ID4g PiA+IHN1Y2Nlc3MgcGF0aC4KPiA+ID4gPgo+ID4gPiA+IEZpeGVzOiA1ODA5ZDc0MjBmOTcgKCJk cm0vYW1kL3Bvd2VycGxheTogaW1wbGVtZW50IHN5c2ZzIG9mIHBwX3RhYmxlIGZvcgo+ID4gPiA+ IHNtdTExICh2MikiKQo+ID4gPiA+IFNpZ25lZC1vZmYtYnk6IERhbiBDYXJwZW50ZXIgPGRhbi5j YXJwZW50ZXJAb3JhY2xlLmNvbT4KPiA+ID4KPiA+ID4gTmljZSBjYXRjaCEgIFRoYW5rcywgRGFu Lgo+ID4gPiBLZXZpbiwgY291bGQgeW91IHBsZWFzZSB2ZXJpZnkgdGhpcyBwYXRjaC4KPiA+ID4g UmV2aWV3ZWQtYnk6IEh1YW5nIFJ1aSA8cmF5Lmh1YW5nQGFtZC5jb20+Cj4gPiA+Cj4gPiA+ID4g LS0tCj4gPiA+ID4gSSdtIG5vdCBzdXJlIHdoYXQgdGhpcyBidWcgbG9va3MgbGlrZSBhdCBydW50 aW1lLCBidXQgaXQncyBzbGlnaHRseSB3ZWlyZCB0aGF0IG5vCj4gPiA+ID4gb25lIG5vdGljZWQu ICBUaGlzIGlzIGZyb20gc3RhdGljIGFuYWx5c2lzIGFuZCBJIGhhdmVuJ3QgdGVzdGVkIGl0IG15 c2VsZi4KPiA+ID4gPgo+ID4gPiA+ICBkcml2ZXJzL2dwdS9kcm0vYW1kL3Bvd2VycGxheS9hbWRn cHVfc211LmMgfCAyICsrCj4gPiA+ID4gIDEgZmlsZSBjaGFuZ2VkLCAyIGluc2VydGlvbnMoKykK PiA+ID4gPgo+ID4gPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vYW1kL3Bvd2VycGxh eS9hbWRncHVfc211LmMKPiA+ID4gPiBiL2RyaXZlcnMvZ3B1L2RybS9hbWQvcG93ZXJwbGF5L2Ft ZGdwdV9zbXUuYwo+ID4gPiA+IGluZGV4IDAwYjdjODg1NzcyYi4uN2U4Yzc0ZGE2YTc0IDEwMDY0 NAo+ID4gPiA+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9hbWQvcG93ZXJwbGF5L2FtZGdwdV9zbXUu Ywo+ID4gPiA+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9hbWQvcG93ZXJwbGF5L2FtZGdwdV9zbXUu Ywo+ID4gPiA+IEBAIC0xODcsNiArMTg3LDggQEAgaW50IHNtdV9zeXNfc2V0X3BwX3RhYmxlKHN0 cnVjdCBzbXVfY29udGV4dCAqc211LAo+ID4gPiA+IHZvaWQgKmJ1Ziwgc2l6ZV90IHNpemUpCj4g PiA+ID4gIAlpZiAocmV0KQo+ID4gPiA+ICAJCXByX2luZm8oInNtdSByZXNldCBmYWlsZWQsIHJl dCA9ICVkXG4iLCByZXQpOwo+ID4gPiA+Cj4gPiA+ID4gKwlyZXR1cm4gcmV0Owo+ID4KPiA+IFdo eSBub3QgcmV0dXJuIDA/Cj4KPiBJdCdzIG5vdCBuZWNlc3NhcmlseSB6ZXJvLgoKT29wcywgSSB3 YXMgbG9va2luZyBhdCB0aGUgaW52aXNibGUgZ290byBhZnRlciB0aGUgcHJfaW5mbyA6KQoKanVs aWEKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KYW1kLWdm eCBtYWlsaW5nIGxpc3QKYW1kLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9hbWQtZ2Z4