qemu-s390x
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 1/9] s390x: smp: s390x dedicated smp parsing


From: Pierre Morel
Subject: Re: [PATCH v1 1/9] s390x: smp: s390x dedicated smp parsing
Date: Tue, 20 Jul 2021 10:33:18 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0



On 7/20/21 9:37 AM, Pierre Morel wrote:


On 7/19/21 5:52 PM, Daniel P. Berrangé wrote:
On Mon, Jul 19, 2021 at 05:43:29PM +0200, Cornelia Huck wrote:
(restored cc:s)

On Fri, Jul 16 2021, Pierre Morel <pmorel@linux.ibm.com> wrote:

On 7/16/21 11:14 AM, Daniel P. Berrangé wrote:
I increasingly worry that we're making a mistake by going down the
route of having custom smp_parse implementations per target, as this
is showing signs of inconsistent behaviour and error reportings. I
think the differences / restrictions have granularity at a different
level that is being tested in many cases too.

Whether threads != 1 is valid will likely vary depending on what
CPU model is chosen, rather than what architecture is chosen.
The same is true for dies != 1. We're not really checking this
closely even in x86 - for example I can request nonsense such
as a 25 year old i486 CPU model with hyperthreading and multiple
dies

    qemu-system-x86_64 -cpu 486 -smp 16,cores=4,dies=2,threads=2

Now that's what I'd call an upgrade :)


In this patch, there is no error reporting if the user specifies
dies != 1 or threads != 1 - it just silently ignores the request
which is not good.

yes, I should change this


Some machine types may have constraints on CPU sockets.

This can of course all be handled by custom smp_parse impls, but
this is ultimately going to lead to alot of duplicated and
inconsistent logic I fear.

I wonder if we would be better off having machine class callback
that can report topology constraints for the current configuration,
along lines ofsmp_constraints(MachineState *ms,

       smp_constraints(MachineState *ms,
                       int *max_sockets,
                       int *max_dies,
                       int *max_cores,
                       int *max_threads)

I find the idee good, but what about making it really machine agnostic
by removing names and using a generic

    smp_constraints(MachineState *ms,
            int *nb_levels,
            int *levels[]
            );

Level can be replaced by another name like container.
The machine could also provide the level/container names according to
its internal documentation.

In theory, this could give us more flexibility; however, wouldn't
that still mean that the core needs to have some knowledge of the
individual levels? We also have the command line parsing to consider,
and that one uses concrete names (which may or may not make sense,
depending on what machine you are trying to configure), and we'd still
have to map these to 'levels'.

Yeah, we need to deal with names in several places, so I don't think
abstracting it in one place is desirable, as it introduces the need
to convert between the two and potentially obscures the semantics.


Converting with names looks possible to me, every architecture can export a topology_name array or structure indicating names and other informations like the maximum possible count of entries at this level.

We have now the SMPConfiguration, couldn't we use it for this?

Hum, I think I over estimated my understanding of what json is capable of.
Sorry, forget it.


--
Pierre Morel
IBM Lab Boeblingen



reply via email to

[Prev in Thread] Current Thread [Next in Thread]