Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <0eed9aa8-1229-daca-330c-9d71f2b0688d@oracle.com>
Date:   Fri, 1 Apr 2022 13:33:04 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.1
Subject: Re: [PATCH v5 4/8] crash: generic crash hotplug support
 infrastructure
Content-Language: en-US
To:     Baoquan He <bhe@redhat.com>
Cc:     linux-kernel@vger.kernel.org, x86@kernel.org,
        kexec@lists.infradead.org, ebiederm@xmission.com,
        dyoung@redhat.com, vgoyal@redhat.com, tglx@linutronix.de,
        mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
        hpa@zytor.com, nramas@linux.microsoft.com, thomas.lendacky@amd.com,
        robh@kernel.org, efault@gmx.de, rppt@kernel.org, david@redhat.com,
        konrad.wilk@oracle.com, boris.ostrovsky@oracle.com
References: <20220303162725.49640-1-eric.devolder@oracle.com>
 <20220303162725.49640-5-eric.devolder@oracle.com>
 <20220324133856.GC354864@MiWiFi-R3L-srv>
 <20220324134911.GA354791@MiWiFi-R3L-srv>
 <d12cd750-4f7e-46e8-8f31-66e29edbede9@oracle.com>
 <20220324143300.GD354864@MiWiFi-R3L-srv>
 <8b68f5b6-f6e0-ca61-56cf-69046edce58d@oracle.com>
 <6f59af6a-8a70-85ce-b36c-38eb31503c7d@oracle.com>
 <YkJciuCXKuU4a4gp@MiWiFi-R3L-srv>
