[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] xen: Swap order of actions in the FREE*() macros
- To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
- From: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
- Date: Thu, 8 Feb 2024 15:06:28 +0000
- Accept-language: en-GB, en-US
- Arc-authentication-results: i=2; mx.microsoft.com 1; spf=pass (sender ip is 63.35.35.123) smtp.rcpttodomain=lists.xenproject.org smtp.mailfrom=arm.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com; arc=pass (0 oda=1 ltdi=1 spf=[1,1,smtp.mailfrom=arm.com] dkim=[1,1,header.d=arm.com] dmarc=[1,1,header.from=arm.com])
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
- Arc-message-signature: i=2; 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=ROFXKZ8pa30sestvhraDOKnrZn0OrdE8qBJgGskW+o8=; b=b++lmlYEHIu8GMQjhEiX+4uCCwPq9FeUkUwNrQVET4vbu2GiXyF6EWuEfZw9uUWbxzdwhnKsl2If/fD4wGHE/Lw5Ws9ZrNtTurbplSpD+TgDgxd224MwWcIOtMPNNG1fJG7PfsNZV9ZHbKdWVJG48aLEgkaxyi8gnLuJhv+5rDRZimC151emgPyP2Tfk7zq71+xNocTbkEfNtBgzReSOTDvujB6GdmGttIachZ2lxerZMCMJS4sj5SblUVgan5h+08HXezeITOKcJGp1vykhqwyWrE0KaRU8xonyy0nW5IDFYeBu0f7XZMV2zluV0ao2ur9+xynKJuCqj+NjXNDaaQ==
- 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=ROFXKZ8pa30sestvhraDOKnrZn0OrdE8qBJgGskW+o8=; b=QKJqcOqtMEQcB0Ve/5m+OHSUFaPhnT/4CRnDDUuVxWe5a1H4EJsWu5IU5aw40U0LEd+dd52v7pgekPW/3klgcJakt2KMPsryD+vKjygUqhUrs6hrpdgqYvbZPNPQgHp3aW+arFZZxQbgWTg3U4QNfH9JwIiyJ2CJa5FytbUkY7V1J45dgYYawBoCe5mi3rlypySgeeSM3qCGWWeubx8/WffB4xgc7R+Cj6zC37O52irnl9FY9TGEud4LgvDdlUqCbm8T8H4d1bqdlXSRDUuYM8dF65G3mQsbcfDVuQtfBaR3aNLbGala2zdyGW2y8qr2TMcthnz3Vz3HJ89dhR7I/g==
- Arc-seal: i=2; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=pass; b=ngQuuuJPYzPpoKaLJbulVBUuCYt5veGPA6k0NTWEdIVK1XU6HMwB00fVKV/kPjPzKu0n/PXzG8K2bwH08tvkiZ5OkXvShGSHnUf+kLRTK9qFd+uJytu4vZDDhp3lBqk+nW7JqYTLiAr4JJhGR28xYxIPR3SP5nsW/hXfe/ZiuC+CiUdbOD5DK9lcNfn+wnwkikuNAdz7Cox79KthlO5/ymLCatwa9ugd6o9QyaCVlhO1OURO4Icil0rWwjTai2BmMHZCfssY/7vyGNdFSfMgZxpyZasapicdXmNyDxiEQrQXoRbay49Y1rZI1PFtpaIYy6aazm/z4IZ9AqV0FKU/iQ==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=dm6PFpSOJxzAztFFgwaeMduO5KJs2d7sZLgKEKpuaOaB07jtZoBvDRn+gDXU+vWadjR+4XXN3VkmIPYIRRLXBoYXRMTIlqBzPec+02oSrHhNX42MfjSK8j55EYI78ZQYdnGj9rj4z3BzIRRQhbuY8WLUKlrKd4cXVguHEufw3RiXSKyVf2Xsu1+cRW3/dEpG2F/jBqU/kjfffyZAJSefBXbr2DAbW1JO0DCIxiQdUUTfgovhS4Wt2epFdnQ8KaBqW5eJs2+fyXx3SaBoq5U7aSB1bLLRvqOx9KZfQu/IFEFAkPPygjOksggOJLr6QHZW9AhDGmpqgp+g6XieGv3R9g==
- Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
- Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxx>, Jan Beulich <JBeulich@xxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Julien Grall <julien@xxxxxxx>
- Delivery-date: Thu, 08 Feb 2024 15:07:02 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
- Nodisclaimer: true
- Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
- Thread-index: AQHaVXBn1d4D59eCl0GZpSsSk57lq7EAldsA
- Thread-topic: [PATCH] xen: Swap order of actions in the FREE*() macros
Hi Andrew,
> On 2 Feb 2024, at 00:39, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>
> Wherever possible, it is a good idea to NULL out the visible reference to an
> object prior to freeing it. The FREE*() macros already collect together both
> parts, making it easy to adjust.
>
> This has a marginal code generation improvement, as some of the calls to the
> free() function can be tailcall optimised.
>
Could you improve a bit the commit message and explain a bit why it is a good
idea
and also how the code might be improved because i do not get it ?
Cheers
Bertrand
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: George Dunlap <George.Dunlap@xxxxxxxxxx>
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Wei Liu <wl@xxxxxxx>
> CC: Julien Grall <julien@xxxxxxx>
> ---
> xen/include/xen/mm.h | 3 ++-
> xen/include/xen/xmalloc.h | 7 ++++---
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
> index 3d9b2d05a5c8..044f3f3b19c8 100644
> --- a/xen/include/xen/mm.h
> +++ b/xen/include/xen/mm.h
> @@ -92,8 +92,9 @@ bool scrub_free_pages(void);
>
> /* Free an allocation, and zero the pointer to it. */
> #define FREE_XENHEAP_PAGES(p, o) do { \
> - free_xenheap_pages(p, o); \
> + void *_ptr_ = (p); \
> (p) = NULL; \
> + free_xenheap_pages(_ptr_, o); \
> } while ( false )
> #define FREE_XENHEAP_PAGE(p) FREE_XENHEAP_PAGES(p, 0)
>
> diff --git a/xen/include/xen/xmalloc.h b/xen/include/xen/xmalloc.h
> index 9ecddbff5e00..1b88a83be879 100644
> --- a/xen/include/xen/xmalloc.h
> +++ b/xen/include/xen/xmalloc.h
> @@ -66,9 +66,10 @@
> extern void xfree(void *p);
>
> /* Free an allocation, and zero the pointer to it. */
> -#define XFREE(p) do { \
> - xfree(p); \
> - (p) = NULL; \
> +#define XFREE(p) do { \
> + void *_ptr_ = (p); \
> + (p) = NULL; \
> + xfree(_ptr_); \
> } while ( false )
>
> /* Underlying functions */
>
> base-commit: 3f819af8a796c0e2f798dd301ec8c3f8cccbc9fc
> --
> 2.30.2
>
>
|
