From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54866) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adx44-00046e-De for qemu-devel@nongnu.org; Thu, 10 Mar 2016 04:39:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1adx40-0002qr-CB for qemu-devel@nongnu.org; Thu, 10 Mar 2016 04:39:28 -0500 Date: Thu, 10 Mar 2016 10:39:13 +0100 From: Igor Mammedov Message-ID: <20160310103913.2070bdf7@nial.brq.redhat.com> In-Reply-To: <20160309234226.GO22546@voom.fritz.box> References: <1457074461-14285-1-git-send-email-bharata@linux.vnet.ibm.com> <1457074461-14285-6-git-send-email-bharata@linux.vnet.ibm.com> <20160304113845.667c19ad@nial.brq.redhat.com> <20160304110253.GB5054@in.ibm.com> <20160304190720.4d64abc4@nial.brq.redhat.com> <20160307033655.GD22546@voom.fritz.box> <20160307112929.7578fa9b@nial.brq.redhat.com> <20160308042627.GV22546@voom.fritz.box> <20160309114053.58ebd9fe@nial.brq.redhat.com> <20160309234226.GO22546@voom.fritz.box> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH v1 05/10] cpu: Abstract CPU core type List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: mjrosato@linux.vnet.ibm.com, thuth@redhat.com, pkrempa@redhat.com, ehabkost@redhat.com, aik@ozlabs.ru, armbru@redhat.com, qemu-devel@nongnu.org, agraf@suse.de, borntraeger@de.ibm.com, qemu-ppc@nongnu.org, Bharata B Rao , g@voom.fritz.box, pbonzini@redhat.com, mdroth@linux.vnet.ibm.com, afaerber@suse.de On Thu, 10 Mar 2016 10:42:26 +1100 David Gibson wrote: > On Wed, Mar 09, 2016 at 11:40:53AM +0100, Igor Mammedov wrote: > > On Tue, 8 Mar 2016 15:26:27 +1100 > > David Gibson wrote: > > > On Mon, Mar 07, 2016 at 11:29:29AM +0100, Igor Mammedov wrote: > > > > On Mon, 7 Mar 2016 14:36:55 +1100 > > > > David Gibson wrote: > [snip] > > > > > > > > on top of that I'd add numeric 'threads' property to base class so > > > > > > > > all derived cores would inherit it. > > > > > > > > > > > > > > > > Then as easy integration with -smp threads=x, a machine could push > > > > > > > > a global variable 'cpu-core.threads=[smp_threads]' which would > > > > > > > > make every created cpu-core object to have threads set > > > > > > > > at instance_init() time (device_init). > > > > > > > > > > > > > > > > That way user won't have to specify 'threads=y' for every > > > > > > > > device_add spapr-core,core=x > > > > > > > > as it will be taken from global property 'cpu-core.threads' > > > > > > > > but if user wishes he/she still could override global by explicitly > > > > > > > > providing thread property at device_add time: > > > > > > > > device_add spapr-core,core=x,threads=y > > > > > > > > > > > > > > > > wrt this series it would mean, instead of creating threads in property > > > > > > > > setter, delaying threads creation to core.realize() time, > > > > > > > > but since realize is allowed to fail it should be fine do so. > > > > > > > > > > > > > > Ok that would suit us as there are two properties on which thread creation > > > > > > > is dependent upon: nr_threads and cpu_model. If thread objects can be > > > > > > > created at core realize time, then we don't have to resort to the ugliness > > > > > > > of creating the threads from either of the property setters. I always > > > > > > > assumed that we shouldn't be creating objects from realize, but if that > > > > > > > is fine, it is good. > > > > > > since realize is allowed to fail, it should be safe from hotplug pov > > > > > > to create internal objects there, as far as proper cleanups are done > > > > > > for failure path. > > > > > > > > > [...] > > > > > I'm not clear from the above if you're also intending to move at least > > > > > the adding of the threads as child properties is supposed to go into > > > > > the base type, > > > > I'm not sure that I've got question, could you please rephrase? > > > > > > So, it seems like we're agreed that moving the nr_threads property to > > > the base type is a good idea. > > > > > > My question is, do we also move the object_property_add_child() calls > > > for each thread to the base type (possibly via a helper function or > > > method the base type provides to derived types)? > > I can't think of a reason to do so, > > why can't subtype-core.realize() do it? > > It can, but I'm always suspicious of boilerplate stuff that every > subtype *has* to do in order to work properly. > > > What would one gain creating callbacks and calling them from base class? > > Enforcing - or at least making as easy as possible - consistency in > the child object naming. > I guess a patch would help to understand if it's worth of effort of adding extra callbacks in base class.