Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 7 Feb 2022 11:04:37 -0700
From:      Warner Losh <imp@bsdimp.com>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        Sebastian Huber <sebastian.huber@embedded-brains.de>,  FreeBSD Hackers <freebsd-hackers@freebsd.org>
Subject:   Re: ntp_init() looks like a nop
Message-ID:  <CANCZdfoOVekuyZkkvdgzRQPYhgyOWjaqLagD0HNMFPquQAt1hw@mail.gmail.com>
In-Reply-To: <YgE4RaZVZhOlhJHW@kib.kiev.ua>
References:  <ac806cf6-a17f-77ba-5633-0c95eda1f8cf@embedded-brains.de> <YgE4RaZVZhOlhJHW@kib.kiev.ua>

next in thread | previous in thread | raw e-mail | index | archive | help

[-- Attachment #1 --]
On Mon, Feb 7, 2022 at 8:20 AM Konstantin Belousov <kostikbel@gmail.com>
wrote:

> On Mon, Feb 07, 2022 at 04:04:56PM +0100, Sebastian Huber wrote:
> > Hello,
> >
> > I review currently the kern_ntptime.c module since I would like to use
> it in
> > RTEMS. I have a question to ntp_init():
> >
> > /*
> >  * ntp_init() - initialize variables and structures
> >  *
> >  * This routine must be called after the kernel variables hz and tick
> >  * are set or changed and before the next tick interrupt. In this
> >  * particular implementation, these values are assumed set elsewhere in
> >  * the kernel. The design allows the clock frequency and tick interval
> >  * to be changed while the system is running. So, this routine should
> >  * probably be integrated with the code that does that.
> >  */
> > static void
> > ntp_init(void)
> > {
> >
> >       /*
> >        * The following variables are initialized only at startup. Only
> >        * those structures not cleared by the compiler need to be
> >        * initialized, and these only in the simulator. In the actual
> >        * kernel, any nonzero values here will quickly evaporate.
> >        */
> >       L_CLR(time_offset);
> >       L_CLR(time_freq);
> > #ifdef PPS_SYNC
> >       pps_tf[0].tv_sec = pps_tf[0].tv_nsec = 0;
> >       pps_tf[1].tv_sec = pps_tf[1].tv_nsec = 0;
> >       pps_tf[2].tv_sec = pps_tf[2].tv_nsec = 0;
> >       pps_fcount = 0;
> >       L_CLR(pps_freq);
> > #endif /* PPS_SYNC */
> > }
> >
> > SYSINIT(ntpclocks, SI_SUB_CLOCKS, SI_ORDER_MIDDLE, ntp_init, NULL);
> >
> > The ntp_init() function sets a couple of global variables to zero. These
> > variables should be in the .bss section. Are they not already cleared
> during
> > the kernel loading?
> This is nop indeed, be it linked in kernel, or loaded as a module.
>

I just looked at the posted patch, and I concur.

Warner

[-- Attachment #2 --]
<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Feb 7, 2022 at 8:20 AM Konstantin Belousov &lt;<a href="mailto:kostikbel@gmail.com">kostikbel@gmail.com</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, Feb 07, 2022 at 04:04:56PM +0100, Sebastian Huber wrote:<br>
&gt; Hello,<br>
&gt; <br>
&gt; I review currently the kern_ntptime.c module since I would like to use it in<br>
&gt; RTEMS. I have a question to ntp_init():<br>
&gt; <br>
&gt; /*<br>
&gt;  * ntp_init() - initialize variables and structures<br>
&gt;  *<br>
&gt;  * This routine must be called after the kernel variables hz and tick<br>
&gt;  * are set or changed and before the next tick interrupt. In this<br>
&gt;  * particular implementation, these values are assumed set elsewhere in<br>
&gt;  * the kernel. The design allows the clock frequency and tick interval<br>
&gt;  * to be changed while the system is running. So, this routine should<br>
&gt;  * probably be integrated with the code that does that.<br>
&gt;  */<br>
&gt; static void<br>
&gt; ntp_init(void)<br>
&gt; {<br>
&gt; <br>
&gt;       /*<br>
&gt;        * The following variables are initialized only at startup. Only<br>
&gt;        * those structures not cleared by the compiler need to be<br>
&gt;        * initialized, and these only in the simulator. In the actual<br>
&gt;        * kernel, any nonzero values here will quickly evaporate.<br>
&gt;        */<br>
&gt;       L_CLR(time_offset);<br>
&gt;       L_CLR(time_freq);<br>
&gt; #ifdef PPS_SYNC<br>
&gt;       pps_tf[0].tv_sec = pps_tf[0].tv_nsec = 0;<br>
&gt;       pps_tf[1].tv_sec = pps_tf[1].tv_nsec = 0;<br>
&gt;       pps_tf[2].tv_sec = pps_tf[2].tv_nsec = 0;<br>
&gt;       pps_fcount = 0;<br>
&gt;       L_CLR(pps_freq);<br>
&gt; #endif /* PPS_SYNC */ <br>
&gt; }<br>
&gt; <br>
&gt; SYSINIT(ntpclocks, SI_SUB_CLOCKS, SI_ORDER_MIDDLE, ntp_init, NULL);<br>
&gt; <br>
&gt; The ntp_init() function sets a couple of global variables to zero. These<br>
&gt; variables should be in the .bss section. Are they not already cleared during<br>
&gt; the kernel loading?<br>
This is nop indeed, be it linked in kernel, or loaded as a module.<br></blockquote><div><br></div><div>I just looked at the posted patch, and I concur.</div><div><br></div><div>Warner </div></div></div>

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfoOVekuyZkkvdgzRQPYhgyOWjaqLagD0HNMFPquQAt1hw>