From owner-freebsd-current@FreeBSD.ORG Mon May 16 20:46:05 2011 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3E66E106564A; Mon, 16 May 2011 20:46:05 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 147068FC0C; Mon, 16 May 2011 20:46:05 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id A879546B09; Mon, 16 May 2011 16:46:04 -0400 (EDT) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 121298A050; Mon, 16 May 2011 16:46:04 -0400 (EDT) From: John Baldwin To: Andriy Gapon Date: Mon, 16 May 2011 16:09:21 -0400 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110325; KDE/4.5.5; amd64; ; ) References: <4DCD357D.6000109@FreeBSD.org> <201105161421.27665.jhb@freebsd.org> <4DD17AB3.1070606@FreeBSD.org> In-Reply-To: <4DD17AB3.1070606@FreeBSD.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201105161609.21898.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.6 (bigwig.baldwin.cx); Mon, 16 May 2011 16:46:04 -0400 (EDT) Cc: Max Laier , FreeBSD current , neel@freebsd.org, Peter Grehan Subject: Re: proposed smp_rendezvous change X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 May 2011 20:46:05 -0000 On Monday, May 16, 2011 3:27:47 pm Andriy Gapon wrote: > on 16/05/2011 21:21 John Baldwin said the following: > > How about this: > ... > > /* > > * Shared mutex to restrict busywaits between smp_rendezvous() and > > @@ -311,39 +312,62 @@ restart_cpus(cpumask_t map) > > void > > smp_rendezvous_action(void) > > { > > - void* local_func_arg = smp_rv_func_arg; > > - void (*local_setup_func)(void*) = smp_rv_setup_func; > > - void (*local_action_func)(void*) = smp_rv_action_func; > > - void (*local_teardown_func)(void*) = smp_rv_teardown_func; > > + void *local_func_arg; > > + void (*local_setup_func)(void*); > > + void (*local_action_func)(void*); > > + void (*local_teardown_func)(void*); > > + int generation; > > > > /* Ensure we have up-to-date values. */ > > atomic_add_acq_int(&smp_rv_waiters[0], 1); > > while (smp_rv_waiters[0] < smp_rv_ncpus) > > cpu_spinwait(); > > > > - /* setup function */ > > + /* Fetch rendezvous parameters after acquire barrier. */ > > + local_func_arg = smp_rv_func_arg; > > + local_setup_func = smp_rv_setup_func; > > + local_action_func = smp_rv_action_func; > > + local_teardown_func = smp_rv_teardown_func; > > I want to ask once again - please pretty please convince me that the above > cpu_spinwait() loop is really needed and, by extension, that the assignments > should be moved behind it. > Please :) Well, moving the assignments down is a style fix for one, and we can always remove the first rendezvous as a follow up if desired. However, suppose you have an arch where sending an IPI is not a barrier (this seems unlikely). In that case, the atomic_add_acq_int() will not succeed (and return) until it has seen the earlier write by the CPU initiating the rendezvous to clear smp_rv_waiters[0] to zero. The actual spin on the smp_rv_waiters[] value is not strictly necessary however and is probably just cut and pasted to match the other uses of values in the smp_rv_waiters[] array. (atomic_add_acq_int() could spin on architectures where it is implemented using compare-and-swap (e.g. sparc64) or locked-load conditional-store (e.g. Alpha).) -- John Baldwin