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

[Viresh Kumar]

On 9 April 2013 20:22, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
> [Steven replied to a personal Ping!!, including everybody again]
>
> On 9 April 2013 19:25, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>> On Tue, 2013-04-09 at 14:05 +0530, Viresh Kumar wrote:
>>> Ping!!
>>>
>>
>> Remind me again. What problem are you trying to solve?
>
> I was trying to migrate a running timer which arms itself, so that we don't
> keep a cpu busy just for servicing this timer.
>
>>> On 20 March 2013 20:43, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
>>> >
>>> > Hi Steven/Thomas,
>>> >
>>> > I came back to this patch after completing some other stuff and posting
>>> > wq part of this patchset separately.
>>> >
>>> > I got your point and understand how this would fail.
>>> >
>>> > @Thomas: I need your opinion first. Do you like this concept of migrating
>>> > running timer or not? Or you see some basic problem with this concept?
>>
>> I'll let Thomas answer this, but to me, this sounds really racy.
>
> Sure.
>
>>> > If no (i.e. i can go ahead with another version), then i have some solution to
>>> > fix earlier problems reported by Steven:
>>> >
>>> > The problem lies with del_timer_sync() which just checks
>>> > base->running_timer != timer to check if timer is currently running or not.
>>> >
>>> > What if we add another variable in struct timer_list, that will store if we are
>>> > running timer callback or not. And so, before we call callback in timer core,
>>> > we will set this variable and will reset it after finishing callback.
>>> >
>>> > del_timer_sync() will have something like:
>>> >
>>> > if (base->running_timer != timer)
>>> >     remove timer and return;
>>
>> For example, this didn't fix the issue. You removed the timer when it
>> was still running, because base->running_timer did not equal timer.
>
> You are correct and i was stupid. I wanted to write this instead:
>
> del_timer_sync() will have something like:
>
> if (base->running_timer != timer)
>     if (timer->running_callback)
>         go back to its loop...
>     else
>         remove timer and return;
>
> i.e. if we aren't running on our base cpu, just check if our callback is
> executing somewhere else due to migration.

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/