From: Jan Beulich <jbeulich@suse.com>
To: Oleksii <oleksii.kurochko@gmail.com>
Cc: "Julien Grall" <julien@xen.org>,
"Andrew Cooper" <andrew.cooper3@citrix.com>,
"Stefano Stabellini" <sstabellini@kernel.org>,
"Gianluca Guida" <gianluca@rivosinc.com>,
"Bertrand Marquis" <bertrand.marquis@arm.com>,
"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>,
"George Dunlap" <george.dunlap@citrix.com>,
"Wei Liu" <wl@xen.org>, "Roger Pau Monné" <roger.pau@citrix.com>,
xen-devel@lists.xenproject.org
Subject: Re: [PATCH v6 2/4] xen: change <asm/bug.h> to <xen/bug.h>
Date: Thu, 9 Mar 2023 10:49:59 +0100 [thread overview]
Message-ID: <df9da60f-f443-7c88-1ae8-c2ecb0d2a879@suse.com> (raw)
In-Reply-To: <cfe7b79c02bd5a9686a3018654627cade1c9e2f9.camel@gmail.com>
On 08.03.2023 18:25, Oleksii wrote:
> On Wed, 2023-03-08 at 16:27 +0100, Jan Beulich wrote:
>> On 07.03.2023 16:50, Oleksii Kurochko wrote:
>>> --- a/xen/arch/arm/include/asm/bug.h
>>> +++ b/xen/arch/arm/include/asm/bug.h
>>> @@ -1,6 +1,8 @@
>>> #ifndef __ARM_BUG_H__
>>> #define __ARM_BUG_H__
>>>
>>> +#include <xen/types.h>
>>> +
>>> #if defined(CONFIG_ARM_32)
>>> # include <asm/arm32/bug.h>
>>> #elif defined(CONFIG_ARM_64)
>>> @@ -9,10 +11,16 @@
>>> # error "unknown ARM variant"
>>> #endif
>>>
>>> +#undef BUG_DISP_WIDTH
>>> +#undef BUG_LINE_LO_WIDTH
>>> +#undef BUG_LINE_HI_WIDTH
>>
>> Why? For Arm ...
>>
>>> #define BUG_DISP_WIDTH 24
>>> #define BUG_LINE_LO_WIDTH (31 - BUG_DISP_WIDTH)
>>> #define BUG_LINE_HI_WIDTH (31 - BUG_DISP_WIDTH)
>>
>> ... you could purge these as unused, even in a standalone prereq
>> patch.
>> And on x86 ...
>>
>>> --- a/xen/arch/x86/include/asm/bug.h
>>> +++ b/xen/arch/x86/include/asm/bug.h
>>> @@ -1,19 +1,18 @@
>>> #ifndef __X86_BUG_H__
>>> #define __X86_BUG_H__
>>>
>>> +#undef BUG_DISP_WIDTH
>>> +#undef BUG_LINE_LO_WIDTH
>>> +#undef BUG_LINE_HI_WIDTH
>>> +
>>> #define BUG_DISP_WIDTH 24
>>> #define BUG_LINE_LO_WIDTH (31 - BUG_DISP_WIDTH)
>>> #define BUG_LINE_HI_WIDTH (31 - BUG_DISP_WIDTH)
>>
>> ... there's no reason to #undef just to the #define again to the same
>> values. All of this would be removed in a subsequent patch anyway,
>> and
>> it's less code churn (with code nevertheless being correct) if you
>> get
>> rid of the #define-s right away (as iirc you had it in an earlier
>> version). If you agree then with these adjustments
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>
> It won't be compilation issue (redefinition) in the current one case
> because defines are the same as in <xen/bug.h> so it is possible to not
> add #undef in this patch.
Avoiding to add the #undef is the minimal approach. Yet as indicated I
think the #define-s should also be dropped right here (x86) and in a
prereq patch (Arm).
Jan
next prev parent reply other threads:[~2023-03-09 9:50 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-07 15:50 [PATCH v6 0/4] [PATCH v5 0/4] introduce generic implementation of macros from bug.h Oleksii Kurochko
2023-03-07 15:50 ` [PATCH v6 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME Oleksii Kurochko
2023-03-08 15:06 ` Jan Beulich
2023-03-08 17:18 ` Oleksii
2023-03-07 15:50 ` [PATCH v6 2/4] xen: change <asm/bug.h> to <xen/bug.h> Oleksii Kurochko
2023-03-08 15:27 ` Jan Beulich
2023-03-08 17:25 ` Oleksii
2023-03-09 9:49 ` Jan Beulich [this message]
2023-03-07 15:50 ` [PATCH v6 3/4] xen/arm: switch ARM to use generic implementation of bug.h Oleksii Kurochko
2023-03-07 15:50 ` [PATCH v6 4/4] xen/x86: switch x86 to use generic implemetation " Oleksii Kurochko
2023-03-08 15:33 ` Jan Beulich
2023-03-08 17:48 ` Oleksii
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=df9da60f-f443-7c88-1ae8-c2ecb0d2a879@suse.com \
--to=jbeulich@suse.com \
--cc=Volodymyr_Babchuk@epam.com \
--cc=andrew.cooper3@citrix.com \
--cc=bertrand.marquis@arm.com \
--cc=george.dunlap@citrix.com \
--cc=gianluca@rivosinc.com \
--cc=julien@xen.org \
--cc=oleksii.kurochko@gmail.com \
--cc=roger.pau@citrix.com \
--cc=sstabellini@kernel.org \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.