From:   Eric DeVolder <eric.devolder@oracle.com>
In-Reply-To: <YkJciuCXKuU4a4gp@MiWiFi-R3L-srv>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?aXBucXgvb0xpNmhIMXI1Tjlvc2dEdGMxK241dHIrVFJ4OU1BUVljVVp2aVJo?=
 =?utf-8?B?c2xqRHRudXNoVzRCaU8yTTFqZ091RWtBK0w2NWVLYU93NnN3dXM1N1RCUTJw?=
 =?utf-8?B?R0hYd3V3eDRGeWR4ME9wRWtmNEx5YkxSZ0JRV20zR20vV1B0aVY3TFJhZm5q?=
 =?utf-8?B?V0doaEF6Rk5yUGpXNVpIMUdSdTQ0WFhQUW9PQXcwZVJEcmtnaDJmbGRFS2Rq?=
 =?utf-8?B?K2NyTjZMNEV4cUZ0dHFQV1VJa1NPOFpZempGNWsyVUozTlA2SzVxRTFYT2FS?=
 =?utf-8?B?a0FBT0NJVDJpY2l0bEhvQ2NQZkQwY2tHVXQ0dUxFRi9wSGpzYTJWTTBNenVE?=
 =?utf-8?B?cTlNWG5xTFBSdUIvY1o4b2J2cFJOWkpBN0JHMk9aUW12VUVGSFhzWnkzanZ4?=
 =?utf-8?B?VWQvQmxxaHJYalBhOHVBNUxlS2E2THFnTDdOM0JZRzJSWHM4ZHk0bDlkdVJk?=
 =?utf-8?B?eHhzWS9maGFmc1BrdXMyVm9leHU5Ykk1OXFYcmR6bUJEZ1k3b0IyTytnQVdS?=
 =?utf-8?B?RDhPRzd4ZEhJQ1RTOHlSb3pGNHc5WFpaN1JhQzVJdkVZRnRhRzE2a3JPTGhw?=
 =?utf-8?B?WUNub3NOUGVTNTBtcGZFTzFZNTkzYWdFTDZDUGRYMDF6cEp0bm1QbXJiTXRw?=
 =?utf-8?B?cW5wRXQrU0NRMzBidzdIUUh1ck9QOVFSOTVnSFhtYkkzQTRMb0NSMUw1ek5U?=
 =?utf-8?B?UjZXcWloNHJJVXJLc1kwNmlmU2ErN1g5VWZ0U2wrSWlXQ1J5bEEyN2xhMy9P?=
 =?utf-8?B?ZzZ0b0w5eExoekxDMUJINlJGZU1iSERrUWRjTnVDRHVVQndTL25uK0o1dHMr?=
 =?utf-8?B?Zmh4UHBHRkhIU0NzVytOWTRnWWdFbjhQY1JRNDRUVk1HWmt4dEdBY1FJL1FK?=
 =?utf-8?B?MDJId3JDY0s5c1JrZDJUUVZRTWRhNUdsVUdrYTVwd3VrWkxxRGJobmg2R01u?=
 =?utf-8?B?eVFoUGVqenkzTHRBL0dkU0VJRnEweFdZQlZpd1phc3NCbitJaFNZcm5jeFcv?=
 =?utf-8?B?VWpkR2hEWGVEclJFajl1a1c1M1ZNa3dhdFREK3hWMVBrSWZTQytpNE9JaHVK?=
 =?utf-8?B?R2tMVXZuYXJPVEhIVm4vNDMyWHNZZUtqTEJ1QSt5eXVQRjRJVlY3OGVjMHVE?=
 =?utf-8?B?UW9KeHVYaHZ4L0dFQlVjcm45d1VRNjIzZVNxUjRTMFA3N1I4MTRNMHB6K2NX?=
 =?utf-8?B?NUQzVmU2Q1JXR0FDd2hPMGxIYjhaRnBheWljd1NoZ1cwaHV0S3RraXNjVEtN?=
 =?utf-8?B?dUlwRWlJR3l6bVpwVFAyc2dvdVUrajN6eGREQ0ZXb0ZDRmpmS0dmSnpJMkVP?=
 =?utf-8?B?YThnSGZTdlRCZnNYUHY1WWFFbU4xVnFGV3VuVSt2cnZIb0VIN0Nqc0pFUWNI?=
 =?utf-8?B?K25aeENHN0RLRzNxSVM4Y25COUpsaWhRQ3hEUVdXbitkcjUyQXFEc05aek5q?=
 =?utf-8?B?M2xraFhRbzVyNUxuOXF3UjQ1K1VmeUsxV1cxUEFpVW5KN2FXbUhBb1B3R2xU?=
 =?utf-8?B?enRLd1RibytuMWlzNnVNN2xncldBV0FWZ1ZlbnJ5bjlNZFY4am9QamZYQ0lL?=
 =?utf-8?B?enVnRldUeHJXUzIyaW4xZkc2amFldHEyNkZOSElxUkJrZmdjb2MrSWxVcURo?=
 =?utf-8?B?bHJjbG9icVV1bkJqQk5YZEsybzgwYlVEaTVFWGJvUXIzSmlnM0Q3VDl5d0tT?=
 =?utf-8?B?WjBDUVE5WWhRVHNQZGZ3eVZlMHZRTVBnNUw1ZFVzZEovZ1ZkNVdqbnJqdzZR?=
 =?utf-8?B?Vzd3YmhxYnR5T0J0dTY5YmljdEtoR1ZkalhLaGJCQ3lobDI2ak1Ra3Irb3Fh?=
 =?utf-8?B?ZnpabWwxOVcxUzJiNzAzbll2ZG5YWHh4eEhHczVJTS90eElzRkZNM3lmN3Uy?=
 =?utf-8?B?Y3k1NTRhZ1QyblRMVDFqWmEydzgzOXYrYTZKcU1IZXVTWFA4V3gvVXJpL1Bu?=
 =?utf-8?B?bGtyN1BHVFdWOUlVeXd1bEVWd0VrZFJIbU13amt0R0dEUEJQdlJQbjFaUTdi?=
 =?utf-8?B?UXFKQ3pvQkNockIrU3FVUU1wdnQyZ1BVa25pNWJHRzJPdmxsTW8vN3lzOHNK?=
 =?utf-8?B?SjF6dWZ0WlpDd2lIQk8vN2U3V2U5bUlIbWh4ZU5YNHkvL3ByUHdUeTFNcVF0?=
 =?utf-8?B?NmFqWFJ3bnhEaVFmOW5Ea0hCS0UyVnVLZTlQUGFOeVhQb3RBeEtpSUI1c2Nw?=
 =?utf-8?B?NDY5djdZTXVSbFE5N05DdCtHM2NkdVVsZ2hSdDdKNG1jRGNWakY3R05ReS9F?=
 =?utf-8?B?aXNTSGVSdmI2L2dSZGNnK1lFUkQ2RGVGNDdFajUxZXhHTDhVSGQ5QUs4dEJQ?=
 =?utf-8?B?SCs1anhFT2JmR3JBZ3NwNUcvTG5nT21BWEQxRkxjWjdkNWZOOE43UjEyRnJ2?=
 =?utf-8?Q?r09olAF9YhhMeYC7eVcDXY0Obsqv6eSAfkfib?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4a8c167b-c110-4bee-1b55-08da140e11b9
