Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934409AbXFGRdf (ORCPT ); Thu, 7 Jun 2007 13:33:35 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S933764AbXFGRdU (ORCPT ); Thu, 7 Jun 2007 13:33:20 -0400 Received: from wr-out-0506.google.com ([64.233.184.235]:35921 "EHLO wr-out-0506.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933268AbXFGRdS (ORCPT ); Thu, 7 Jun 2007 13:33:18 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=received:message-id:date:from:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references; b=PFUhxJdDhrDJfNgw+AQXg3CZW2+QdYNb6E/kFTa8nRW8nLAR9QL/LDyIWLjkidjdgPXRsARuoziQiQOIPi94KE3qVZnsCEi187VPLjvEkGswbvUss1xUbC74M0vgcJlShgGNTkgtImRdVSXcqd3LJT1A/cB5fkQNRWaXc4cUKwQ= Message-ID: Date: Thu, 7 Jun 2007 23:03:08 +0530 From: "Satyam Sharma" To: "Avi Kivity" Subject: Re: [patch] i386/x86_64: smp_call_function locking inconsistency Cc: "Heiko Carstens" , "Jan Glauber" , "David Miller" , akpm@osdl.org, mingo@elte.hu, ak@suse.de, schwidefsky@de.ibm.com, linux-kernel@vger.kernel.org, "Alan Cox" In-Reply-To: <46683EC4.4030604@qumranet.com> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <20070208203210.GB9798@osiris.ibm.com> <20070208.124328.88477956.davem@davemloft.net> <20070209084221.GA8259@osiris.boeblingen.de.ibm.com> <1171025838.5349.14.camel@localhost.localdomain> <20070607162743.GA9433@osiris.ibm.com> <46683EC4.4030604@qumranet.com> Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2089 Lines: 54 On 6/7/07, Avi Kivity wrote: > Satyam Sharma wrote: > > > > Oh wait, the on_one_cpu() patch proposes on UP: > > > > +static inline int on_one_cpu(int cpu, void (*func)(void *info), void > > *info, > > + int retry, int wait) > > +{ > > > > /* this needs a if (cpu == 0) check here, IMO */ > > > > + local_irq_disable(); > > + func(info); > > + local_irq_enable(); > > + return 0; > > > > /* else WARN and return -EINVAL; */ > > > > +} > > > > which is broken without the suggested additions, IMHO > > (this is what got me into this in the first place). There > > _is_ a difference between on_each_cpu() and the > > smp_call_function* semantics (as discussed on the other > > thread -- gargh! my mistake for opening this discussion up > > on so many threads), and in its current form on_one_cpu() > > has quite confused semantics, trying to mix the two. I guess > > on_one_cpu() would be better off simply being just an > > atomic wrapper over smp_processor_id() and > > smp_call_function_single() (which is the *real* issue that > > needs solving in the first place), and do it well. > > > > This is on UP, so (cpu == 0) is trivially true. Yes, the caller code might derive the value for the cpu arg in such a manner to always only ever yield 0 on UP. OTOH, WARN_ON(!...)'s are often added for such assumptions that are understood to be trivially true. Note that a warning for cpu != 0 would be perfectly justified, we'd clearly want to flag such (errant) users. Anyway, I guess another problem being tackled here is avoding #ifdef CONFIG_SMP's to mask calls to smp_call_function* (and thus on_cpu()) in kernel code(?) Avoiding putting WARN_ON() in the UP cases could be useful to minimize noise, in that case. It (and smp_call_function*) could still return -EINVAL for the invalid cases, though. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/