[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:24:08 +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=wHWYKZ9o0K97kUOYCXLO7HOm1sIUmATcrVHMotVnOP4=; b=N/2rkqEhuXFp8Nrn+i+XFcudZSbSEvnOiXHXLAJOD4D9SYe144F3Rqn81Wz8bfxz+MhDHkkbWd01ZLmAnxMBKEnoI8TQ8CX5Inixunl009aOzG8tliiwp6/xCiO78q+xozUNEUCWi9xRZ5niRhDn3YpCdOi8TgI2Nl8xXXdVfgzDGB3vW+H9tkg4HP5F7QrzyjbGx75AG1vjKTJSuzighUUXPxTtERx1uqyrtmU8o0ePf6hVXiislvOuSxaHPJlDZnuvDsoOxdyJMndk35evTXFQcs58/UfNVkdRTqMqwRB8W63HrPGSriDl6Poxld1cFadaw+rtTUuIepxBvjYN9A==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=GT7+TzpRkI48eP1VQ8fC2Ih8YTcAVhJzj3hGk3h5VporWEzE8oVpIW8i6VYlEIsozFDs7oHs0LyAwQwLqYZTVC0Q0F5PyeE1bN/RnNu2c7Lrl7UMocmqnTyg9WBlundGJ+m6N+RDYrSZ7bOSU4KcDhe/id4wtiguPcOfO+b4KG3nzBAgJjhTgomsGds+DITQxZ9W3g0gTi91M8wwtLVgPgt/zL27/nWiF65g7OitF/h8MI2DZACvp0DdQqTmt0bRaNcPbR28NFAw4HC091araoC60+rcNVQGirA/Y7fTf2now+Nt9O9UUH7Yx1dkPP18S3UksNZDEiIN199IjYN1Mw==
- 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:24:21 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 25.02.2022 09:55, Juergen Gross wrote:
> On 25.02.22 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?
>
> BTW, it might make sense to add another bool for the debug case to mark
> recursive lock usage. I don't think anything good can come from using a
> lock in both modes (recursive and non-recursive).
But beware of the coexisting paging_lock() and paging_lock_recursive().
Albeit I guess your comment was for spinlocks in general, not the
mm-lock machinery. Yet mentioning this reminds me of the page alloc
lock, which different paths acquire in different ways. So the bit
couldn't be a sticky one.
Jan
|