X-MS-Exchange-CrossTenant-AuthSource: CO1PR10MB4531.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Apr 2022 18:33:08.7506
 (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: ltWo3aadIGgYRoaMyPdoz+QLqH+6/E5D8hIQhZQD+Tbw752MUS5SlEhKO34f2UpeD1v/9VRSNIKa5BHpSAUlJf8whKbzOFZK9wZt7bmYQKg=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8PR10MB3268
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.425,18.0.850
 definitions=2022-04-01_05:2022-03-30,2022-04-01 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 bulkscore=0 malwarescore=0
 mlxscore=0 phishscore=0 spamscore=0 suspectscore=0 mlxlogscore=914
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000
 definitions=main-2204010088
X-Proofpoint-ORIG-GUID: bVeESMbZGi7UYCaEcw5kOBrOiR4ogNJo
X-Proofpoint-GUID: bVeESMbZGi7UYCaEcw5kOBrOiR4ogNJo
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 3/28/22 20:10, Baoquan He wrote:
> On 03/28/22 at 11:08am, Eric DeVolder wrote:
>> Baoquan, a comment below.
>> eric
>>
>> On 3/24/22 09:37, Eric DeVolder wrote:
>>>
>>>
>>> On 3/24/22 09:33, Baoquan He wrote:
>>>> On 03/24/22 at 08:53am, Eric DeVolder wrote:
>>>>> Baoquan,
>>>>> Thanks, I've offered a minor correction below.
>>>>> eric
>>>>>
>>>>> On 3/24/22 08:49, Baoquan He wrote:
>>>>>> On 03/24/22 at 09:38pm, Baoquan He wrote:
>>>>>>> On 03/03/22 at 11:27am, Eric DeVolder wrote:
>>>>>>>> This patch introduces a generic crash hot plug/unplug infrastructure
>>>>>>>> for CPU and memory changes. Upon CPU and memory changes, a generic
>>>>>>>> crash_hotplug_handler() obtains the appropriate lock, does some
>>>>>>>> important house keeping and then dispatches the hot plug/unplug event
>>>>>>>> to the architecture specific arch_crash_hotplug_handler(), and when
>>>>>>>> that handler returns, the lock is released.
>>>>>>>>
>>>>>>>> This patch modifies crash_core.c to implement a subsys_initcall()
>>>>>>>> function that installs handlers for hot plug/unplug events. If CPU
>>>>>>>> hotplug is enabled, then cpuhp_setup_state() is invoked to register a
>>>>>>>> handler for CPU changes. Similarly, if memory hotplug is enabled, then
>>>>>>>> register_memory_notifier() is invoked to install a handler for memory
>>>>>>>> changes. These handlers in turn invoke the common generic handler
>>>>>>>> crash_hotplug_handler().
>>>>>>>>
>>>>>>>> On the CPU side, cpuhp_setup_state_nocalls() is invoked with parameter
>>>>>>>> CPUHP_AP_ONLINE_DYN. While this works, when a CPU is being unplugged,
>>>>>>>> the CPU still shows up in foreach_present_cpu() during the regeneration
>>>>>>>> of the new CPU list, thus the need to explicitly check and exclude the
>>>>>>>> soon-to-be offlined CPU in crash_prepare_elf64_headers().
>>>>>>>>
>>>>>>>> On the memory side, each un/plugged memory block passes through the
>>>>>>>> handler. For example, if a 1GiB DIMM is hotplugged, that generate 8
>>>>>>>> memory events, one for each 128MiB memblock.
>>>>>>>
>>>>>>> I rewrite the log as below with my understanding. Hope it's simpler to
>>>>>>> help people get what's going on here. Please consider to take if it's
>>>>>>> OK to you or adjust based on this. The code looks good to me.
>>>>>>>
>>>>>> Made some tuning:
>>>>>>
>>>>>> crash: add generic infrastructure for crash hotplug support
>>>>>>
>>>>>> Upon CPU and memory changes, a generic crash_hotplug_handler() is added
>>>>>> to dispatch the hot plug/unplug event to the architecture specific
>>>>>> arch_crash_hotplug_handler(). During the process, kexec_mutex need be
>>>>>> held.
>>>>>>
>>>>>> To support cpu hotplug, one callback pair are registered to capture
>>>>>> KEXEC_CRASH_HP_ADD_CPU and KEXEC_CRASH_HP_REMOVE_CPU events via
>>>>>> cpuhp_setup_state_nocalls().
>>>>> s/KEXEC_CRASH_HP_ADD}REMOVE_CPU/CPUHP_AP_ONLINE_DYN/ as the KEXEC_CRASH are the
>>>>> names I've introduced with this patch?
>>>>
>>>> Right.

Updated commit message.

>>>>
>>>> While checking it, I notice hp_action which you don't use actually.
>>>> Can you reconsider that part of design, the hp_action, the a, b
>>>> parameter passed to handler?
>>>
>>> Sure I can remove. I initially put in there as this was generic
>>> infrastructure and not sure if it would benefit others.
>>> eric
>>>
>>
>> Actually, I will keep the hp_action as the work by Sourabh Jain for PPC uses
>> the hp_action. I'll drop the a and b.
> 
> Sounds great.

Turns out hp_action and a are utilized, so I just left it alone. If you'd rather I remove b, I can 
do so.

> 
>>
>> Also, shall I post v6, or are you still looking at patches 7 and 8?
> 
> Will check today, thanks for the effort.
>