From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Carpenter Date: Thu, 21 Mar 2019 08:23:29 +0000 Subject: Re: [PATCH] drm/amd/powerplay: Fix double unlock bug in smu_sys_set_pp_table() Message-Id: <20190321082329.GV2227@kadam> List-Id: References: <20190321062822.GD21489@kadam> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Julia Lawall Cc: "Zhou, David(ChunMing)" , "Gui, Jack" , David Airlie , "Wang, Kevin(Yang)" , "kernel-janitors-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org" , "Huang, Ray" , Daniel Vetter , "Deucher, Alexander" , "Gao, Likun" , "Koenig, Christian" 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. regards, dan carpenter From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Carpenter Subject: Re: [PATCH] drm/amd/powerplay: Fix double unlock bug in smu_sys_set_pp_table() Date: Thu, 21 Mar 2019 11:23:29 +0300 Message-ID: <20190321082329.GV2227@kadam> References: <20190321062822.GD21489@kadam> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: 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: Julia Lawall Cc: "Zhou, David(ChunMing)" , "Gui, Jack" , David Airlie , "Wang, Kevin(Yang)" , "kernel-janitors-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org" , "Huang, Ray" , Daniel Vetter , "Deucher, Alexander" , "Gao, Likun" , "Koenig, Christian" T24gVGh1LCBNYXIgMjEsIDIwMTkgYXQgMDk6MjA6MzhBTSArMDEwMCwgSnVsaWEgTGF3YWxsIHdy b3RlOgo+IAo+IAo+IE9uIFRodSwgMjEgTWFyIDIwMTksIEh1YW5nLCBSYXkgd3JvdGU6Cj4gCj4g PiA+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tCj4gPiA+IEZyb206IERhbiBDYXJwZW50ZXIg W21haWx0bzpkYW4uY2FycGVudGVyQG9yYWNsZS5jb21dCj4gPiA+IFNlbnQ6IFRodXJzZGF5LCBN YXJjaCAyMSwgMjAxOSAyOjI4IFBNCj4gPiA+IFRvOiBEZXVjaGVyLCBBbGV4YW5kZXIgPEFsZXhh bmRlci5EZXVjaGVyQGFtZC5jb20+OyBXYW5nLCBLZXZpbihZYW5nKQo+ID4gPiA8S2V2aW4xLldh bmdAYW1kLmNvbT4KPiA+ID4gQ2M6IEtvZW5pZywgQ2hyaXN0aWFuIDxDaHJpc3RpYW4uS29lbmln QGFtZC5jb20+OyBaaG91LCBEYXZpZChDaHVuTWluZykKPiA+ID4gPERhdmlkMS5aaG91QGFtZC5j b20+OyBEYXZpZCBBaXJsaWUgPGFpcmxpZWRAbGludXguaWU+OyBEYW5pZWwgVmV0dGVyCj4gPiA+ IDxkYW5pZWxAZmZ3bGwuY2g+OyBIdWFuZywgUmF5IDxSYXkuSHVhbmdAYW1kLmNvbT47IEdhbywg TGlrdW4KPiA+ID4gPExpa3VuLkdhb0BhbWQuY29tPjsgR3VpLCBKYWNrIDxKYWNrLkd1aUBhbWQu Y29tPjsgYW1kLQo+ID4gPiBnZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnOyBrZXJuZWwtamFuaXRv cnNAdmdlci5rZXJuZWwub3JnCj4gPiA+IFN1YmplY3Q6IFtQQVRDSF0gZHJtL2FtZC9wb3dlcnBs YXk6IEZpeCBkb3VibGUgdW5sb2NrIGJ1ZyBpbgo+ID4gPiBzbXVfc3lzX3NldF9wcF90YWJsZSgp Cj4gPiA+Cj4gPiA+IFdlIGFscmVhZHkgdW5sb2NrZWQgYSBmZXcgbGluZXMgZWFybGllciBzbyB0 aGlzIGNvZGUgdW5sb2NrcyB0d2ljZSBvbiB0aGUKPiA+ID4gc3VjY2VzcyBwYXRoLgo+ID4gPgo+ ID4gPiBGaXhlczogNTgwOWQ3NDIwZjk3ICgiZHJtL2FtZC9wb3dlcnBsYXk6IGltcGxlbWVudCBz eXNmcyBvZiBwcF90YWJsZSBmb3IKPiA+ID4gc211MTEgKHYyKSIpCj4gPiA+IFNpZ25lZC1vZmYt Ynk6IERhbiBDYXJwZW50ZXIgPGRhbi5jYXJwZW50ZXJAb3JhY2xlLmNvbT4KPiA+Cj4gPiBOaWNl IGNhdGNoISAgVGhhbmtzLCBEYW4uCj4gPiBLZXZpbiwgY291bGQgeW91IHBsZWFzZSB2ZXJpZnkg dGhpcyBwYXRjaC4KPiA+IFJldmlld2VkLWJ5OiBIdWFuZyBSdWkgPHJheS5odWFuZ0BhbWQuY29t Pgo+ID4KPiA+ID4gLS0tCj4gPiA+IEknbSBub3Qgc3VyZSB3aGF0IHRoaXMgYnVnIGxvb2tzIGxp a2UgYXQgcnVudGltZSwgYnV0IGl0J3Mgc2xpZ2h0bHkgd2VpcmQgdGhhdCBubwo+ID4gPiBvbmUg bm90aWNlZC4gIFRoaXMgaXMgZnJvbSBzdGF0aWMgYW5hbHlzaXMgYW5kIEkgaGF2ZW4ndCB0ZXN0 ZWQgaXQgbXlzZWxmLgo+ID4gPgo+ID4gPiAgZHJpdmVycy9ncHUvZHJtL2FtZC9wb3dlcnBsYXkv YW1kZ3B1X3NtdS5jIHwgMiArKwo+ID4gPiAgMSBmaWxlIGNoYW5nZWQsIDIgaW5zZXJ0aW9ucygr KQo+ID4gPgo+ID4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2FtZC9wb3dlcnBsYXkv YW1kZ3B1X3NtdS5jCj4gPiA+IGIvZHJpdmVycy9ncHUvZHJtL2FtZC9wb3dlcnBsYXkvYW1kZ3B1 X3NtdS5jCj4gPiA+IGluZGV4IDAwYjdjODg1NzcyYi4uN2U4Yzc0ZGE2YTc0IDEwMDY0NAo+ID4g PiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vYW1kL3Bvd2VycGxheS9hbWRncHVfc211LmMKPiA+ID4g KysrIGIvZHJpdmVycy9ncHUvZHJtL2FtZC9wb3dlcnBsYXkvYW1kZ3B1X3NtdS5jCj4gPiA+IEBA IC0xODcsNiArMTg3LDggQEAgaW50IHNtdV9zeXNfc2V0X3BwX3RhYmxlKHN0cnVjdCBzbXVfY29u dGV4dCAqc211LAo+ID4gPiB2b2lkICpidWYsIHNpemVfdCBzaXplKQo+ID4gPiAgCWlmIChyZXQp Cj4gPiA+ICAJCXByX2luZm8oInNtdSByZXNldCBmYWlsZWQsIHJldCA9ICVkXG4iLCByZXQpOwo+ ID4gPgo+ID4gPiArCXJldHVybiByZXQ7Cj4gCj4gV2h5IG5vdCByZXR1cm4gMD8KCkl0J3Mgbm90 IG5lY2Vzc2FyaWx5IHplcm8uCgpyZWdhcmRzLApkYW4gY2FycGVudGVyCgpfX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwphbWQtZ2Z4IG1haWxpbmcgbGlzdAph bWQtZ2Z4QGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9y Zy9tYWlsbWFuL2xpc3RpbmZvL2FtZC1nZng=