Re: CVS commit: src/sys

[Martin Husemann]

On Thu, Nov 29, 2018 at 10:07:11PM +0000, Christos Zoulas wrote:
> >The pool is just called three times, so it's not a big deal.
> 
> I don't know, from reading the manual page using the pool_cache_ api seems
> to be the preferred way... Plus I don't think that a language deficiency
> should be visible in many places in the code (these memsets appear
> unnecessary) and if we do them using the pool_cache code we pay the cost
> once (at construction), not every use (however minor that cost is).

But it comes at a cost (the per-CPU cache) - how often is this code run?

Does using compound literals clear the gaps? Example of what I mean
below (incomplete, just for clarification)

Martin


Index: kern_time.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_time.c,v
retrieving revision 1.192
diff -u -p -r1.192 kern_time.c
--- kern_time.c 28 Nov 2018 15:10:40 -0000      1.192
+++ kern_time.c 30 Nov 2018 08:03:52 -0000
@@ -593,6 +593,7 @@ timer_create1(timer_t *tid, clockid_t id
        struct ptimers *pts;
        struct ptimer *pt;
        struct proc *p;
+       struct sigevent ev;
 
        p = l->l_proc;
 
@@ -602,20 +603,19 @@ timer_create1(timer_t *tid, clockid_t id
        if ((pts = p->p_timers) == NULL)
                pts = timers_alloc(p);
 
-       pt = pool_get(&ptimer_pool, PR_WAITOK);
-       memset(pt, 0, sizeof(*pt));
        if (evp != NULL) {
                if (((error =
-                   (*fetch_event)(evp, &pt->pt_ev, sizeof(pt->pt_ev))) != 0) ||
-                   ((pt->pt_ev.sigev_notify < SIGEV_NONE) ||
-                       (pt->pt_ev.sigev_notify > SIGEV_SA)) ||
-                       (pt->pt_ev.sigev_notify == SIGEV_SIGNAL &&
-                        (pt->pt_ev.sigev_signo <= 0 ||
-                         pt->pt_ev.sigev_signo >= NSIG))) {
-                       pool_put(&ptimer_pool, pt);
+                   (*fetch_event)(evp, &ev, sizeof(ev))) != 0) ||
+                   ((ev.sigev_notify < SIGEV_NONE) ||
+                       (ev.sigev_notify > SIGEV_SA)) ||
+                       (ev.sigev_notify == SIGEV_SIGNAL &&
+                        (ev.sigev_signo <= 0 ||
+                         ev.sigev_signo >= NSIG))) {
                        return (error ? error : EINVAL);
                }
        }
+       pt = pool_get(&ptimer_pool, PR_WAITOK);
+       *pt = (struct ptimer) { .pt_ev = ev, };
 
        /* Find a free timer slot, skipping those reserved for setitimer(). */
        mutex_spin_enter(&timer_lock);