From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julia Lawall Date: Thu, 21 Mar 2019 08:20:38 +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> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: "Huang, Ray" Cc: "Zhou, David(ChunMing)" , "Gui, Jack" , David Airlie , "Wang, Kevin(Yang)" , "kernel-janitors-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org" , Daniel Vetter , "Deucher, Alexander" , "Gao, Likun" , "Koenig, Christian" , Dan Carpenter 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? julia > > + > > failed: > > mutex_unlock(&smu->mutex); > > return ret; > > -- > > 2.17.1 > > 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:20:38 +0100 (CET) Message-ID: References: <20190321062822.GD21489@kadam> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: 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: "Huang, Ray" Cc: "Zhou, David(ChunMing)" , "Gui, Jack" , David Airlie , "Wang, Kevin(Yang)" , "kernel-janitors-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org" , Daniel Vetter , "Deucher, Alexander" , "Gao, Likun" , "Koenig, Christian" , Dan Carpenter CgpPbiBUaHUsIDIxIE1hciAyMDE5LCBIdWFuZywgUmF5IHdyb3RlOgoKPiA+IC0tLS0tT3JpZ2lu YWwgTWVzc2FnZS0tLS0tCj4gPiBGcm9tOiBEYW4gQ2FycGVudGVyIFttYWlsdG86ZGFuLmNhcnBl bnRlckBvcmFjbGUuY29tXQo+ID4gU2VudDogVGh1cnNkYXksIE1hcmNoIDIxLCAyMDE5IDI6Mjgg UE0KPiA+IFRvOiBEZXVjaGVyLCBBbGV4YW5kZXIgPEFsZXhhbmRlci5EZXVjaGVyQGFtZC5jb20+ OyBXYW5nLCBLZXZpbihZYW5nKQo+ID4gPEtldmluMS5XYW5nQGFtZC5jb20+Cj4gPiBDYzogS29l bmlnLCBDaHJpc3RpYW4gPENocmlzdGlhbi5Lb2VuaWdAYW1kLmNvbT47IFpob3UsIERhdmlkKENo dW5NaW5nKQo+ID4gPERhdmlkMS5aaG91QGFtZC5jb20+OyBEYXZpZCBBaXJsaWUgPGFpcmxpZWRA bGludXguaWU+OyBEYW5pZWwgVmV0dGVyCj4gPiA8ZGFuaWVsQGZmd2xsLmNoPjsgSHVhbmcsIFJh eSA8UmF5Lkh1YW5nQGFtZC5jb20+OyBHYW8sIExpa3VuCj4gPiA8TGlrdW4uR2FvQGFtZC5jb20+ OyBHdWksIEphY2sgPEphY2suR3VpQGFtZC5jb20+OyBhbWQtCj4gPiBnZnhAbGlzdHMuZnJlZWRl c2t0b3Aub3JnOyBrZXJuZWwtamFuaXRvcnNAdmdlci5rZXJuZWwub3JnCj4gPiBTdWJqZWN0OiBb UEFUQ0hdIGRybS9hbWQvcG93ZXJwbGF5OiBGaXggZG91YmxlIHVubG9jayBidWcgaW4KPiA+IHNt dV9zeXNfc2V0X3BwX3RhYmxlKCkKPiA+Cj4gPiBXZSBhbHJlYWR5IHVubG9ja2VkIGEgZmV3IGxp bmVzIGVhcmxpZXIgc28gdGhpcyBjb2RlIHVubG9ja3MgdHdpY2Ugb24gdGhlCj4gPiBzdWNjZXNz IHBhdGguCj4gPgo+ID4gRml4ZXM6IDU4MDlkNzQyMGY5NyAoImRybS9hbWQvcG93ZXJwbGF5OiBp bXBsZW1lbnQgc3lzZnMgb2YgcHBfdGFibGUgZm9yCj4gPiBzbXUxMSAodjIpIikKPiA+IFNpZ25l ZC1vZmYtYnk6IERhbiBDYXJwZW50ZXIgPGRhbi5jYXJwZW50ZXJAb3JhY2xlLmNvbT4KPgo+IE5p Y2UgY2F0Y2ghICBUaGFua3MsIERhbi4KPiBLZXZpbiwgY291bGQgeW91IHBsZWFzZSB2ZXJpZnkg dGhpcyBwYXRjaC4KPiBSZXZpZXdlZC1ieTogSHVhbmcgUnVpIDxyYXkuaHVhbmdAYW1kLmNvbT4K Pgo+ID4gLS0tCj4gPiBJJ20gbm90IHN1cmUgd2hhdCB0aGlzIGJ1ZyBsb29rcyBsaWtlIGF0IHJ1 bnRpbWUsIGJ1dCBpdCdzIHNsaWdodGx5IHdlaXJkIHRoYXQgbm8KPiA+IG9uZSBub3RpY2VkLiAg VGhpcyBpcyBmcm9tIHN0YXRpYyBhbmFseXNpcyBhbmQgSSBoYXZlbid0IHRlc3RlZCBpdCBteXNl bGYuCj4gPgo+ID4gIGRyaXZlcnMvZ3B1L2RybS9hbWQvcG93ZXJwbGF5L2FtZGdwdV9zbXUuYyB8 IDIgKysKPiA+ICAxIGZpbGUgY2hhbmdlZCwgMiBpbnNlcnRpb25zKCspCj4gPgo+ID4gZGlmZiAt LWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9hbWQvcG93ZXJwbGF5L2FtZGdwdV9zbXUuYwo+ID4gYi9k cml2ZXJzL2dwdS9kcm0vYW1kL3Bvd2VycGxheS9hbWRncHVfc211LmMKPiA+IGluZGV4IDAwYjdj ODg1NzcyYi4uN2U4Yzc0ZGE2YTc0IDEwMDY0NAo+ID4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2Ft ZC9wb3dlcnBsYXkvYW1kZ3B1X3NtdS5jCj4gPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vYW1kL3Bv d2VycGxheS9hbWRncHVfc211LmMKPiA+IEBAIC0xODcsNiArMTg3LDggQEAgaW50IHNtdV9zeXNf c2V0X3BwX3RhYmxlKHN0cnVjdCBzbXVfY29udGV4dCAqc211LAo+ID4gdm9pZCAqYnVmLCBzaXpl X3Qgc2l6ZSkKPiA+ICAJaWYgKHJldCkKPiA+ICAJCXByX2luZm8oInNtdSByZXNldCBmYWlsZWQs IHJldCA9ICVkXG4iLCByZXQpOwo+ID4KPiA+ICsJcmV0dXJuIHJldDsKCldoeSBub3QgcmV0dXJu IDA/CgpqdWxpYQoKPiA+ICsKPiA+ICBmYWlsZWQ6Cj4gPiAgCW11dGV4X3VubG9jaygmc211LT5t dXRleCk7Cj4gPiAgCXJldHVybiByZXQ7Cj4gPiAtLQo+ID4gMi4xNy4xCj4KPgpfX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwphbWQtZ2Z4IG1haWxpbmcgbGlz dAphbWQtZ2Z4QGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9w Lm9yZy9tYWlsbWFuL2xpc3RpbmZvL2FtZC1nZng=