[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




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.