From: Chen Gong <gong.chen@linux.intel.com>
To: Jan Beulich <JBeulich@novell.com>
Cc: fenghua.yu@intel.com, linux-kernel@vger.kernel.org,
lm-sensors@lm-sensors.org
Subject: Re: [lm-sensors] [PATCH] x86/hwmon: fix module
Date: Tue, 14 Sep 2010 07:46:43 +0000 [thread overview]
Message-ID: <4C8F2863.5070003@linux.intel.com> (raw)
In-Reply-To: <4C8F36B40200007800015E6B@vpn.id2.novell.com>
于 9/14/2010 2:47 PM, Jan Beulich 写道:
>>>> On 14.09.10 at 08:30, Chen Gong<gong.chen@linux.intel.com> wrote:
>> 于 9/13/2010 6:05 PM, Jan Beulich 写道:
>>> In commit 0dca94baeab4a1a514841b0a4c8e3a51dfb4d5ae the call to
>>> platform_driver_unregister() was made conditional upon !HOTPLUG_CPU,
>>> but the return value from coretemp_init() was left to indicate an
>>> error. This isn't correct, as the negative return value indicates to
>>> the module loader that initialization failed, which isn't intended
>>
>> only !HOTPLUG_CPU can remove the driver. That is what we need to do
>> because under the hotplug condition, we need to consider those "offline"
>> cpus. I don't understand what you are afraid. Can you give me a scinario ?
>
> If the driver's init routine returns -ENODEV (like is the case without this
> change when HOTPLUG_CPU is enabled and there is no device found),
> the platform driver will remain registered, yet the driver memory will
> get freed (leaving dangling pointers into the driver's former image). Or
Under this condition, I can't fine who frees the memory of the platform
driver
> in other words - when leaving the driver registered, the return value
> of the init function must be zero.
Is it mandotary or just some kind of convention ?
>
> Jan
>
>>> here and results in dangling pointers.
>>>
>>> Signed-off-by: Jan Beulich<jbeulich@novell.com>
>>> Cc: Chen Gong<gong.chen@linux.intel.com>
>>>
>>> ---
>>> drivers/hwmon/coretemp.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> --- linux-2.6.36-rc4/drivers/hwmon/coretemp.c 2010-09-13 08:45:02.000000000
>> +0200
>>> +++
>> 2.6.36-rc4-x86-coretemp-hotplug-init-return/drivers/hwmon/coretemp.c 2010-
>> 09-03 17:54:14.000000000 +0200
>>> @@ -541,16 +541,19 @@ static int __init coretemp_init(void)
>>> " has no thermal sensor.\n", c->x86_model);
>>> }
>>> }
>>> +
>>> +#ifndef CONFIG_HOTPLUG_CPU
>>> if (list_empty(&pdev_list)) {
>>> err = -ENODEV;
>>> goto exit_driver_unreg;
>>> }
>>> +#endif
>>>
>>> register_hotcpu_notifier(&coretemp_cpu_notifier);
>>> return 0;
>>>
>>> -exit_driver_unreg:
>>> #ifndef CONFIG_HOTPLUG_CPU
>>> +exit_driver_unreg:
>>> platform_driver_unregister(&coretemp_driver);
>>> #endif
>>> exit:
>>>
>>>
>>>
>>>
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
WARNING: multiple messages have this Message-ID (diff)
From: Chen Gong <gong.chen@linux.intel.com>
To: Jan Beulich <JBeulich@novell.com>
Cc: fenghua.yu@intel.com, linux-kernel@vger.kernel.org,
lm-sensors@lm-sensors.org
Subject: Re: [lm-sensors] [PATCH] x86/hwmon: fix module init for hotplug-but-no-device-found case
Date: Tue, 14 Sep 2010 15:46:43 +0800 [thread overview]
Message-ID: <4C8F2863.5070003@linux.intel.com> (raw)
In-Reply-To: <4C8F36B40200007800015E6B@vpn.id2.novell.com>
于 9/14/2010 2:47 PM, Jan Beulich 写道:
>>>> On 14.09.10 at 08:30, Chen Gong<gong.chen@linux.intel.com> wrote:
>> 于 9/13/2010 6:05 PM, Jan Beulich 写道:
>>> In commit 0dca94baeab4a1a514841b0a4c8e3a51dfb4d5ae the call to
>>> platform_driver_unregister() was made conditional upon !HOTPLUG_CPU,
>>> but the return value from coretemp_init() was left to indicate an
>>> error. This isn't correct, as the negative return value indicates to
>>> the module loader that initialization failed, which isn't intended
>>
>> only !HOTPLUG_CPU can remove the driver. That is what we need to do
>> because under the hotplug condition, we need to consider those "offline"
>> cpus. I don't understand what you are afraid. Can you give me a scinario ?
>
> If the driver's init routine returns -ENODEV (like is the case without this
> change when HOTPLUG_CPU is enabled and there is no device found),
> the platform driver will remain registered, yet the driver memory will
> get freed (leaving dangling pointers into the driver's former image). Or
Under this condition, I can't fine who frees the memory of the platform
driver
> in other words - when leaving the driver registered, the return value
> of the init function must be zero.
Is it mandotary or just some kind of convention ?
>
> Jan
>
>>> here and results in dangling pointers.
>>>
>>> Signed-off-by: Jan Beulich<jbeulich@novell.com>
>>> Cc: Chen Gong<gong.chen@linux.intel.com>
>>>
>>> ---
>>> drivers/hwmon/coretemp.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> --- linux-2.6.36-rc4/drivers/hwmon/coretemp.c 2010-09-13 08:45:02.000000000
>> +0200
>>> +++
>> 2.6.36-rc4-x86-coretemp-hotplug-init-return/drivers/hwmon/coretemp.c 2010-
>> 09-03 17:54:14.000000000 +0200
>>> @@ -541,16 +541,19 @@ static int __init coretemp_init(void)
>>> " has no thermal sensor.\n", c->x86_model);
>>> }
>>> }
>>> +
>>> +#ifndef CONFIG_HOTPLUG_CPU
>>> if (list_empty(&pdev_list)) {
>>> err = -ENODEV;
>>> goto exit_driver_unreg;
>>> }
>>> +#endif
>>>
>>> register_hotcpu_notifier(&coretemp_cpu_notifier);
>>> return 0;
>>>
>>> -exit_driver_unreg:
>>> #ifndef CONFIG_HOTPLUG_CPU
>>> +exit_driver_unreg:
>>> platform_driver_unregister(&coretemp_driver);
>>> #endif
>>> exit:
>>>
>>>
>>>
>>>
next prev parent reply other threads:[~2010-09-14 7:46 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-09-13 10:05 [lm-sensors] [PATCH] x86/hwmon: fix module init for Jan Beulich
2010-09-13 10:05 ` [PATCH] x86/hwmon: fix module init for hotplug-but-no-device-found case Jan Beulich
2010-09-14 6:30 ` [lm-sensors] [PATCH] x86/hwmon: fix module init Chen Gong
2010-09-14 6:30 ` [lm-sensors] [PATCH] x86/hwmon: fix module init for hotplug-but-no-device-found case Chen Gong
2010-09-14 6:47 ` [lm-sensors] [PATCH] x86/hwmon: fix module init Jan Beulich
2010-09-14 6:47 ` [lm-sensors] [PATCH] x86/hwmon: fix module init for hotplug-but-no-device-found case Jan Beulich
2010-09-14 7:46 ` Chen Gong [this message]
2010-09-14 7:46 ` Chen Gong
2010-09-14 8:19 ` [lm-sensors] [PATCH] x86/hwmon: fix module init for Jan Beulich
2010-09-14 8:19 ` [lm-sensors] [PATCH] x86/hwmon: fix module init for hotplug-but-no-device-found case Jan Beulich
2010-09-24 4:43 ` Guenter Roeck
2010-09-24 18:50 ` [lm-sensors] x86/hwmon: fix module init for Guenter Roeck
2010-09-24 18:50 ` x86/hwmon: fix module init for hotplug-but-no-device-found case Guenter Roeck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4C8F2863.5070003@linux.intel.com \
--to=gong.chen@linux.intel.com \
--cc=JBeulich@novell.com \
--cc=fenghua.yu@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lm-sensors@lm-sensors.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.