Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 18 Sep 2019 12:22:48 +0530
From:   "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
Subject: Re: [PATCH 0/2] pseries/hotplug: Change the default behaviour of
 cede_offline
To:     "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>,
        Michael Ellerman <mpe@ellerman.id.au>,
        Nathan Lynch <nathanl@linux.ibm.com>,
        Nicholas Piggin <npiggin@gmail.com>,
        Tyrel Datwyler <tyreld@linux.ibm.com>
Cc:     "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>,
        Kamalesh Babulal <kamaleshb@in.ibm.com>,
        linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
        Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
References: <1568284541-15169-1-git-send-email-ego@linux.vnet.ibm.com>
        <87r24ew5i0.fsf@mpe.ellerman.id.au>
In-Reply-To: <87r24ew5i0.fsf@mpe.ellerman.id.au>
MIME-Version: 1.0
User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid)
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8BIT
Message-Id: <1568788924.kxcnnog4r7.naveen@linux.ibm.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Michael Ellerman wrote:
> "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> writes:
>> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
>>
>> Currently on Pseries Linux Guests, the offlined CPU can be put to one
>> of the following two states:
>>    - Long term processor cede (also called extended cede)
>>    - Returned to the Hypervisor via RTAS "stop-self" call.
>>
>> This is controlled by the kernel boot parameter "cede_offline=on/off".
>>
>> By default the offlined CPUs enter extended cede.
> 
> Since commit 3aa565f53c39 ("powerpc/pseries: Add hooks to put the CPU into an appropriate offline state") (Nov 2009)
> 
> Which you wrote :)
> 
> Why was that wrong?
> 
>> The PHYP hypervisor considers CPUs in extended cede to be "active"
>> since the CPUs are still under the control fo the Linux Guests. Hence, when we change the
>> SMT modes by offlining the secondary CPUs, the PURR and the RWMR SPRs
>> will continue to count the values for offlined CPUs in extended cede
>> as if they are online.
>>
>> One of the expectations with PURR is that the for an interval of time,
>> the sum of the PURR increments across the online CPUs of a core should
>> equal the number of timebase ticks for that interval.
>>
>> This is currently not the case.
> 
> But why does that matter? It's just some accounting stuff, does it
> actually break something meaningful?

Yes, this broke lparstat at the very least (though its quite unfortunate 
we took so long to notice).

With SMT disabled, and under load:
  $ sudo lparstat 1 10

  System Configuration
  type=Shared mode=Uncapped smt=Off lcpu=2 mem=7759616 kB cpus=6 ent=1.00 

  %user  %sys %wait    %idle    physc %entc lbusy  vcsw phint
  ----- ----- -----    -----    ----- ----- ----- ----- -----
  100.00  0.00  0.00     0.00     1.10 110.00 100.00 128784460     0
  100.00  0.00  0.00     0.00     1.07 107.00 100.00 128784860     0
  100.00  0.00  0.00     0.00     1.07 107.00 100.00 128785260     0
  100.00  0.00  0.00     0.00     1.07 107.00 100.00 128785662     0
  100.00  0.00  0.00     0.00     1.07 107.00 100.00 128786062     0
  100.00  0.00  0.00     0.00     1.07 107.00 100.00 128786462     0
  100.00  0.00  0.00     0.00     1.07 107.00 100.00 128786862     0
  100.00  0.00  0.00     0.00     1.07 107.00 100.00 128787262     0
  100.00  0.00  0.00     0.00     1.07 107.00 100.00 128787664     0
  100.00  0.00  0.00     0.00     1.07 107.00 100.00 128788064     0


With cede_offline=off:
  $ sudo lparstat 1 10

  System Configuration
  type=Shared mode=Uncapped smt=Off lcpu=2 mem=7759616 kB cpus=6 ent=1.00 

  %user  %sys %wait    %idle    physc %entc lbusy  vcsw phint
  ----- ----- -----    -----    ----- ----- ----- ----- -----
  100.00  0.00  0.00     0.00     1.94 194.00 100.00 128961588     0
  100.00  0.00  0.00     0.00     1.91 191.00 100.00 128961988     0
  100.00  0.00  0.00     0.00      inf   inf 100.00 128962392     0
  100.00  0.00  0.00     0.00     1.91 191.00 100.00 128962792     0
  100.00  0.00  0.00     0.00     1.91 191.00 100.00 128963192     0
  100.00  0.00  0.00     0.00     1.91 191.00 100.00 128963592     0
  100.00  0.00  0.00     0.00     1.91 191.00 100.00 128963992     0
  100.00  0.00  0.00     0.00     1.91 191.00 100.00 128964392     0
  100.00  0.00  0.00     0.00     1.91 191.00 100.00 128964792     0
  100.00  0.00  0.00     0.00     1.91 191.00 100.00 128965194     0

[The 'inf' values there show a different bug]

Also, since we expose [S]PURR through sysfs, any tools that make use of 
that directly are also affected due to this.


- Naveen