[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] xen/spinlock: merge recurse_cpu and debug.cpu fields in struct spinlock
- To: Juergen Gross <jgross@xxxxxxxx>
- From: Jan Beulich <jbeulich@xxxxxxxx>
- Date: Fri, 25 Feb 2022 10:13:51 +0100
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=TrTFREjytDbzqeDL8ZkSTClwJ54uo6X9ShAXL2biEHg=; b=EukByAmETfc/cFH2lXFS4fc/yxdVYzpRuZCMTvnG4zRIyDY6xoJpUNfpAS68ouPAsrBZsjTfrPSc45lFe6ZVScD5MibuPdViqDycYzJUlzNurgDeW/joa2OePeyY3NXNd/YZegOMzVXKF22MuKCO3aR8XJGY2CQTrkjpmm8n6dmEzMvuq6hy1FTsQ/TBlA0yFZ80FvIPLXhBs+TgQA8PkobG3qZTUZmaL5fKBxJEIQwyPbRvM4UFfjAkpkGuadjRVabjL7YmhQP6OgYJGQd+3MUZvFMbj6+wi5ocVphcEOH4KiWDW56lrEANPSnSdHB7biCCUfjzIh8qPfnBWDYGlw==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=R6NNch5Is1TkuOLcMWFvC166AZcQ1AX6BPbgKYHMNRq5VHUGxxXx31rGdPpUv3qnMV51qd1Zxk3JQTqioL8kOTD0Nj/jRkct54QubbKJT3jlqWWnSTRjravfzrdtz06wD2O4nI0R29AP5erJStpMAAl2JwM+ACoVCeC35ksyTXC2cTvAeFdGg43GfIU4tJ736Vfh4O/T65REnCwY7Qs7TQiZQmNMPn+ebmw9p9vQkg7+DQn9GAI6y6ee2nqYEpeYx028jTjjDE+so2TE5YfLtMoCzWyfApGCdB4gLAGpV6Xm46TqM9UkjVX81z7yMvYAWHkeiJdPzQwJ4RFKkFG1rg==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
- Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
- Delivery-date: Fri, 25 Feb 2022 09:14:16 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 25.02.2022 09:36, Juergen Gross wrote:
> On 24.02.22 17:11, Jan Beulich wrote:
>> On 24.02.2022 11:54, Juergen Gross wrote:
>>> --- a/xen/arch/x86/mm/mm-locks.h
>>> +++ b/xen/arch/x86/mm/mm-locks.h
>>> @@ -42,7 +42,7 @@ static inline void mm_lock_init(mm_lock_t *l)
>>>
>>> static inline bool mm_locked_by_me(const mm_lock_t *l)
>>> {
>>> - return (l->lock.recurse_cpu == current->processor);
>>> + return (l->lock.data.cpu == current->processor);
>>> }
>>
>> I see a fair risk with this: Behavior will now differ between debug and
>> non-debug builds. E.g. a livelock because of trying to acquire the same
>> lock again would not be noticed in a debug build if the acquire is
>> conditional upon this function's return value. I think this is the main
>> reason behind having two separate field, despite the apparent redundancy.
>
> You are aware that mm_locked_by_me() is used for recursive spinlocks
> only?
I will admit that it occurred to me only a while after writing the earlier
reply that it's used only with recursive locking, due to _mm_lock() indeed
using spin_lock_recursive() unconditionally (i.e. independent of its "rec"
parameter). Nevertheless I continue to have the vague recollection that
the duplication of the two fields was intentional and deemed necessary.
Jan
|