qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Alexey Kardashevskiy <aik@ozlabs.ru>
To: Alexander Graf <agraf@suse.de>
Cc: Mike Day <ncmike@ncultra.org>, Paul Mackerras <paulus@samba.org>,
	"qemu-ppc@nongnu.org" <qemu-ppc@nongnu.org>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [RFC PATCH v2] PPC: smp: autodetect numbers of threads per core
Date: Fri, 10 Jan 2014 12:39:03 +1100	[thread overview]
Message-ID: <52CF4F37.4050306@ozlabs.ru> (raw)
In-Reply-To: <D1E4D3FD-6107-46D1-9B3A-F40AD1F8E4AF@suse.de>

On 01/10/2014 10:40 AM, Alexander Graf wrote:
> 
> 
>> Am 09.01.2014 um 23:12 schrieb Alexey Kardashevskiy <aik@ozlabs.ru>:
>>
>>> On 01/10/2014 08:00 AM, Mike Day wrote:
>>>
>>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>>>
>>>>         /* compute missing values, prefer sockets over cores over threads */
>>>>         if (cpus == 0 || sockets == 0) {
>>>>             sockets = sockets > 0 ? sockets : 1;
>>>>             cores = cores > 0 ? cores : 1;
>>>> -            threads = threads > 0 ? threads : 1;
>>>> +            if (threads_max) {
>>>> +                if (threads > 0) {
>>>> +                    fprintf(stderr, "Use either threads or threads_max\n");
>>>> +                    exit(1);
>>>
>>> If you went ahead with the threads="max" string option you wouldn't need
>>> to check here for mutual excusivity and the user wouldn't need to worry
>>> about an extra command options.
>>
>>
>> Is this the only concern and the rest is fine and can go to upstream? If
>> so, I'll fix it and repost.
> 

> What if we make the max thread count a property of our cpu class? The
> we
can add a threads=max option which will be identical between kvm and tcg.


You lost me here :)
Right now the sequence is:
1. smp_parse
2. config_accelerator
3. machine_init

I proposed
1. config_accelerator - reads max threads from KVM (and initializes "host"
type)
2. smp_parse - does the parsing using smp_threads tweaked in 1)
3. machine_init - creates CPUs which may or may be not "host".

>From what you said I conclude that we add this "max" property only to the
"host" CPU type and then in smp_parse() we look if "host" CPU is going to
be used and if so, then we read "max" property from it and use it. But we
cannot read property for a class, only from an instance and it is not
created yet. And we also need to tweak the number depending on "compat".

And we still need "-smp ...threads_max" or "-smp ...threads=max" property
for "-smp".

Where am I wrong here?

> Overall I'm not yet fully convinced this whole idea is eventually going
> to improve the situation though.


My goal is to have a script which would automatically set threads to the
maximum value supported by the host hardware. Reading /proc/cpuinfo and
parsing for POWER6/7/8 and setting threads to 2/4/8 is lame for my taste
because what was the point of adding cap_ppc_smt at the first place then?



> 
> 
> Alex
> 
>>
>>
>>>> +                }
>>>> +                threads = smp_threads > 0 ? smp_threads : 1;
>>>> +            } else {
>>>> +                threads = threads > 0 ? threads : 1;
>>>> +            }
>>>>             if (cpus == 0) {
>>>>                 cpus = cores * threads * sockets;
>>>>             }
>>
>>
>> -- 
>> Alexey


-- 
Alexey

  reply	other threads:[~2014-01-10  1:39 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-09  5:34 [Qemu-devel] [RFC PATCH v2] PPC: smp: autodetect numbers of threads per core Alexey Kardashevskiy
2014-01-09 21:00 ` Mike Day
2014-01-09 22:12   ` Alexey Kardashevskiy
2014-01-09 23:40     ` Alexander Graf
2014-01-10  1:39       ` Alexey Kardashevskiy [this message]
2014-01-10 13:03         ` Mike Day
2014-01-10 13:28           ` Alexander Graf
2014-01-10 13:42             ` Alexey Kardashevskiy
2014-01-10 14:00               ` Alexander Graf
2014-01-10 14:13                 ` Alexey Kardashevskiy
2014-01-10 14:20                   ` Alexander Graf
2014-01-10 14:21                   ` Mike Day
2014-01-10 14:25                     ` Alexander Graf
2014-01-10 14:29                       ` Mike Day
2014-01-10 14:35                       ` Alexey Kardashevskiy
2014-01-10 14:12             ` Mike Day
2014-01-09 22:50 ` Scott Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52CF4F37.4050306@ozlabs.ru \
    --to=aik@ozlabs.ru \
    --cc=agraf@suse.de \
    --cc=ncmike@ncultra.org \
    --cc=paulus@samba.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).