[lm-sensors] [PATCH 2/4] hwmon: (coretemp) update hotplug condition check

Chen Gong gong.chen at linux.intel.com
Tue Jul 20 07:26:58 CEST 2010


于 7/19/2010 7:32 PM, Jean Delvare 写道:
> Hi Chen,
>
> On Thu, 15 Jul 2010 10:46:42 +0800, Chen Gong wrote:
>> this patch fixes two errors about hotplug. One is for
>> hotplug notifier. The other is unnecessary driver unregister.
>> Because even none of online cpus supports coretemp, we can't
>> assume new onlined cpu doesn't support it either. If related
>> driver is unregistered there we have no chance to use coretemp
>> from then on.
>>
>> Signed-off-by: Chen Gong<gong.chen at linux.intel.com>
>> ---
>>   drivers/hwmon/coretemp.c |    9 +++------
>>   1 files changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
>> index b89f6a2..7b7c5b8 100644
>> --- a/drivers/hwmon/coretemp.c
>> +++ b/drivers/hwmon/coretemp.c
>> @@ -514,10 +514,13 @@ static int __cpuinit coretemp_cpu_callback(struct notifier_block *nfb,
>>
>>   	switch (action) {
>>   	case CPU_ONLINE:
>> +	case CPU_ONLINE_FROZEN:
>>   	case CPU_DOWN_FAILED:
>> +	case CPU_DOWN_FAILED_FROZEN:
>>   		coretemp_device_add(cpu);
>>   		break;
>>   	case CPU_DOWN_PREPARE:
>> +	case CPU_DOWN_PREPARE_FROZEN:
>>   		coretemp_device_remove(cpu);
>>   		break;
>>   	}
>
> Hmm. Please discuss this with Rafael (Cc'd). We already had 2 changes
> in this area:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=8bb7844286fb8c9fce6f65d8288aeb09d03a5e0d#patch22
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=561d9a969455cb009bb15b63e1d925dc527e7a9d
>
> As you can see, Rafael has been removing what you are trying to add
> back. And Rafael knows quite a bit when it comes to power management.
> So I will not accept this patch unless he approves them.
>

Deadlock ? Looks weird. But since Rafael ever met this issue, I agree 
with you.

>> @@ -559,10 +562,6 @@ static int __init coretemp_init(void)
>>   				" has no thermal sensor.\n", c->x86_model);
>>   		}
>>   	}
>> -	if (list_empty(&pdev_list)) {
>> -		err = -ENODEV;
>> -		goto exit_driver_unreg;
>> -	}
>>
>>   #ifdef CONFIG_HOTPLUG_CPU
>>   	register_hotcpu_notifier(&coretemp_cpu_notifier);
>> @@ -577,8 +576,6 @@ exit_devices_unreg:
>>   		kfree(p);
>>   	}
>>   	mutex_unlock(&pdev_list_mutex);
>> -exit_driver_unreg:
>> -	platform_driver_unregister(&coretemp_driver);
>>   exit:
>>   	return err;
>>   }
>
> I think the code is that way on purpose. The problem you mention is
> theoretical and can't happen in practice. In practice, either all the
> processor cores on the system are supported by the driver, or none is.
> And if are running some code, then at least one core is up, so it will
> detect itself.
>
> If I'm wrong, then please present a realistic case where we may find no
> supported core at some point in time, and one shows up later.
>

I don't agree. We know it maybe happens under some special environment, 
which means it is hard to be reproduced. But as we know it is possible 
why don't fix it ?




More information about the lm-sensors mailing list