Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Message-ID: <0ec5f56e-6b55-627a-39c0-ff0a1680794d@oracle.com>
Date:   Fri, 22 Sep 2023 12:35:13 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH] kexec: change locking mechanism to a mutex
Content-Language: en-US
To:     Valentin Schneider <vschneid@redhat.com>,
        linux-kernel@vger.kernel.org, bhe@redhat.com, vgoyal@redhat.com,
        dyoung@redhat.com, ebiederm@xmission.com, kexec@lists.infradead.org
Cc:     sourabhjain@linux.ibm.com, konrad.wilk@oracle.com,
        boris.ostrovsky@oracle.com,
        Andrew Morton <akpm@linux-foundation.org>
References: <20230921215938.2192-1-eric.devolder@oracle.com>
 <xhsmh7coiuq8z.mognet@vschneid.remote.csb>
From:   Eric DeVolder <eric.devolder@oracle.com>
In-Reply-To: <xhsmh7coiuq8z.mognet@vschneid.remote.csb>
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?QjNrZ0NOSXdaLyt3YnQzTHUzL1c5b2cyd1JOZnV5THRBVmlLYWFsNGlJOVJl?=
 =?utf-8?B?b09oQmFHMUJ4cmFNRGhsYTNERHgwZ1l4MnFzUzlHV0N6ZFQzeWJHK1BlcTUw?=
 =?utf-8?B?QXNsTzdtbllGTVpJU2R6TDRldnR2aG9jblpKelAyUklQRmZNT0h2K2lSZzdr?=
 =?utf-8?B?dU1GSHB1VnlMaFl1ZDZXQThWaUVydmI3dnVpTlFvY0p5TjRMUkpkbmFUcmN6?=
 =?utf-8?B?SGtQbFJHSkY0T2s3aFpIRzZSTkFnTnZFZU4xTklHQkI5RHpJVVZQS1AxT0JZ?=
 =?utf-8?B?N3REdHE4aTA5UjFnbTFBeWJWMjI0NG1tVDZFRWc4cHZHMjd5bjd0bTZqUTQ2?=
 =?utf-8?B?em9kZDc2dlNvbHBWRTIvNHJ4aXZFK0ZjUmNuZUNSMXNDenkxN2s4VklaVjBG?=
 =?utf-8?B?VWY4d1JvekhRRzdkbEhOeEJZTEhYak9TNk5Oc0RhNzhybmR1SUk2QjJ5YWdp?=
 =?utf-8?B?MitibDVuY1RLalh3V1pxVWR4LzJZQzFBWU5MMEhLSzh3ZGV2U0RUeDlsK2JT?=
 =?utf-8?B?eXoveXE4YjZRWXRsZUZzeWcxVTlzdmZjTm9PZ0tMTGZYdnl0d1VVR3F4Rmty?=
 =?utf-8?B?OEw0Zm9nKythbjBFWmp2Z3Z3YjFOVk1uZE9XeXI3UGlqK25tLzNuUFVkMTU2?=
 =?utf-8?B?UVJUVFEyVDFmU05hT1BDak1ib0FXSDZaTG00NlY4VEpzTStHSW5uTnJIejFP?=
 =?utf-8?B?OTVYaTcvR1B4UGpWSEVZa3UwdytaTEJMV2tpRzk0M1cxS1FZeVBTRUhabzMw?=
 =?utf-8?B?bTRjdkV1WmluRDZEdlljRnI1M21RSEtNejdkK2hlSXp4YlNKM1VoWUVoTmpo?=
 =?utf-8?B?M3ZVUEQ1V2YySW9pYVNMVjI2OVRVMFEzME1MeFJLdW5zekc5WVJmSUt0dFBY?=
 =?utf-8?B?cGhMNWJER29ZUUdaRWFJUmloajlHdThpNU1peVI4ak0zNFlhUCtzU3hZdkk4?=
 =?utf-8?B?RU50dlpLZ3VYZGx4WDc4dDk3ak0xckZRNUxDWGkzNXNtdW5Ld09EbGFqQVlH?=
 =?utf-8?B?enF1UjVPZHh2dGJJQlI2NmlHdDV4MkViZHRoTEhYcUlSUFQ3bnBjM0JVUzYr?=
 =?utf-8?B?SlVBc3htQTR6aG5VMG9aY0FYTFl0WnVHeDVWcFEzaW1tVzhRWmdZQm9wQW0y?=
 =?utf-8?B?eThMSTdiUHMrNmxja2xPSi8yTE11Wkg2SERCUWQwaTFTRUpnVVJYVWlzWDNY?=
 =?utf-8?B?WTRrdjR4QXppYXRHSE5JK0Z2c0RtdWpJbUdjVXpTQUxZSGd6bmpzZG56UlJ0?=
 =?utf-8?B?enBjQVVUN2pnbHppVmlvV3RHYXcyTDU3bjdONUtWOStrTnQwV2dJV3p6VCtV?=
 =?utf-8?B?Ymo4Y0dTVlFzWjhyZjRsK2VPNDc5SzhjNGYzaS9RR29vWG92TWV0clRlZXgz?=
 =?utf-8?B?aTJvd1M4bzNpRjZlMEJtcGl1VjA2cDdveVIwWm1ERlIzM2VhUmxPenBhNzIy?=
 =?utf-8?B?NmtCSGxHN2FVTk5nbWVEbzd0bG1XWXdMajdTc0ZxSk5POURhTHQ2a3ZqcFFi?=
 =?utf-8?B?RVFxdUVFNXFLZy92RDg4UlNFNFZWQWZzaUc1b2loZEVvNkxGVndCeG4waWw2?=
 =?utf-8?B?Um83LzZLSTlMOHhCRzVjbVQzTUJEeEpDMDBRM040c1Zqc0YrVGFUd3NLRmJa?=
 =?utf-8?B?dmtYbi9oZmhzNmRSSlBFR0l1UTNVU2pQZk02UlRuOHBPL0N1KzIzWUI4Smlo?=
 =?utf-8?B?eGFQNzh1aSt4Y043Zy9sdTJXb0RCTEtLQ2dWYm12UHBLSEVMQjgrc2h2djUz?=
 =?utf-8?B?MmFzN1I4bVY1NnVYTmNZVlNPckh3dktYR3BjK0lmWGc3VFkyY0I0L3NoWFNi?=
 =?utf-8?B?Y1ZIamREVWpGVWlQR21qZTVuM3hubGZ6UGE5SktId0JBYUpFZGhWV0ZRaGts?=
 =?utf-8?B?WFhacTd1M3pQVlhFaWk2N05MeERVRzhsUkxuUTBxU2szUytEdzYzNnMzeGcr?=
 =?utf-8?B?Q1dQdnVxYnR4RUpPb2pUNlM2ejJ4Wk5tWFpnTERoQjVldkFyTEVvMVAwRzBH?=
 =?utf-8?B?QzkxTXBQMDFJSTlTSmwzalV5SzRnU2VCZkQvWHozNTR1SFhXQlVNYWJsZlJj?=
 =?utf-8?B?VHRaRUdXUlJyaCs2Smp6Rm5WdXZWY2hBUkFFM0FhdVBmTDd1bHgyVzJldTIx?=
 =?utf-8?B?Qk54T25PNGFtTkVUdXExKzN1bzEvRXI3TlA2eFFXV2k5bXNYTVArUnNkK1Nj?=
 =?utf-8?B?a0E9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?utf-8?B?UXVkRi9KN0Q0cTF1dEprOWJacE9DdkQ0SEFwZmdmSW05eEVxTGxqblg0SDlU?=
 =?utf-8?B?N2gvK1NGbXphZlR5WXRIdXFFNVlnK2YzemZqVE53ZmlGOTdoVTZaN1hSVjRG?=
 =?utf-8?B?RHRzcTFqV2FlVDhJUFYzS29vMzhYWXNlWXBaRlBzMW5SV0hvMmZNbk4yZkt3?=
 =?utf-8?B?T3FrUTlCam9HN0dnMVJPcCs3cjE4VWRHY1FiN29aQURuQkFNM1N6eTRCL0ZS?=
 =?utf-8?B?RUVYK0RGeUhNek54SEdoQmxmR1dzcHAyNHV6R01UVHVzRVppKzMvK0FWY0Nw?=
 =?utf-8?B?cHppVUN3ZnBPaU5yTUNTUnZIMytxVmlzK1RQRE5kbWwzWDhtSm5nSXcrQklE?=
 =?utf-8?B?UklNREhHaTNlb1dNMkdVY0dzSjhieENFUmVkWG5udjFoY3ZGd2lhaHFYcHlX?=
 =?utf-8?B?NkUxR3BiaTduQmhHWUJhWEVScjY5N1BhWXh5Z3BiVWF5bjhMSlhOWEF2Sy9G?=
 =?utf-8?B?YUwvMDhMQ285M1Q5NHRwRjlUQnd3dnNMaEFHU3hFRVpyQUxrenVUMDBoUC84?=
 =?utf-8?B?SVVwRUNGRnd4aVhaZEpLbWxNaUFuNEFkZjBOTkoyMnpzNks0YVBHVFpaNlQ5?=
 =?utf-8?B?Q3VDNTdaRUxYYUFtbjRjOUVxZ1FTZmt3VGxFQUZhY2pTWDV3MG1ZM0hsUUV1?=
 =?utf-8?B?Sy9DN3RSR2pXM2UyWk9QZFJ2NnBZb3JvbEFKcXNFeXJxYUZzWUVPSnRlWTVw?=
 =?utf-8?B?NUNPdVpyRzh1TzB2bkl0OENLY2E0aTlFZUNpRDREMkttU2hFN0RXenZubG1Z?=
 =?utf-8?B?WHhEMEdmcXQvaE9ML0owM0dMMmYzZkFOc28wMndGV0FPd0JXeE9rQ3pZWUJh?=
 =?utf-8?B?aWtMTnRsYTlpTnAyVDJQeTNNeEh0aXlWbXJ2Vnl5aU42bkdxZzZsd2loUUls?=
 =?utf-8?B?WE5MampJTmZGSmlWZFUwNjFGMEw3bnZmOTFpYmQ0Q0cyaW8yQ01zR2tVSUIv?=
 =?utf-8?B?SzRQR3dmSVgyWHBYOVFPbUYxK3VsWUpVOHpkRmthUEFhZ0ViLzVJZEsrUkV2?=
 =?utf-8?B?NktRUU9xeDJRSGQ1Q3RZNjdRajJGQTNmRFRLMTJGdWVFTGdKeWlDRW1HalVH?=
 =?utf-8?B?L0cxcVlqUmN2bStwcXlpMld3ellLT2lNUUFQSGtyOHFwcTc2cEJWNzBPQ2cw?=
 =?utf-8?B?elBOWjd6Y08xUW45MTNlRTlRSXkvWDNHMHFORVdVK2dTSXpuNHVPcFZHTUJl?=
 =?utf-8?B?aWRjRk1YYjAwMi82ZElkTEJmcGp6VFhvbXhLM3AvNDJxNmlNVGdSOCtzOHIw?=
 =?utf-8?B?VWk0VDE3ZVNRcVZGK0J5REJta29ocjV1Tno0dE1ETlBmbUdKZz09?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c6786419-a262-4260-4c8b-08dbbb924971
