From patchwork Fri Jun 5 14:55:57 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rusty Russell X-Patchwork-Id: 28249 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n55F8XMC025055 for ; Fri, 5 Jun 2009 15:08:33 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754198AbZFEPIY (ORCPT ); Fri, 5 Jun 2009 11:08:24 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753220AbZFEPIX (ORCPT ); Fri, 5 Jun 2009 11:08:23 -0400 Received: from ozlabs.org ([203.10.76.45]:55226 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752672AbZFEPIV (ORCPT ); Fri, 5 Jun 2009 11:08:21 -0400 Received: from vivaldi.localnet (caffeine.cc.com.au [150.101.221.106]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPSA id A9275DDD01; Sat, 6 Jun 2009 01:08:22 +1000 (EST) From: Rusty Russell To: paulmck@linux.vnet.ibm.com Subject: Re: [RFC PATCH v2 00/19] virtual-bus Date: Sat, 6 Jun 2009 00:25:57 +0930 User-Agent: KMail/1.11.2 (Linux/2.6.28-11-generic; KDE/4.2.2; i686; ; ) Cc: Gregory Haskins , "Michael S. Tsirkin" , Avi Kivity , Gregory Haskins , linux-kernel@vger.kernel.org, agraf@suse.de, pmullaney@novell.com, pmorreale@novell.com, anthony@codemonkey.ws, netdev@vger.kernel.org, kvm@vger.kernel.org, bhutchings@solarflare.com, andi@firstfloor.org, gregkh@suse.de, herber@gondor.apana.org.au, chrisw@sous-sol.org, shemminger@vyatta.com References: <20090409155200.32740.19358.stgit@dev.haskins.net> <200906051425.02924.rusty@rustcorp.com.au> <20090605053010.GD7125@linux.vnet.ibm.com> In-Reply-To: <20090605053010.GD7125@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Disposition: inline Message-Id: <200906060025.57961.rusty@rustcorp.com.au> Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Fri, 5 Jun 2009 03:00:10 pm Paul E. McKenney wrote: > On Fri, Jun 05, 2009 at 02:25:01PM +0930, Rusty Russell wrote: > > + /* lg->eventfds is RCU-protected */ > > + preempt_disable(); > > Suggest changing to rcu_read_lock() to match the synchronize_rcu(). Ah yes, much better. As I was implementing it I warred with myself since lguest aims for simplicity above all else. But since we only ever add things to the array, RCU probably is simpler. > > + for (i = 0; i < cpu->lg->num_eventfds; i++) { > > + if (cpu->lg->eventfds[i].addr == cpu->pending_notify) { > > + eventfd_signal(cpu->lg->eventfds[i].event, 1); > > Shouldn't this be something like the following? > > p = rcu_dereference(cpu->lg->eventfds); > if (p[i].addr == cpu->pending_notify) { > eventfd_signal(p[i].event, 1); Hmm, need to read num_eventfds first, too. It doesn't matter if we get the old ->num_eventfds and the new ->eventfds, but the other way around would be bad. Here's the inter-diff: Thanks! Rusty. --- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/lguest/lguest_user.c b/drivers/lguest/lguest_user.c --- a/drivers/lguest/lguest_user.c +++ b/drivers/lguest/lguest_user.c @@ -39,18 +39,24 @@ static int break_guest_out(struct lg_cpu bool send_notify_to_eventfd(struct lg_cpu *cpu) { - unsigned int i; + unsigned int i, num; + struct lg_eventfds *eventfds; + + /* Make sure we grab the total number before accessing the array. */ + cpu->lg->num_eventfds = num; + rmb(); /* lg->eventfds is RCU-protected */ rcu_read_lock(); - for (i = 0; i < cpu->lg->num_eventfds; i++) { - if (cpu->lg->eventfds[i].addr == cpu->pending_notify) { - eventfd_signal(cpu->lg->eventfds[i].event, 1); + eventfds = rcu_dereference(cpu->lg->eventfds); + for (i = 0; i < num; i++) { + if (eventfds[i].addr == cpu->pending_notify) { + eventfd_signal(eventfds[i].event, 1); cpu->pending_notify = 0; break; } } - preempt_enable(); + rcu_read_unlock(); return cpu->pending_notify == 0; }