From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755558AbaCRNeT (ORCPT ); Tue, 18 Mar 2014 09:34:19 -0400 Received: from e06smtp17.uk.ibm.com ([195.75.94.113]:41269 "EHLO e06smtp17.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755379AbaCRNeR (ORCPT ); Tue, 18 Mar 2014 09:34:17 -0400 Date: Tue, 18 Mar 2014 14:34:07 +0100 From: Gerald Schaefer To: Zhouyi Zhou Cc: "linux-kernel@vger.kernel.org" , linux-s390@vger.kernel.org, joe@perches.com, gang.chen@asianux.com, linux390@de.ibm.com, heiko.carstens@de.ibm.com, schwidefsky@de.ibm.com, Zhouyi Zhou Subject: Re: [PATCH] s390: correct misuses of module_put in appldata_generic_handler. Message-ID: <20140318143407.0df4457f@thinkpad> In-Reply-To: References: <1394890540-2435-1-git-send-email-zhouzhouyi@gmail.com> <20140317142851.50e5f533@thinkpad> X-Mailer: Claws Mail 3.8.1 (GTK+ 2.24.13; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14031813-0542-0000-0000-0000085D4C53 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 18 Mar 2014 09:24:55 +0800 Zhouyi Zhou wrote: > Thanks Gerald for reviewing and sorry for not elaborated it in the e-mail. > > Firstly, I think you can't call module_put after fail try_module_get There is another try_module_get() in this function before that, so I need to call module_put() once if the second try_module_get() fails. The first try_module_get() is only needed within the function, while the second one is needed to prevent a module from being unloaded while its callback is registered, so that reference is kept as long as the module is active. > > Secondly, there exists duplicate module_put on the program path (the last > one is before return 0) There is one module_put() for every try_module_get(). If "1" is written to the sysctl, the function exits with one "missing" module_put(), which is held to protect the callback. When "0" is written, this reference is returned again with an "extra" module_put(). > > On Mon, Mar 17, 2014 at 9:28 PM, Gerald Schaefer > wrote: > > On Sat, 15 Mar 2014 21:35:40 +0800 > > Zhouyi Zhou wrote: > > > >> correct misuses of module_put in appldata_generic_handler > > > > Sorry, I don't see any misuse, could you elaborate? > > > >> > >> Signed-off-by: Zhouyi Zhou > >> --- > >> arch/s390/appldata/appldata_base.c | 3 --- > >> 1 file changed, 3 deletions(-) > >> > >> diff --git a/arch/s390/appldata/appldata_base.c b/arch/s390/appldata/appldata_base.c > >> index 47c8630..683e0282 100644 > >> --- a/arch/s390/appldata/appldata_base.c > >> +++ b/arch/s390/appldata/appldata_base.c > >> @@ -343,7 +343,6 @@ appldata_generic_handler(struct ctl_table *ctl, int write, > >> // protect work queue callback > >> if (!try_module_get(ops->owner)) { > >> mutex_unlock(&appldata_ops_mutex); > >> - module_put(ops->owner); > >> return -ENODEV; > >> } > >> ops->callback(ops->data); // init record > >> @@ -354,7 +353,6 @@ appldata_generic_handler(struct ctl_table *ctl, int write, > >> if (rc != 0) { > >> pr_err("Starting the data collection for %s " > >> "failed with rc=%d\n", ops->name, rc); > >> - module_put(ops->owner); > >> } else > >> ops->active = 1; > >> } else if ((buf[0] == '0') && (ops->active == 1)) { > >> @@ -365,7 +363,6 @@ appldata_generic_handler(struct ctl_table *ctl, int write, > >> if (rc != 0) > >> pr_err("Stopping the data collection for %s " > >> "failed with rc=%d\n", ops->name, rc); > >> - module_put(ops->owner); > >> } > >> mutex_unlock(&appldata_ops_mutex); > >> out: > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-s390" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >