From owner-freebsd-arch Thu Jul 11 8: 1: 7 2002 Delivered-To: freebsd-arch@freebsd.org Received: from mx1.FreeBSD.org (mx1.FreeBSD.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id A5DE237B43B; Thu, 11 Jul 2002 08:00:55 -0700 (PDT) Received: from prism.flugsvamp.com (66-191-112-47.mad.wi.charter.com [66.191.112.47]) by mx1.FreeBSD.org (Postfix) with ESMTP id 893EE43E31; Thu, 11 Jul 2002 08:00:53 -0700 (PDT) (envelope-from jlemon@flugsvamp.com) Received: (from jlemon@localhost) by prism.flugsvamp.com (8.11.6/8.11.6) id g6BExDa58451; Thu, 11 Jul 2002 09:59:13 -0500 (CDT) (envelope-from jlemon) Date: Thu, 11 Jul 2002 09:59:13 -0500 From: Jonathan Lemon To: Archie Cobbs Cc: Jonathan Lemon , John Baldwin , davidx@viasoft.com.cn, freebsd-arch@FreeBSD.ORG, julian@elischer.org Subject: Re: Timeout and SMP race Message-ID: <20020711095913.H65393@prism.flugsvamp.com> References: <20020710171552.F65393@prism.flugsvamp.com> <200207110025.g6B0PHA30341@arch20m.dellroad.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Mailer: Mutt 1.0pre2i In-Reply-To: <200207110025.g6B0PHA30341@arch20m.dellroad.org> Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Wed, Jul 10, 2002 at 05:25:17PM -0700, Archie Cobbs wrote: > Jonathan Lemon writes: > > > extern int timer_start(timer_handle_t *handlep, mutex_t *mutexp, > > > timer_func_t tfunc, void *arg, u_int delay, > > > int flags); > > > extern void timer_cancel(timer_handle_t *handlep); > > > extern int timer_remaining(timer_handle_t handle, u_int *delayp); > > > > It seems to me that we can achieve the same functionality without > > changing the callout API too much. You mention that the mutex (if supplied) > > will be acquired before calling tfunc. This means that it has to be > > stored somewhere, presumably in the callout structure itself. > > > > The callout() consumers typically allocate their own storage, so perhaps > > we should add an (optional) pointer to a mutex to the callout structure, > > where the mutex is obtained/released before the callout function is made. > > Yep, that would work too.. essentially it's the same thing. > > If you're doing that, why not just store the mutex itself in the > callout structure, rather than a pointer to it? I guess if you did > that then you would then need some kind of flag that says whether > to use it or not. Or.. maybe there would be some way for the timer > code to tell if the mutex has been initialized or not, and use this > to decide whether to use it or not? Well, the other day, I was thinking we have this current API: callout_init(struct callout *c, int mpsafe) where 'mpsafe' indicates whether the callout code should grab the Giant lock or not. It wouldn't seem too much of a stretch to change this to: callout_init(struct callout *c, struct mtx *m) (perhaps redundant, since *m will be stored in the callout structure). Now, this removes the special cases dealing with 'Giant' lock in the softclock, since those callers who need the Giant lock would be able to specify it themselves. For TCP timeouts, all of the timeouts would point to the lock on the structure: /* cut down pseudocode */ struct inp_tp { struct mtx inp_mtx; struct callout inp_tp_rexmt; struct callout inp_tp_persist; struct callout inp_tp_keep; struct callout inp_tp_delack; } callout_init(&inp->inp_tp_rexmt, &inp->inp_mtx); callout_init(&inp->inp_tp_persist, &inp->inp_mtx); callout_init(&inp->inp_tp_keep, &inp->inp_mtx); callout_init(&inp->inp_tp_delack, &inp->inp_mtx); So TCP code, which grabs the inp_mtx for the duration, would be protected against the timer, and no additional locking is needed. The timer code would grab the appropriate lock before calling the timeout. I wouldn't want an individual mutex in each timer, as by storing a pointer, I can obtain/protect the appropriate resource, which extends further than just the callout structure. -- Jonathan To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message