intel_ips: Convert timers to use timer_setup()

PATCH In review Kees Cook 3 participants 7 messages Oct 5, 2017 – Nov 20, 2017 x86 platform drivers

Share & tools

Kees Cook

author

2017-10-05 00:54 UTC #0 lore raw

PATCH 1/1 intel_ips: Convert timers to use timer_setup()

In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly. Moves timer structure off stack and into struct ips_driver.

Cc Darren Hart <[email protected]>

Cc Andy Shevchenko <[email protected]>

Cc [email protected]

Cc Thomas Gleixner <[email protected]>

Signed-off-by Kees Cook <[email protected]>

---
This requires commit 686fef928bba ("timer: Prepare to change timer callback argument type") in v4.14-rc3, but should be otherwise stand-alone.

---
 drivers/platform/x86/intel_ips.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

drivers/platform/x86/intel_ips.c +7 -8

index 58dcee562d64..056afb731d79 100644
@@ -300,6 +300,7 @@ struct ips_driver {
 	struct task_struct *monitor;
 	struct task_struct *adjust;
 	struct dentry *debug_root;
+	struct timer_list timer;
 
 	/* Average CPU core temps (all averages in .01 degrees C for precision) */
 	u16 ctv1_avg_temp;

@@ -942,9 +943,10 @@ static u32 calc_avg_power(struct ips_driver *ips, u32 *array)
 	return avg;
 }
 
-static void monitor_timeout(unsigned long arg)
+static void monitor_timeout(struct timer_list *t)
 {
-	wake_up_process((struct task_struct *)arg);
+	struct ips_driver *ips = from_timer(ips, t, timer);
+	wake_up_process(ips->monitor);
 }
 
 /**

@@ -961,7 +963,6 @@ static void monitor_timeout(unsigned long arg)
 static int ips_monitor(void *data)
 {
 	struct ips_driver *ips = data;
-	struct timer_list timer;
 	unsigned long seqno_timestamp, expire, last_msecs, last_sample_period;
 	int i;
 	u32 *cpu_samples, *mchp_samples, old_cpu_power;

@@ -1049,8 +1050,7 @@ static int ips_monitor(void *data)
 	schedule_timeout_interruptible(msecs_to_jiffies(IPS_SAMPLE_PERIOD));
 	last_sample_period = IPS_SAMPLE_PERIOD;
 
-	setup_deferrable_timer_on_stack(&timer, monitor_timeout,
-					(unsigned long)current);
+	timer_setup(&ips->timer, monitor_timeout, TIMER_DEFERRABLE);
 	do {
 		u32 cpu_val, mch_val;
 		u16 val;

@@ -1107,7 +1107,7 @@ static int ips_monitor(void *data)
 		expire = jiffies + msecs_to_jiffies(IPS_SAMPLE_PERIOD);
 
 		__set_current_state(TASK_INTERRUPTIBLE);
-		mod_timer(&timer, expire);
+		mod_timer(&ips->timer, expire);
 		schedule();
 
 		/* Calculate actual sample period for power averaging */

@@ -1116,8 +1116,7 @@ static int ips_monitor(void *data)
 			last_sample_period = 1;
 	} while (!kthread_should_stop());
 
-	del_timer_sync(&timer);
-	destroy_timer_on_stack(&timer);
+	del_timer_sync(&ips->timer);
 
 	dev_dbg(&ips->dev->dev, "ips-monitor thread stopped\n");

Signature · 6 lines

2.7.4


-- 
Kees Cook
Pixel Security

Andy Shevchenko

2017-10-05 08:41 UTC #1 lore raw

Quoted 4 lines · Kees Cook

In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly. Moves timer structure off stack and into struct ips_driver.

Pushed to my testing queue, thanks!

Quoted 83 lines

Cc Darren Hart <[email protected]>

Cc Andy Shevchenko <[email protected]>

Cc [email protected]

Cc Thomas Gleixner <[email protected]>

Signed-off-by Kees Cook <[email protected]>

---
This requires commit 686fef928bba ("timer: Prepare to change timer callback argument type") in v4.14-rc3, but should be otherwise stand-alone.

---
 drivers/platform/x86/intel_ips.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

drivers/platform/x86/intel_ips.c +7 -9

index 58dcee562d64..056afb731d79 100644
@@ -300,6 +300,7 @@ struct ips_driver {
        struct task_struct *monitor;
        struct task_struct *adjust;
        struct dentry *debug_root;
+       struct timer_list timer;
 
        /* Average CPU core temps (all averages in .01 degrees C for precision) */
        u16 ctv1_avg_temp;

@@ -942,9 +943,10 @@ static u32 calc_avg_power(struct ips_driver *ips, u32 *array)
        return avg;
 }
 
