* [PATCH] s390: correct misuses of module_put in appldata_generic_handler. @ 2014-03-15 13:35 Zhouyi Zhou 2014-03-17 13:28 ` Gerald Schaefer 0 siblings, 1 reply; 5+ messages in thread From: Zhouyi Zhou @ 2014-03-15 13:35 UTC (permalink / raw) To: linux-kernel, linux-s390, joe, gang.chen, gerald.schaefer, linux390, heiko.carstens, schwidefsky Cc: Zhouyi Zhou, Zhouyi Zhou correct misuses of module_put in appldata_generic_handler Signed-off-by: Zhouyi Zhou <yizhouzhou@ict.ac.cn> --- 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: -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] s390: correct misuses of module_put in appldata_generic_handler. 2014-03-15 13:35 [PATCH] s390: correct misuses of module_put in appldata_generic_handler Zhouyi Zhou @ 2014-03-17 13:28 ` Gerald Schaefer 2014-03-18 1:24 ` Zhouyi Zhou 0 siblings, 1 reply; 5+ messages in thread From: Gerald Schaefer @ 2014-03-17 13:28 UTC (permalink / raw) To: Zhouyi Zhou Cc: linux-kernel, linux-s390, joe, gang.chen, linux390, heiko.carstens, schwidefsky, Zhouyi Zhou On Sat, 15 Mar 2014 21:35:40 +0800 Zhouyi Zhou <zhouzhouyi@gmail.com> 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 <yizhouzhou@ict.ac.cn> > --- > 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: ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] s390: correct misuses of module_put in appldata_generic_handler. 2014-03-17 13:28 ` Gerald Schaefer @ 2014-03-18 1:24 ` Zhouyi Zhou 2014-03-18 13:34 ` Gerald Schaefer 0 siblings, 1 reply; 5+ messages in thread From: Zhouyi Zhou @ 2014-03-18 1:24 UTC (permalink / raw) To: Gerald Schaefer Cc: linux-kernel@vger.kernel.org, linux-s390, joe, gang.chen, linux390, heiko.carstens, schwidefsky, Zhouyi Zhou 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 Secondly, there exists duplicate module_put on the program path (the last one is before return 0) On Mon, Mar 17, 2014 at 9:28 PM, Gerald Schaefer <gerald.schaefer@de.ibm.com> wrote: > On Sat, 15 Mar 2014 21:35:40 +0800 > Zhouyi Zhou <zhouzhouyi@gmail.com> 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 <yizhouzhou@ict.ac.cn> >> --- >> 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: > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] s390: correct misuses of module_put in appldata_generic_handler. 2014-03-18 1:24 ` Zhouyi Zhou @ 2014-03-18 13:34 ` Gerald Schaefer 2014-03-19 0:59 ` Zhouyi Zhou 0 siblings, 1 reply; 5+ messages in thread From: Gerald Schaefer @ 2014-03-18 13:34 UTC (permalink / raw) To: Zhouyi Zhou Cc: linux-kernel@vger.kernel.org, linux-s390, joe, gang.chen, linux390, heiko.carstens, schwidefsky, Zhouyi Zhou On Tue, 18 Mar 2014 09:24:55 +0800 Zhouyi Zhou <zhouzhouyi@gmail.com> 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 > <gerald.schaefer@de.ibm.com> wrote: > > On Sat, 15 Mar 2014 21:35:40 +0800 > > Zhouyi Zhou <zhouzhouyi@gmail.com> 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 <yizhouzhou@ict.ac.cn> > >> --- > >> 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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] s390: correct misuses of module_put in appldata_generic_handler. 2014-03-18 13:34 ` Gerald Schaefer @ 2014-03-19 0:59 ` Zhouyi Zhou 0 siblings, 0 replies; 5+ messages in thread From: Zhouyi Zhou @ 2014-03-19 0:59 UTC (permalink / raw) To: Gerald Schaefer Cc: linux-kernel@vger.kernel.org, linux-s390, joe, gang.chen, linux390, heiko.carstens, schwidefsky, Zhouyi Zhou Delicate design! thanks for replying, sorry for the trouble On Tue, Mar 18, 2014 at 9:34 PM, Gerald Schaefer <gerald.schaefer@de.ibm.com> wrote: > On Tue, 18 Mar 2014 09:24:55 +0800 > Zhouyi Zhou <zhouzhouyi@gmail.com> 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 >> <gerald.schaefer@de.ibm.com> wrote: >> > On Sat, 15 Mar 2014 21:35:40 +0800 >> > Zhouyi Zhou <zhouzhouyi@gmail.com> 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 <yizhouzhou@ict.ac.cn> >> >> --- >> >> 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 >> > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-03-19 0:59 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-03-15 13:35 [PATCH] s390: correct misuses of module_put in appldata_generic_handler Zhouyi Zhou 2014-03-17 13:28 ` Gerald Schaefer 2014-03-18 1:24 ` Zhouyi Zhou 2014-03-18 13:34 ` Gerald Schaefer 2014-03-19 0:59 ` Zhouyi Zhou
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox