Message-ID: <5a687e9c-f375-d9cc-f781-37cd5bedacb9@oracle.com>
Date:   Mon, 6 Dec 2021 08:39:53 -0500
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.3.2
Subject: Re: [PATCH] cpu/hotplug: Allow the CPU in CPU_UP_PREPARE state to be
 brought up again.
Content-Language: en-US
To:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        "Longpeng (Mike, Cloud Infrastructure Service Product Dept.)" 
        <longpeng2@huawei.com>, linux-kernel@vger.kernel.org,
        "Gonglei (Arei)" <arei.gonglei@huawei.com>, x86@kernel.org,
        xen-devel@lists.xenproject.org,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@kernel.org>,
        Valentin Schneider <valentin.schneider@arm.com>,
        Juergen Gross <jgross@suse.com>,
        Stefano Stabellini <sstabellini@kernel.org>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        "H. Peter Anvin" <hpa@zytor.com>
References: <20211122154714.xaoxok3fpk5bgznz@linutronix.de>
 <87y25djhaj.ffs@tglx> <4d04ea65-ea74-dcfd-5b0e-984b44f91961@oracle.com>
 <20211206112555.ubscfwwxnn5bnyte@linutronix.de>
From:   Boris Ostrovsky <boris.ostrovsky@oracle.com>
In-Reply-To: <20211206112555.ubscfwwxnn5bnyte@linutronix.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UUd4NXJMV1diY0o3aDdFMGlqeTBaUktsRjk3Y2FyNXJ5U0lEV09MTzNiU0hq?=
 =?utf-8?B?UjZHNk1xNSt4MjZCNXVFYU02NmE0VGJpOTZqUVA2Wlg2OXM1cmFTQ09Nd1pz?=
 =?utf-8?B?cjlmc0FBdUpKQlRHNmtSR2RvL0FDb0ZFYnU5TkhmWE9TaVhwTzRsZVhwZDll?=
 =?utf-8?B?VlRXSVdGaFlaaGFsaUVodENHUmp5VGlsblk3TG45VlFnUkErK1pXS3kvczJs?=
 =?utf-8?B?MlphZ1Y3VE1BSGxQZFpqQVZqaWZBdHFIS3ZYMkRCTE9UZVJkVmRyQnJjdHFq?=
 =?utf-8?B?QThyTUdtQzNjV2VzY1AxRktCNG9KTlpzdnlBR291ZThDYlhVSHVUQ0IveVZi?=
 =?utf-8?B?bE11dHZWVlkzMlRKdldTUjE2WXhZdFhkRE05R2JvM3BraWlpcXRwUkt6NnZL?=
 =?utf-8?B?bmZXaG8vQ2YyV28zbStWdnhaeEpCS01lMW9Pdkxvb2hBUldabmpBL1Vuelg2?=
 =?utf-8?B?V0R0NDVjaEIraHBjT3V3SW1YaTh4THhTYnpUL3RvY1lZM3FsSXNaMFd1SjNt?=
 =?utf-8?B?c2poZzJJb0tFT0locnNYZi82QmhjcU5zRS9aajZYak5QZzJCM3ZpVUNTS3lO?=
 =?utf-8?B?VlNoQ1lrUjhpU2tsb3dHU25LZUM2NkRlak84RVdFeUV3WUhldGI4UURvYkh3?=
 =?utf-8?B?T3gzZUZxNmJ1a2VwOHN4eFFJMW9iUnVyQ2tUeGwzQ1BLQXRjS0tOOUxuYURF?=
 =?utf-8?B?UGhha2l6bFR1UDNSOVJnZ2lQNlVDcUNYN0RvR2xyOEcyY24vdTJOdGl5dk5L?=
 =?utf-8?B?ampONjk1WXRWZi9mNlFmdnRVVk93VWdtSk9aMUhmc3g4Y2paZ1E4d0txRHdE?=
 =?utf-8?B?aEVVdjZRSDV4QXJhanNDN2kvZjExK05oRkJ6M1FicXF6VEFNdlN2SjNzNFhh?=
 =?utf-8?B?ZURXQzhPV0dPWFh0a3Vudy9pbzdmUUNPZGF1bXpPSi9zUFFFVzJNZ3lnbGp0?=
 =?utf-8?B?Q3FoMldHc2ovWWJRNjFOY29SZU13bHB1SzhLTmhOZUt6SjJRUHlWc3hydnRE?=
 =?utf-8?B?TXdLWjJBNExxVUhQMkhneHBQdnB3SzkvQXRsQ2IwcXBtclByWTlveVZIUWhw?=
 =?utf-8?B?ZzNMMFZMWDlTTkI4cTVFLzlPSVZkdThnd0hPdWtYdmtYb0dTR3hra2hOekFJ?=
 =?utf-8?B?Q2pGS0dJb3M2dkdXdmhUczJsWnBTMWd4cksvK2ZwbGUrME1CMTQ2bjFUNklv?=
 =?utf-8?B?UytiTURodE9iU2FFWWkxUlJZS1VmVithbVhJUGZHMFRCeGwveXRXbjZnSUZu?=
 =?utf-8?B?L0Zzdk5PRWtIUy95eEtoSlFuQmJ0bHFBUTkxcWswM21ER0ljODFrZnZFaVhy?=
 =?utf-8?B?a2syanErdnFZRFNVVFlBY05XRy9CSWFxRkcrbWZ3M2dxWUNuaXBlZGVVVnp3?=
 =?utf-8?B?ME52M1RLYmV0RzcxWWgyZU5raXpSYlZkdFBLeFJhNytNc0hxVVoyc092dndM?=
 =?utf-8?B?LzNuenNHRUc5RHRZTC9jSUh2YlN1Sm52eWxRL09DcHcrcUlTSkVPSEpuNFNt?=
 =?utf-8?B?ellZOWM4S3VSaURkSzFyYkNpQWwzbWNpRVFUV3B3eHRWSWI4Rmg3c0JtQ1N3?=
 =?utf-8?B?ZlJRTU9OQ3VLdksvUU1Cb3FTRC9uUVhqU2tSUDBzQnVKd0ltZWdHYi9GRTlp?=
 =?utf-8?B?TTFEOGUrTXhmWXByUVloN0VCM1c4SlpqYk9LY3BmNVdsZzVQek4yYXpTTEEy?=
 =?utf-8?B?aTlCWG1HZ0VVQmhaaXc2aCtVQUJZdnJMcHpWOWN5c1JFMTUxZUtiZGRmUCtN?=
 =?utf-8?B?Njl4bjNZSGdhOEs0K0I3dEd6N1k5L2ZkaGZtVDZHQjJGK0pYRHRzbE5IRXhP?=
 =?utf-8?B?cVdlb2hiaUlPQUhpY2hzOWMwWkhZQjVxY09mYXFWdnBDTXp4TDUyOTVzTjZn?=
 =?utf-8?B?ZWlZMzBGTHpRZ3JLR3ZVdUgzRWZqMEpJQnYrdmwwSWRPekVRYzBoS0lxVGJp?=
 =?utf-8?B?dHdwZmdBa0JNNHRDN29ERmJFczRnb2ZUK1phVnJjUUFCZmJjaC95Q0pLODB3?=
 =?utf-8?B?TjNoeEQrY3dWMVBMYkJtSDdCc3ZqUS93eE1JK1pFU2xEWG1VQzZ2VWVBV0RJ?=
 =?utf-8?B?SFlzVTdzY1hoVHpVSU1YK3J1bGlucWVVZ1U3TjdzWkVCcHBaWGp6WldQeHg3?=
 =?utf-8?B?b1hqRERXb0N4Ujl2amFZbUE2U2ZrQ1hvWlBnc2MwdHFRQUk0VGFEdmVQYmlY?=
 =?utf-8?B?NVFQZzF5NnRpSC92UEp6dmowcDFZR1BFbGdPWWN1RStrZllxOTZPU2JJKzMr?=
 =?utf-8?Q?uzE60qXnbkZf9r5cPVaKChwVvYh2ztyL/AR5PKKl7w=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 320bff4b-b5fe-4841-b299-08d9b8bde7bc
