* [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