From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966177AbbBCGB1 (ORCPT ); Tue, 3 Feb 2015 01:01:27 -0500 Received: from blu004-omc2s15.hotmail.com ([65.55.111.90]:63984 "EHLO BLU004-OMC2S15.hotmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965102AbbBCGB0 (ORCPT ); Tue, 3 Feb 2015 01:01:26 -0500 X-Greylist: delayed 301 seconds by postgrey-1.27 at vger.kernel.org; Tue, 03 Feb 2015 01:01:25 EST X-TMN: [rvN/ayavErG028x17FrwBl//oVEL9D6I] X-Originating-Email: [junjie.mao@hotmail.com] Message-ID: From: Junjie Mao To: Guenter Roeck CC: Mauro Carvalho Chehab , Doug Thompson , Borislav Petkov , linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] i7core_edac: release resources on error in i7core_create_sysfs_devices References: <20150203041532.GA27380@roeck-us.net> Date: Tue, 3 Feb 2015 13:56:16 +0800 In-Reply-To: <20150203041532.GA27380@roeck-us.net> (Guenter Roeck's message of "Mon, 2 Feb 2015 20:15:32 -0800") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (windows-nt) MIME-Version: 1.0 Content-Type: text/plain X-OriginalArrivalTime: 03 Feb 2015 05:56:23.0037 (UTC) FILETIME=[23A3C2D0:01D03F76] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Guenter Roeck writes: > On Tue, Feb 03, 2015 at 10:40:14AM +0800, Junjie Mao wrote: >> v2: do not call kfree on allocated devs after device_initialize is called >> >> Signed-off-by: Junjie Mao >> --- >> drivers/edac/i7core_edac.c | 37 +++++++++++++++++++++++++------------ >> 1 file changed, 25 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c >> index 9cd0b301f81b..f6a7b676d69c 100644 >> --- a/drivers/edac/i7core_edac.c >> +++ b/drivers/edac/i7core_edac.c >> @@ -1167,17 +1167,17 @@ static int i7core_create_sysfs_devices(struct mem_ctl_info *mci) >> return rc; >> rc = device_create_file(&mci->dev, &dev_attr_inject_type); >> if (rc < 0) >> - return rc; >> + goto fail_remove_inject_section; >> rc = device_create_file(&mci->dev, &dev_attr_inject_eccmask); >> if (rc < 0) >> - return rc; >> + goto fail_remove_inject_type; >> rc = device_create_file(&mci->dev, &dev_attr_inject_enable); >> if (rc < 0) >> - return rc; >> + goto fail_remove_inject_eccmask; >> >> pvt->addrmatch_dev = kzalloc(sizeof(*pvt->addrmatch_dev), GFP_KERNEL); >> if (!pvt->addrmatch_dev) >> - return rc; >> + goto fail_remove_inject_enable; >> >> pvt->addrmatch_dev->type = &addrmatch_type; >> pvt->addrmatch_dev->bus = mci->dev.bus; >> @@ -1190,16 +1190,13 @@ static int i7core_create_sysfs_devices(struct mem_ctl_info *mci) >> >> rc = device_add(pvt->addrmatch_dev); >> if (rc < 0) >> - return rc; >> + goto fail_free_addrmatch_dev; >> >> if (!pvt->is_registered) { >> pvt->chancounts_dev = kzalloc(sizeof(*pvt->chancounts_dev), >> GFP_KERNEL); >> - if (!pvt->chancounts_dev) { >> - put_device(pvt->addrmatch_dev); >> - device_del(pvt->addrmatch_dev); >> - return rc; >> - } >> + if (!pvt->chancounts_dev) >> + goto fail_del_addrmatch_dev; >> >> pvt->chancounts_dev->type = &all_channel_counts_type; >> pvt->chancounts_dev->bus = mci->dev.bus; >> @@ -1211,10 +1208,26 @@ static int i7core_create_sysfs_devices(struct mem_ctl_info *mci) >> edac_dbg(1, "creating %s\n", dev_name(pvt->chancounts_dev)); >> >> rc = device_add(pvt->chancounts_dev); >> - if (rc < 0) >> - return rc; >> + if (rc < 0) { >> + put_device(pvt->chancounts_dev); >> + goto fail_del_addrmatch_dev; >> + } >> } >> return 0; >> + >> +fail_del_addrmatch_dev: >> + device_del(pvt->addrmatch_dev); >> +fail_free_addrmatch_dev: >> + put_device(pvt->addrmatch_dev); >> +fail_remove_inject_enable: >> + device_create_file(&mci->dev, &dev_attr_inject_enable); >> +fail_remove_inject_eccmask: >> + device_create_file(&mci->dev, &dev_attr_inject_eccmask); >> +fail_remove_inject_type: >> + device_create_file(&mci->dev, &dev_attr_inject_type); >> +fail_remove_inject_section: >> + device_create_file(&mci->dev, &dev_attr_inject_section); > > I don't know the code, but calling device_create_file() on failures > is quite unusual. Are you sure this is correct ? > > Guenter I should call device_remove_file here. Terribly sorry for the carelessness. Best Regards Junjie Mao