[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] misra: address Rule 11.3 in spin_unlock_common()
- To: Dmytro Prokopchuk1 <dmytro_prokopchuk1@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
- Date: Mon, 3 Nov 2025 11:26:48 +0000
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; 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=bz6NHlqvVuX/qzIUaXLjAjYgOzXqFBuLRwKCx+QlN4A=; b=KaHwbo6Uj41Er9NoUuOTaUBNRVMRpsA2/uStg5y/sKt1NI5qh2AV7VZdDESmI9JB8v/ByJOml6y2SQrnKMzsQl+o1qqD3YgsXqHdX6A3xlPzVp3uHgt85FJvhj+YoMbCso+LcFBx3EOI314KlVzVr57ZxdMnoz7s8JiZ/tbGHt3QLJ1hANXane5+DfxSzeTetWwAR5S6LLR5tb1yemn7ORWaU0rK9Ymy3Ro1fpQNb59dfoOVawmc5hHziP0Z9NyIcm2f7/sExVHr2QyX4ZM/fMKOdhq10Q5PzsH0vNnkINGLhO82hPeZhtBrMO2fW3K5/Jj/afhYtZpZJnL9l8bMPg==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=TqZ71BPqDP1Vmb/rD0pj9WLKeXJ6FNG22j6pYgaZBaxc3jbeQav/SUUVRIU5dvJ5usOb4R5DanjVat0PljxM3ftSyW7odkO1XHzG/V66n5sgfxWO7/9IN/1pH60qrRv/VatR16YuVjM7W5a34SXJQb3Wbu6UsEFTeAXtWq2XyYrRdZ76cfT2ikqtYUz7q7Oqj/xvVWN2bp1Nct/zi6hSuxjg7TB5QQNzhe+yYnG+ceAguu8m9VLwXiq1UDYSP9RpZZE9Zl4zfmXq6tw17h8XNkYs7fKV1gAbhlChnUmC+7my2m1XuBZZDbWRywFjccBzQED9aonXbGrTc+kkvaHPDQ==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: Alistair Francis <alistair.francis@xxxxxxx>, Bob Eshleman <bobbyeshleman@xxxxxxxxx>, Connor Davis <connojdavis@xxxxxxxxx>, Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Julien Grall <julien@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>
- Delivery-date: Mon, 03 Nov 2025 11:27:15 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 03/11/2025 10:11 am, Dmytro Prokopchuk1 wrote:
> diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
> index 0389293b09..d9dc9998e6 100644
> --- a/xen/common/spinlock.c
> +++ b/xen/common/spinlock.c
> @@ -367,7 +367,8 @@ static void always_inline
> spin_unlock_common(spinlock_tickets_t *t,
> LOCK_PROFILE_REL;
> rel_lock(debug);
> arch_lock_release_barrier();
> - add_sized(&t->head, 1);
> + BUILD_BUG_ON(sizeof(t->head) != sizeof(uint16_t));
> + add_u16_sized(&t->head, 1);
This is an example where MISRA's opinions actively making the logic less
safe.
It's not possible for add_sized() to use the wrong type (as it
calculates it internally), whereas it's quite possible to update the
BUILD_BUG_ON() and fail to adjust the add.
Specifically, you've made it more complicated to reason about, and
created an opportunity to make an unsafe change where that opportunity
does not exist in the code as-is.
Furthermore, read and write atomic have exactly the same internal
pattern as add_sized(), so how does this not just kick the can down the
road?
~Andrew
|