Re: CVS commit: src/sys

[Martin Husemann <martin%duskware.de@localhost>]

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);