Re: [PATCH V2 Resend 4/4] timer: Migrate running timer

[Viresh Kumar]

On 31 May 2013 16:19, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
> On 22 May 2013 14:04, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
>> Sorry for being late in replying to your queries.
>>
>> On 13 May 2013 16:05, Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
>>> Which mechanism is migrating the timer away?
>>
>> It will be the same: get_nohz_timer_target() which will decide target
>> cpu for migration.
>>
>>> I have no objections to the functionality per se, but the proposed
>>> solution is not going to fly.
>>>
>>> Aside of bloating the data structure you're changing the semantics of
>>> __mod_timer(). No __mod_timer() caller can deal with -EBUSY. So you'd
>>> break the world and some more.
>>
>> Ahh.. That idea was dropped already.
>>
>>> Here is a list of questions:
>>>
>>>       - Which mechanism migrates timers?
>>>
>>>       - How is that mechanism triggered?
>>
>> The mechanism remains the same as is for non-rearmed timers.
>> i.e. get_nohz_timer_target()..
>>
>> We are just trying to give a approach with which we can migrate
>> running timers. i.e. those which re-arm themselves from their
>> handlers.
>>
>>>       - How does that deal with CPU bound timers?
>>
>> We will still check 'Pinned' for this timer as is done for any other
>> normal timer. So, we don't migrate them.
>>
>> So, this is the clean draft for the idea I had.. (Naming is poor for
>> now):
>>
>> diff --git a/include/linux/timer.h b/include/linux/timer.h
>> index 8c5a197..ad00ebe 100644
>> --- a/include/linux/timer.h
>> +++ b/include/linux/timer.h
>> @@ -20,6 +20,7 @@ struct timer_list {
>>
>>         void (*function)(unsigned long);
>>         unsigned long data;
>> +       int wait_for_migration_to_complete;
>>
>>         int slack;
>>
>> diff --git a/kernel/timer.c b/kernel/timer.c
>> index a860bba..7791f28 100644
>> --- a/kernel/timer.c
>> +++ b/kernel/timer.c
>> @@ -746,21 +746,15 @@ __mod_timer(struct timer_list *timer, unsigned
>> long expires,
>>         new_base = per_cpu(tvec_bases, cpu);
>>
>>         if (base != new_base) {
>> -               /*
>> -                * We are trying to schedule the timer on the local CPU.
>> -                * However we can't change timer's base while it is running,
>> -                * otherwise del_timer_sync() can't detect that the timer's
>> -                * handler yet has not finished. This also guarantees that
>> -                * the timer is serialized wrt itself.
>> -                */
>> -               if (likely(base->running_timer != timer)) {
>> -                       /* See the comment in lock_timer_base() */
>> -                       timer_set_base(timer, NULL);
>> -                       spin_unlock(&base->lock);
>> -                       base = new_base;
>> -                       spin_lock(&base->lock);
>> -                       timer_set_base(timer, base);
>> -               }
>> +               if (base->running_timer == timer)
>> +                       timer->wait_for_migration_to_complete = 1;
>> +
>> +               /* See the comment in lock_timer_base() */
>> +               timer_set_base(timer, NULL);
>> +               spin_unlock(&base->lock);
>> +               base = new_base;
>> +               spin_lock(&base->lock);
>> +               timer_set_base(timer, base);
>>         }
>>
>>         timer->expires = expires;
>> @@ -990,7 +984,8 @@ int try_to_del_timer_sync(struct timer_list *timer)
>>
>>         base = lock_timer_base(timer, &flags);
>>
>> -       if (base->running_timer != timer) {
>> +       if ((base->running_timer != timer) &&
>> +                       !timer->wait_for_migration_to_complete) {
>>                 timer_stats_timer_clear_start_info(timer);
>>                 ret = detach_if_pending(timer, base, true);
>>         }
>> @@ -1183,6 +1178,8 @@ static inline void __run_timers(struct tvec_base *base)
>>                                 call_timer_fn(timer, fn, data);
>>                                 spin_lock_irq(&base->lock);
>>                         }
>> +                       if (timer->wait_for_migration_to_complete)
>> +                               timer->wait_for_migration_to_complete = 0;
>>                 }
>>         }
>>         base->running_timer = NULL;
>>
>>
>> Please see if it a junk idea or has some light of hope :)
>
> Ping!!

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