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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 24 Jul 2013 14:47:52 +0530
From:	Viresh Kumar <viresh.kumar@...aro.org>
To:	Steven Rostedt <rostedt@...dmis.org>,
	Thomas Gleixner <tglx@...utronix.de>
Cc:	pjt@...gle.com, paul.mckenney@...aro.org, tj@...nel.org,
	mingo@...hat.com, peterz@...radead.org, Arvind.Chauhan@....com,
	linaro-dev@...ts.linaro.org, patches@...aro.org,
	pdsw-power-team@....com, linux-kernel@...r.kernel.org,
	linux-rt-users@...r.kernel.org,
	John Stultz <john.stultz@...aro.org>
Subject: Re: [PATCH V2 Resend 4/4] timer: Migrate running timer

On 18 June 2013 10:21, Viresh Kumar <viresh.kumar@...aro.org> wrote:
> On 31 May 2013 16:19, Viresh Kumar <viresh.kumar@...aro.org> wrote:
>> On 22 May 2013 14:04, Viresh Kumar <viresh.kumar@...aro.org> wrote:
>>> Sorry for being late in replying to your queries.
>>>
>>> On 13 May 2013 16:05, Thomas Gleixner <tglx@...utronix.de> 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!!

Hi Thomas,

Do you still see some light of hope in this patch :) ?

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