Xen-Devel Archive mirror
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Cc: "Stefano Stabellini" <sstabellini@kernel.org>,
	"Julien Grall" <julien@xen.org>,
	"Bertrand Marquis" <bertrand.marquis@arm.com>,
	"Michal Orzel" <michal.orzel@amd.com>,
	"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"George Dunlap" <george.dunlap@citrix.com>,
	"Shawn Anastasio" <sanastasio@raptorengineering.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH v9 03/15] xen/bitops: implement fls{l}() in common logic
Date: Wed, 15 May 2024 11:09:21 +0200	[thread overview]
Message-ID: <c16c0554-c376-41f4-bfe8-04c977702a04@suse.com> (raw)
In-Reply-To: <fe183f7e8ada7c3fb00ebf9b38f1fffffcc9c2d7.1714988096.git.oleksii.kurochko@gmail.com>

On 06.05.2024 12:15, Oleksii Kurochko wrote:
> Changes in V9:
>  - update return type of fls and flsl() to unsigned int to be aligned with other
>    bit ops.

But this then needs carrying through to ...

> --- a/xen/arch/arm/include/asm/arm64/bitops.h
> +++ b/xen/arch/arm/include/asm/arm64/bitops.h
> @@ -22,17 +22,15 @@ static /*__*/always_inline unsigned long __ffs(unsigned long word)
>   */
>  #define ffz(x)  __ffs(~(x))
>  
> -static inline int flsl(unsigned long x)
> +static inline int arch_flsl(unsigned long x)

... e.g. here. You don't want to introduce signed/unsigned mismatches.

Also why do you keep "inline" here, while ...

> --- a/xen/arch/x86/include/asm/bitops.h
> +++ b/xen/arch/x86/include/asm/bitops.h
> @@ -425,7 +425,7 @@ static always_inline unsigned int arch_ffsl(unsigned long x)
>   *
>   * This is defined the same way as ffs.
>   */
> -static inline int flsl(unsigned long x)
> +static always_inline int arch_flsl(unsigned long x)

... you switch to always_inline here?

(replying out of order)

> --- a/xen/arch/arm/include/asm/arm32/bitops.h
> +++ b/xen/arch/arm/include/asm/arm32/bitops.h
> @@ -1,7 +1,7 @@
>  #ifndef _ARM_ARM32_BITOPS_H
>  #define _ARM_ARM32_BITOPS_H
>  
> -#define flsl fls
> +#define arch_flsl fls

It's the Arm maintainers to ultimately judge, but I'd be inclined to suggest

#define arch_flsl arch_fls

instead. That's not only behaviorally closer to what was there before, but
also reduces (a tiny bit) the amount of work the compiler needs to carry out.

Jan


  reply	other threads:[~2024-05-15  9:09 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-06 10:15 [PATCH v9 00/15] Enable build of full Xen for RISC-V Oleksii Kurochko
2024-05-06 10:15 ` [PATCH v9 01/15] xen/riscv: disable unnecessary configs Oleksii Kurochko
2024-05-06 10:15 ` [PATCH v9 02/15] xen: introduce generic non-atomic test_*bit() Oleksii Kurochko
2024-05-15  8:52   ` Jan Beulich
2024-05-15 15:29     ` Oleksii K.
2024-05-15 15:41       ` Jan Beulich
2024-05-15 17:03         ` Oleksii K.
2024-05-16  7:04           ` Jan Beulich
2024-05-16 10:34             ` Oleksii K.
2024-05-16 10:49               ` Jan Beulich
2024-05-17  8:42                 ` Oleksii K.
2024-05-06 10:15 ` [PATCH v9 03/15] xen/bitops: implement fls{l}() in common logic Oleksii Kurochko
2024-05-15  9:09   ` Jan Beulich [this message]
2024-05-15 13:55     ` Oleksii K.
2024-05-15 14:07       ` Jan Beulich
2024-05-15 15:31         ` Oleksii K.
2024-05-17  9:06     ` Oleksii K.
2024-05-17  9:52       ` Jan Beulich
2024-05-06 10:15 ` [PATCH v9 04/15] xen/bitops: put __ffs() into linux compatible header Oleksii Kurochko
2024-05-15 14:34   ` Michal Orzel
2024-05-15 15:08   ` Rahul Singh
2024-05-06 10:15 ` [PATCH v9 05/15] xen/riscv: introduce bitops.h Oleksii Kurochko
2024-05-15  9:29   ` Jan Beulich
2024-05-06 10:15 ` [PATCH v9 06/15] xen/riscv: introduce cmpxchg.h Oleksii Kurochko
2024-05-15  9:38   ` Jan Beulich
2024-05-06 10:15 ` [PATCH v9 07/15] xen/riscv: introduce atomic.h Oleksii Kurochko
2024-05-15  9:49   ` Jan Beulich
2024-05-15 13:59     ` Oleksii K.
2024-05-06 10:15 ` [PATCH v9 08/15] xen/riscv: introduce monitor.h Oleksii Kurochko
2024-05-06 10:15 ` [PATCH v9 09/15] xen/riscv: add definition of __read_mostly Oleksii Kurochko
2024-05-06 10:15 ` [PATCH v9 10/15] xen/riscv: add required things to current.h Oleksii Kurochko
2024-05-06 10:15 ` [PATCH v9 11/15] xen/riscv: add minimal stuff to mm.h to build full Xen Oleksii Kurochko
2024-05-06 10:15 ` [PATCH v9 12/15] xen/riscv: introduce vm_event_*() functions Oleksii Kurochko
2024-05-06 10:15 ` [PATCH v9 13/15] xen/riscv: add minimal amount of stubs to build full Xen Oleksii Kurochko
2024-05-06 10:15 ` [PATCH v9 14/15] xen/riscv: enable full Xen build Oleksii Kurochko
2024-05-06 10:15 ` [PATCH v9 15/15] xen/README: add compiler and binutils versions for RISC-V64 Oleksii Kurochko

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=c16c0554-c376-41f4-bfe8-04c977702a04@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=julien@xen.org \
    --cc=michal.orzel@amd.com \
    --cc=oleksii.kurochko@gmail.com \
    --cc=roger.pau@citrix.com \
    --cc=sanastasio@raptorengineering.com \
    --cc=sstabellini@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).