Re: [PATCH v9 03/15] xen/bitops: implement fls{l}() in common logic

["Oleksii K." <oleksii.kurochko@xxxxxxxxx>]

On Wed, 2024-05-15 at 16:07 +0200, Jan Beulich wrote:
> On 15.05.2024 15:55, Oleksii K. wrote:
> > On Wed, 2024-05-15 at 11:09 +0200, Jan Beulich wrote:
> > > 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.
> > Do you mean that generic flsl() uses 'unsigned int' as a return
> > type,
> > but arch_flsl continue to use 'int'?
> 
> Yes.
> 
> > > 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?
> > Because Adnrew's patch with bitops.h for x86 changes to
> > always_inline,
> > so to be consistent, at least, for architecture.
> 
> And why not extend this to Arm?
No specific reason, just wanted to do minimal amount of necessary
changes. I'll do that in the the next patch version.

~ Oleksii