X-MS-Exchange-CrossTenant-AuthSource: CO1PR10MB4531.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Sep 2023 17:35:17.6363
 (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: txN3wxg1gpGdpQtc9jg3Hhea0TaQFV7ZxZMbYFtm9a6qJVxnJHduK+e593i/cY+heJfcmQC0kwzk/pgkCLdcrUtU/oEfsisF+iNfnFUrsl8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR10MB5182
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.267,Aquarius:18.0.980,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-09-22_16,2023-09-21_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 mlxscore=0 mlxlogscore=808
 suspectscore=0 phishscore=0 bulkscore=0 malwarescore=0 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2309180000
 definitions=main-2309220151
X-Proofpoint-GUID: WE9P_h036LAAteAKXVvYW3LDxE1kHhfN
X-Proofpoint-ORIG-GUID: WE9P_h036LAAteAKXVvYW3LDxE1kHhfN
X-Spam-Status: No, score=-2.3 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Fri, 22 Sep 2023 10:35:59 -0700 (PDT)


On 9/22/23 11:28, Valentin Schneider wrote:
> On 21/09/23 17:59, Eric DeVolder wrote:
>> The design decision to use the atomic lock is described in the comment
>> from kexec_internal.h, cited above. However, examining the code of
>> __crash_kexec():
>>
>>          if (kexec_trylock()) {
>>                  if (kexec_crash_image) {
>>                          ...
>>                  }
>>                  kexec_unlock();
>>          }
>>
>> reveals that the use of kexec_trylock() here is actually a "best effort"
>> due to the atomic lock.  This atomic lock, prior to crash hotplug,
>> would almost always be assured (another kexec syscall could hold the lock
>> and prevent this, but that is about it).
>>
>> So at the point where the capture kernel would be invoked, if the lock
>> is not obtained, then kdump doesn't occur.
>>
>> It is possible to instead use a mutex with proper waiting, and utilize
>> mutex_trylock() as the "best effort" in __crash_kexec(). The use of a
>> mutex then avoids all the lock acquisition problems that were revealed
>> by the crash hotplug activity.
>>
> 
> @Dave thanks for the Cc, I'd have missed this otherwise.
> 
> 
> Prior to the atomic thingie, we actually had a mutex and did
> mutex_trylock() in __crash_kexec(). I'm a bit confused as this looks like a
> revert of
>    05c6257433b7 ("panic, kexec: make __crash_kexec() NMI safe")
> with just the helpers kept in - this doesn't seem to address any of the
> original issues regarding NMIs?
> 
> Sebastian raised some good points in [1] regarding these issues.
> The main hurdle pointed out there is, if we end up in the slowpath during
> the unlock, then we can can up acquiring the ->wait_lock which isn't NMI
> safe.
> 
> This is even worse on PREEMPT_RT, as both trylock and the unlock can end up
> acquiring the ->wait_lock.
> 
> [1]: https://lore.kernel.org/all/YqyZ%2FUf14qkYtMDX@linutronix.de/
> 
Having reviewed the references, it would seem that Baoquan's approach of a new
lock to handle the hotplug activity is the way to go?
Eric