[RFC,PATCHv2,1/3] hrtimer: Add notifer for clock_was_set

When clock_was_set is called in case of system wall time change
or host resume from suspend state, use this notifier for places
where interested in this action.

Signed-off-by: Fan Du <fan.du@windriver.com>
---
 kernel/hrtimer.c |    4 ++++
 1 file changed, 4 insertions(+)

On 08/07/2013 11:04 AM, Fan Du wrote:
> When clock_was_set is called in case of system wall time change
> or host resume from suspend state, use this notifier for places
> where interested in this action.

(Only minor commenting on this one ...)

> Signed-off-by: Fan Du <fan.du@windriver.com>
> ---
>   kernel/hrtimer.c |    4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
> index 383319b..b7c62a9 100644
> --- a/kernel/hrtimer.c
> +++ b/kernel/hrtimer.c
> @@ -755,6 +755,9 @@ static inline void retrigger_next_event(void *arg) { }
>
>   #endif /* CONFIG_HIGH_RES_TIMERS */
>
> +ATOMIC_NOTIFIER_HEAD(clock_change_notifier_list);
> +EXPORT_SYMBOL(clock_change_notifier_list);

This should be static and hidden from other modules, e.g. have a look at
netevent_notif_chain (net/core/netevent.c).

Instead, this should be accessed via registration/un-registration handlers
for notifier blocks, and those can then be exported as EXPORT_SYMBOL_GPL
as this is core area.

> +
>   /*
>    * Clock realtime was set
>    *
> @@ -773,6 +776,7 @@ void clock_was_set(void)
>   	on_each_cpu(retrigger_next_event, NULL, 1);
>   #endif
>   	timerfd_clock_was_set();
> +	atomic_notifier_call_chain(&clock_change_notifier_list, 0, 0);

Also here a small one-line handler call_clock_change_notifiers() would be
better.

>   }
>
>   /*
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 2013年08月07日 17:20, Daniel Borkmann wrote:
> On 08/07/2013 11:04 AM, Fan Du wrote:
>> When clock_was_set is called in case of system wall time change
>> or host resume from suspend state, use this notifier for places
>> where interested in this action.
>
> (Only minor commenting on this one ...)
>
>> Signed-off-by: Fan Du <fan.du@windriver.com>
>> ---
>> kernel/hrtimer.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
>> index 383319b..b7c62a9 100644
>> --- a/kernel/hrtimer.c
>> +++ b/kernel/hrtimer.c
>> @@ -755,6 +755,9 @@ static inline void retrigger_next_event(void *arg) { }
>>
>> #endif /* CONFIG_HIGH_RES_TIMERS */
>>
>> +ATOMIC_NOTIFIER_HEAD(clock_change_notifier_list);
>> +EXPORT_SYMBOL(clock_change_notifier_list);
>
> This should be static and hidden from other modules, e.g. have a look at
> netevent_notif_chain (net/core/netevent.c).
>
> Instead, this should be accessed via registration/un-registration handlers
> for notifier blocks, and those can then be exported as EXPORT_SYMBOL_GPL
> as this is core area.
>
>> +
>> /*
>> * Clock realtime was set
>> *
>> @@ -773,6 +776,7 @@ void clock_was_set(void)
>> on_each_cpu(retrigger_next_event, NULL, 1);
>> #endif
>> timerfd_clock_was_set();
>> + atomic_notifier_call_chain(&clock_change_notifier_list, 0, 0);
>
> Also here a small one-line handler call_clock_change_notifiers() would be
> better.

Thanks for your attention.
After taking a look at netevent_notif_chain, you mean I should do it in below style:

static ATOMIC_NOTIFIER_HEAD(clock_change_notifier_list);

int un/register_clock_change_notifier(struct notifier_block *nb)
{
         int err;

         err = atomic_notifier_chain_un/register(&clock_change_notifier_list, nb);
         return err;
}
EXPORT_SYMBOL_GPL(clock_change_notifier_list);

int call_clock_change_notifiers(unsigned long val, void *v)
{
         return atomic_notifier_call_chain(&clock_change_notifier, val, v);
}
EXPORT_SYMBOL_GPL(call_clock_change_notifiers);

Will do it in next version after others comment rest of patches.

Thanks Daniel.

>> }
>>
>> /*
>>
>

On 08/07/2013 11:31 AM, Fan Du wrote:
> On 2013年08月07日 17:20, Daniel Borkmann wrote:
>> On 08/07/2013 11:04 AM, Fan Du wrote:
>>> When clock_was_set is called in case of system wall time change
>>> or host resume from suspend state, use this notifier for places
>>> where interested in this action.
>>
>> (Only minor commenting on this one ...)
>>
>>> Signed-off-by: Fan Du <fan.du@windriver.com>
>>> ---
>>> kernel/hrtimer.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
>>> index 383319b..b7c62a9 100644
>>> --- a/kernel/hrtimer.c
>>> +++ b/kernel/hrtimer.c
>>> @@ -755,6 +755,9 @@ static inline void retrigger_next_event(void *arg) { }
>>>
>>> #endif /* CONFIG_HIGH_RES_TIMERS */
>>>
>>> +ATOMIC_NOTIFIER_HEAD(clock_change_notifier_list);
>>> +EXPORT_SYMBOL(clock_change_notifier_list);
>>
>> This should be static and hidden from other modules, e.g. have a look at
>> netevent_notif_chain (net/core/netevent.c).
>>
>> Instead, this should be accessed via registration/un-registration handlers
>> for notifier blocks, and those can then be exported as EXPORT_SYMBOL_GPL
>> as this is core area.
>>
>>> +
>>> /*
>>> * Clock realtime was set
>>> *
>>> @@ -773,6 +776,7 @@ void clock_was_set(void)
>>> on_each_cpu(retrigger_next_event, NULL, 1);
>>> #endif
>>> timerfd_clock_was_set();
>>> + atomic_notifier_call_chain(&clock_change_notifier_list, 0, 0);
>>
>> Also here a small one-line handler call_clock_change_notifiers() would be
>> better.
>
> Thanks for your attention.
> After taking a look at netevent_notif_chain, you mean I should do it in below style:
>
> static ATOMIC_NOTIFIER_HEAD(clock_change_notifier_list);
>
> int un/register_clock_change_notifier(struct notifier_block *nb)
> {
>          int err;
>
>          err = atomic_notifier_chain_un/register(&clock_change_notifier_list, nb);
>          return err;

return atomic_notifier_chain_un/register(&clock_change_notifier_list, nb);

> }
> EXPORT_SYMBOL_GPL(clock_change_notifier_list);
>
> int call_clock_change_notifiers(unsigned long val, void *v)
> {
>          return atomic_notifier_call_chain(&clock_change_notifier, val, v);
> }
> EXPORT_SYMBOL_GPL(call_clock_change_notifiers);
>
> Will do it in next version after others comment rest of patches.

Yes, sounds good to me (you might also want to cc Thomas Gleixner on this one).
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html