Re: [RFC v0] thermal: Protect schedule flag by raw spin
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Date: 2014-05-09 12:09:07
* Daniel Wagner | 2014-04-14 16:22:27 [+0200]:
quoted hunk ↗ jump to hunk
diff --git a/drivers/thermal/x86_pkg_temp_thermal.c b/drivers/thermal/x86_pkg_temp_thermal.c index 7257366..7d0b0ac 100644 --- a/drivers/thermal/x86_pkg_temp_thermal.c +++ b/drivers/thermal/x86_pkg_temp_thermal.c@@ -363,16 +363,16 @@ static int pkg_temp_thermal_platform_thermal_notify(__u64 msr_val) * are in the same interrupt state. So scheduling on any one CPU in * the package is enough and simply return for others. */ - spin_lock_irqsave(&pkg_work_lock, flags); + raw_spin_lock_irqsave(&pkg_work_lock, flags); ++pkg_interrupt_cnt; if (unlikely(phy_id > max_phy_id) || unlikely(!pkg_work_scheduled) || pkg_work_scheduled[phy_id]) { disable_pkg_thres_interrupt(); - spin_unlock_irqrestore(&pkg_work_lock, flags); + raw_spin_unlock_irqrestore(&pkg_work_lock, flags); return -EINVAL; } pkg_work_scheduled[phy_id] = 1; - spin_unlock_irqrestore(&pkg_work_lock, flags); + raw_spin_unlock_irqrestore(&pkg_work_lock, flags); disable_pkg_thres_interrupt(); schedule_delayed_work_on(cpu,
This doens't work. Since you come out of the hard-irq context you can not call schedule_work() here. Currently I don't have any better idea than to do something like in "x86/mce: Defer mce wakeups to threads for PREEMPT_RT"
quoted hunk ↗ jump to hunk
@@ -426,19 +426,28 @@ static int pkg_temp_thermal_device_add(unsigned int cpu)goto err_ret_unlock; } - spin_lock_irqsave(&pkg_work_lock, flags); - if (topology_physical_package_id(cpu) > max_phy_id) + if (topology_physical_package_id(cpu) > max_phy_id) { max_phy_id = topology_physical_package_id(cpu);
I like this. You shouldn't however modify max_phy_id outside of the lock. Besides that it looks good.
- temp = krealloc(pkg_work_scheduled,
- (max_phy_id+1) * sizeof(u8), GFP_ATOMIC);
- if (!temp) {
- spin_unlock_irqrestore(&pkg_work_lock, flags);
- err = -ENOMEM;
- goto err_ret_free;
+
+ temp = kmalloc((max_phy_id+1) * sizeof(u8), GFP_KERNEL);
+ if (!temp) {
+ err = -ENOMEM;
+ goto err_ret_free;
+ }
+
+ raw_spin_lock_irqsave(&pkg_work_lock, flags);
+
+ p = pkg_work_scheduled;
+
+ memcpy(temp, pkg_work_scheduled, ksize(pkg_work_scheduled));
+ pkg_work_scheduled = temp;
+
+ pkg_work_scheduled[topology_physical_package_id(cpu)] = 0;This seems to be the "init" which needs to be done. Now you skip it if you are first called with 4 and later with 3,2,1,0 for instance. Using kzalloc() instead of kmalloc() should make it right and avoid the need of = 0 here.
+ + raw_spin_unlock_irqrestore(&pkg_work_lock, flags); + + kfree(p); } - pkg_work_scheduled = temp; - pkg_work_scheduled[topology_physical_package_id(cpu)] = 0; - spin_unlock_irqrestore(&pkg_work_lock, flags); phy_dev_entry->phys_proc_id = topology_physical_package_id(cpu); phy_dev_entry->first_cpu = cpu;
Sebastian