X-MS-Exchange-CrossTenant-AuthSource: BLAPR10MB5009.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Dec 2021 13:40:02.7720
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: pkjaqUObnPSxKDefcTwHWi/5Ke+PRss1uc4DUIp+cMtJ+q+vBI+/u1GsO8yKa6wye1IkhhJ+O54oq400EFJSxoAhEBA++kdyEw4bD9GAfRc=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL0PR10MB2978
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10189 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 mlxscore=0 phishscore=0
 malwarescore=0 spamscore=0 mlxlogscore=999 adultscore=0 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000
 definitions=main-2112060084
X-Proofpoint-ORIG-GUID: 3geAe5Ak03wp3KAmWXAZp0du-t9lWZEL
X-Proofpoint-GUID: 3geAe5Ak03wp3KAmWXAZp0du-t9lWZEL
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 12/6/21 6:25 AM, Sebastian Andrzej Siewior wrote:
> On 2021-11-24 21:17:34 [-0500], Boris Ostrovsky wrote:
>> On 11/24/21 5:54 PM, Thomas Gleixner wrote:
>>> Any comment from XEN folks?
>>
>> If memory allocation in cpu_initialize_context() fails we will not be
>> able to bring up the VCPU because xen_cpu_initialized_map bit at the
>> top of that routine will already have been set. We will BUG in
>> xen_pv_cpu_up() on second (presumably successful) attempt because
>> nothing for that VCPU would be initialized. This can in principle be
>> fixed by moving allocation to the top of the routine and freeing
>> context if the bit in the bitmap is already set.
>>
>>
>> Having said that, allocation really should not fail: for PV guests we
>> first bring max number of VCPUs up and then offline them down to
>> however many need to run. I think if we fail allocation during boot we
>> are going to have a really bad day anyway.
>>
> So can we keep the patch as-is or are some changes needed?


I think for the sake of completeness we could add


diff --git a/arch/x86/xen/smp_pv.c b/arch/x86/xen/smp_pv.c
index 6a8f3b53ab83..86368fcef466 100644
--- a/arch/x86/xen/smp_pv.c
+++ b/arch/x86/xen/smp_pv.c
@@ -277,8 +277,11 @@ cpu_initialize_context(unsigned int cpu, struct task_struct *idle)
                 return 0;

         ctxt = kzalloc(sizeof(*ctxt), GFP_KERNEL);
-       if (ctxt == NULL)
+       if (ctxt == NULL) {
+               cpumask_clear_cpu(cpu, xen_cpu_initialized_map);
+               cpumask_clear_cpu(cpu, cpu_callout_mask);
                 return -ENOMEM;
+       }

         gdt = get_cpu_gdt_rw(cpu);


-boris