-static void monitor_timeout(unsigned long arg)
+static void monitor_timeout(struct timer_list *t)
 {
-       wake_up_process((struct task_struct *)arg);
+       struct ips_driver *ips = from_timer(ips, t, timer);
+       wake_up_process(ips->monitor);
 }
 
 /**

@@ -961,7 +963,6 @@ static void monitor_timeout(unsigned long arg)
 static int ips_monitor(void *data)
 {
        struct ips_driver *ips = data;
-       struct timer_list timer;
        unsigned long seqno_timestamp, expire, last_msecs, last_sample_period;
        int i;
        u32 *cpu_samples, *mchp_samples, old_cpu_power;

@@ -1049,8 +1050,7 @@ static int ips_monitor(void *data)
        schedule_timeout_interruptible(msecs_to_jiffies(IPS_SAMPLE_PERIOD));
        last_sample_period = IPS_SAMPLE_PERIOD;
 
-       setup_deferrable_timer_on_stack(&timer, monitor_timeout,
-                                       (unsigned long)current);
+       timer_setup(&ips->timer, monitor_timeout, TIMER_DEFERRABLE);
        do {
                u32 cpu_val, mch_val;
                u16 val;

@@ -1107,7 +1107,7 @@ static int ips_monitor(void *data)
                expire = jiffies + msecs_to_jiffies(IPS_SAMPLE_PERIOD);
 
                __set_current_state(TASK_INTERRUPTIBLE);
-               mod_timer(&timer, expire);
+               mod_timer(&ips->timer, expire);
                schedule();
 
                /* Calculate actual sample period for power averaging */

@@ -1116,8 +1116,7 @@ static int ips_monitor(void *data)
                        last_sample_period = 1;
        } while (!kthread_should_stop());
 
-       del_timer_sync(&timer);
-       destroy_timer_on_stack(&timer);
+       del_timer_sync(&ips->timer);
 
        dev_dbg(&ips->dev->dev, "ips-monitor thread stopped\n");
 
--

2.7.4

--
Kees Cook
Pixel Security

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Kees Cook

author

2017-11-02 19:55 UTC #2 lore raw

Quoted 7 lines · Andy Shevchenko

Quoted 4 lines · Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly. Moves timer structure off stack and into struct ips_driver.
Pushed to my testing queue, thanks!

Hi,

I don't see this in -next yet. Should the tip tree carry this conversion?

Thanks!

-Kees

Quoted 90 lines

Quoted 81 lines

Cc Darren Hart <[email protected]>

Cc Andy Shevchenko <[email protected]>

Cc [email protected]

Cc Thomas Gleixner <[email protected]>

Signed-off-by Kees Cook <[email protected]>

---
This requires commit 686fef928bba ("timer: Prepare to change timer callback argument type") in v4.14-rc3, but should be otherwise stand-alone.

---
 drivers/platform/x86/intel_ips.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

drivers/platform/x86/intel_ips.c +7 -9

index 58dcee562d64..056afb731d79 100644
@@ -300,6 +300,7 @@ struct ips_driver {
        struct task_struct *monitor;
        struct task_struct *adjust;
        struct dentry *debug_root;
+       struct timer_list timer;
 
        /* Average CPU core temps (all averages in .01 degrees C for precision) */
        u16 ctv1_avg_temp;

@@ -942,9 +943,10 @@ static u32 calc_avg_power(struct ips_driver *ips, u32 *array)
        return avg;
 }
 
-static void monitor_timeout(unsigned long arg)
+static void monitor_timeout(struct timer_list *t)
 {
-       wake_up_process((struct task_struct *)arg);
+       struct ips_driver *ips = from_timer(ips, t, timer);
+       wake_up_process(ips->monitor);
 }
 
 /**

@@ -961,7 +963,6 @@ static void monitor_timeout(unsigned long arg)
 static int ips_monitor(void *data)
 {
        struct ips_driver *ips = data;
-       struct timer_list timer;
        unsigned long seqno_timestamp, expire, last_msecs, last_sample_period;
        int i;
        u32 *cpu_samples, *mchp_samples, old_cpu_power;

@@ -1049,8 +1050,7 @@ static int ips_monitor(void *data)
        schedule_timeout_interruptible(msecs_to_jiffies(IPS_SAMPLE_PERIOD));
        last_sample_period = IPS_SAMPLE_PERIOD;
 
-       setup_deferrable_timer_on_stack(&timer, monitor_timeout,
-                                       (unsigned long)current);
+       timer_setup(&ips->timer, monitor_timeout, TIMER_DEFERRABLE);
        do {
                u32 cpu_val, mch_val;
                u16 val;

@@ -1107,7 +1107,7 @@ static int ips_monitor(void *data)
                expire = jiffies + msecs_to_jiffies(IPS_SAMPLE_PERIOD);
 
                __set_current_state(TASK_INTERRUPTIBLE);
-               mod_timer(&timer, expire);
+               mod_timer(&ips->timer, expire);
                schedule();
 
                /* Calculate actual sample period for power averaging */

@@ -1116,8 +1116,7 @@ static int ips_monitor(void *data)
                        last_sample_period = 1;
        } while (!kthread_should_stop());
 
