netdev - Re: [RFC PATCHv2 1/3] hrtimer: Add notifer for clock_was

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]

Message-ID: <520214C6.6000307@redhat.com>
Date:	Wed, 07 Aug 2013 11:35:02 +0200
From:	Daniel Borkmann <dborkman@...hat.com>
To:	Fan Du <fan.du@...driver.com>
CC:	steffen.klassert@...unet.com, davem@...emloft.net,
	herbert@...dor.hengli.com.au, netdev@...r.kernel.org
Subject: Re: [RFC PATCHv2 1/3] hrtimer: Add notifer for clock_was_set

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@...driver.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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html