diff mbox

[RFC,2/4] timer: don't migrate pinned timers

Message ID 3fa0ea3a19a51ae2797a25e281763451ad8d2844.1395322529.git.viresh.kumar@linaro.org
State New
Headers show

Commit Message

Viresh Kumar March 20, 2014, 1:48 p.m. UTC
migrate_timer() is called when a CPU goes down and its timers are required to be
migrated to some other CPU. Its the responsibility of the users of the timer to
remove it before control reaches to migrate_timers().

As these were the pinned timers, the best we can do is: don't migrate these and
report to the user as well.

That's all this patch does.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 kernel/timer.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Kevin Hilman March 31, 2014, 3:56 p.m. UTC | #1
Viresh Kumar <viresh.kumar@linaro.org> writes:

> migrate_timer() is called when a CPU goes down and its timers are required to be
> migrated to some other CPU. Its the responsibility of the users of the timer to
> remove it before control reaches to migrate_timers().
>
> As these were the pinned timers, the best we can do is: don't migrate these and
> report to the user as well.
>
> That's all this patch does.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  kernel/timer.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/kernel/timer.c b/kernel/timer.c
> index fec4ab4..a7f8b99 100644
> --- a/kernel/timer.c
> +++ b/kernel/timer.c
> @@ -1606,11 +1606,22 @@ static int init_timers_cpu(int cpu)
>  static void migrate_timer_list(struct tvec_base *new_base, struct list_head *head)
>  {
>  	struct timer_list *timer;
> +	int is_pinned;
>  
>  	while (!list_empty(head)) {
>  		timer = list_first_entry(head, struct timer_list, entry);
>  		/* We ignore the accounting on the dying cpu */
>  		detach_timer(timer, false);
> +
> +		is_pinned = tbase_get_pinned(timer->base);
> +
> +		/* Check if CPU still has pinned timers */
> +		if (is_pinned) {
> +			pr_warn("%s: can't migrate pinned timer: %p, removing it\n",
> +					__func__, timer);

printk message will be confusing: removing it from what? 

Kevin


> +			continue;
> +		}
> +
>  		timer_set_base(timer, new_base);
>  		internal_add_timer(new_base, timer);
>  	}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Viresh Kumar April 1, 2014, 4:32 a.m. UTC | #2
On 31 March 2014 21:26, Kevin Hilman <khilman@linaro.org> wrote:
> Viresh Kumar <viresh.kumar@linaro.org> writes:
>> +             if (is_pinned) {
>> +                     pr_warn("%s: can't migrate pinned timer: %p, removing it\n",
>> +                                     __func__, timer);
>
> printk message will be confusing: removing it from what?

Hmm.. So, I am looking to do two modifications here. Just need inputs
if that would be the right thing to do:

- do a WARN() here as these timers should have been already removed
- change print to: "can't migrate pinned timer %p, deactivating timer"

Looks fine?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Kevin Hilman April 4, 2014, 3:15 p.m. UTC | #3
Viresh Kumar <viresh.kumar@linaro.org> writes:

> On 31 March 2014 21:26, Kevin Hilman <khilman@linaro.org> wrote:
>> Viresh Kumar <viresh.kumar@linaro.org> writes:
>>> +             if (is_pinned) {
>>> +                     pr_warn("%s: can't migrate pinned timer: %p, removing it\n",
>>> +                                     __func__, timer);
>>
>> printk message will be confusing: removing it from what?
>
> Hmm.. So, I am looking to do two modifications here. Just need inputs
> if that would be the right thing to do:
>
> - do a WARN() here as these timers should have been already removed
> - change print to: "can't migrate pinned timer %p, deactivating timer"
>
> Looks fine?

Yes.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
diff mbox

Patch

diff --git a/kernel/timer.c b/kernel/timer.c
index fec4ab4..a7f8b99 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1606,11 +1606,22 @@  static int init_timers_cpu(int cpu)
 static void migrate_timer_list(struct tvec_base *new_base, struct list_head *head)
 {
 	struct timer_list *timer;
+	int is_pinned;
 
 	while (!list_empty(head)) {
 		timer = list_first_entry(head, struct timer_list, entry);
 		/* We ignore the accounting on the dying cpu */
 		detach_timer(timer, false);
+
+		is_pinned = tbase_get_pinned(timer->base);
+
+		/* Check if CPU still has pinned timers */
+		if (is_pinned) {
+			pr_warn("%s: can't migrate pinned timer: %p, removing it\n",
+					__func__, timer);
+			continue;
+		}
+
 		timer_set_base(timer, new_base);
 		internal_add_timer(new_base, timer);
 	}