-       del_timer_sync(&timer);
-       destroy_timer_on_stack(&timer);
+       del_timer_sync(&ips->timer);
 
        dev_dbg(&ips->dev->dev, "ips-monitor thread stopped\n");
 
--

2.7.4

Quoted 3 lines

--
Kees Cook
Pixel Security

--
With Best Regards,
Andy Shevchenko

Signature · 2 lines

Kees Cook
Pixel Security

Andy Shevchenko

2017-11-03 12:26 UTC #3 lore raw

Quoted 8 lines · Kees Cook

Quoted 6 lines · Andy Shevchenko
Quoted 4 lines · Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly. Moves timer structure off stack and into struct ips_driver.

Quoted 5 lines

Quoted 1 line
Pushed to my testing queue, thanks!
Hi,
I don't see this in -next yet. Should the tip tree carry this conversion?

I thought it was a result of discussion since we lack of patch that brought the core change.

However, because I merged Wolfram's immutable branch in order to apply Hans' fix, I can carry your patch as well.

Either would be fine with me.

Going ahead, I applied it to my review and testing queue, thanks!

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Kees Cook

author

2017-11-18 01:30 UTC #4 lore raw

Quoted 18 lines · Andy Shevchenko

Quoted 8 lines · Kees Cook
Quoted 6 lines · Andy Shevchenko
Quoted 4 lines · Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly. Moves timer structure off stack and into struct ips_driver.
Quoted 5 lines
Quoted 1 line
Pushed to my testing queue, thanks!
Hi,
I don't see this in -next yet. Should the tip tree carry this conversion?
I thought it was a result of discussion since we lack of patch that brought the core change.

Quoted 6 lines

However, because I merged Wolfram's immutable branch in order to apply Hans' fix, I can carry your patch as well.
Either would be fine with me.
Going ahead, I applied it to my review and testing queue, thanks!

Hi again!

I'm just checking on this patch -- I haven't seen it in a pull request to Linus yet (for the merge window he's threatened will be small). This is one of the remaining conversion patches I need in v4.15. Is this still planned to be merged for v4.15?

Thanks!

-Kees

Signature · 2 lines

Kees Cook
Pixel Security

Darren Hart

2017-11-20 19:45 UTC #5 lore raw

Quoted 25 lines · Kees Cook

Quoted 23 lines · Andy Shevchenko
Quoted 8 lines · Kees Cook
Quoted 6 lines · Andy Shevchenko
Quoted 4 lines · Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly. Moves timer structure off stack and into struct ips_driver.
Quoted 5 lines
Quoted 1 line
Pushed to my testing queue, thanks!
Hi,
I don't see this in -next yet. Should the tip tree carry this conversion?
I thought it was a result of discussion since we lack of patch that brought the core change.
However, because I merged Wolfram's immutable branch in order to apply Hans' fix, I can carry your patch as well.
Either would be fine with me.

Quoted 8 lines

Quoted 1 line
Going ahead, I applied it to my review and testing queue, thanks!
Hi again!
I'm just checking on this patch -- I haven't seen it in a pull request to Linus yet (for the merge window he's threatened will be small). This is one of the remaining conversion patches I need in v4.15. Is this still planned to be merged for v4.15?

Hi Kees,

It went out the day after your note above in the platform-drivers-x86-v4.15-1 pull request. We're working through some issues with this pull request with Linus now, but yes, it will make 4.15.

Signature · 2 lines

Darren Hart
VMware Open Source Technology Center

Kees Cook

author

2017-11-20 19:46 UTC #6 lore raw

Quoted 35 lines · Darren Hart

Quoted 25 lines · Kees Cook
Quoted 23 lines · Andy Shevchenko
Quoted 8 lines · Kees Cook
Quoted 6 lines · Andy Shevchenko
Quoted 4 lines · Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly. Moves timer structure off stack and into struct ips_driver.
Quoted 5 lines
Quoted 1 line
Pushed to my testing queue, thanks!
Hi,
I don't see this in -next yet. Should the tip tree carry this conversion?
I thought it was a result of discussion since we lack of patch that brought the core change.
However, because I merged Wolfram's immutable branch in order to apply Hans' fix, I can carry your patch as well.
Either would be fine with me.
Quoted 8 lines
Quoted 1 line
Going ahead, I applied it to my review and testing queue, thanks!
Hi again!
I'm just checking on this patch -- I haven't seen it in a pull request to Linus yet (for the merge window he's threatened will be small). This is one of the remaining conversion patches I need in v4.15. Is this still planned to be merged for v4.15?

Quoted 5 lines

Hi Kees,
It went out the day after your note above in the platform-drivers-x86-v4.15-1 pull request. We're working through some issues with this pull request with Linus now, but yes, it will make 4.15.

Awesome, thanks! I see it in the tree now. Yay! :)

-Kees

Signature · 2 lines

Kees Cook
Pixel Security