Recent additions have undone prior tidying at the top of the file.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Bertrand Marquis <bertrand.marquis@arm.com>
CC: Michal Orzel <michal.orzel@amd.com>
CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>
CC: Shawn Anastasio <sanastasio@raptorengineering.com>
---
 xen/include/xen/bitops.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h
index ed6b9ccc724f..6f8e36f1c755 100644
--- a/xen/include/xen/bitops.h
+++ b/xen/include/xen/bitops.h
@@ -210,6 +210,8 @@ static always_inline bool test_bit(int nr, const volatile void *addr)
     test_bit(nr, addr);                                 \
 })
 
+/* --------------------- Please tidy above here --------------------- */
+
 static always_inline attr_const unsigned int ffs(unsigned int x)
 {
     if ( __builtin_constant_p(x) )
-- 
2.39.2

On Wed, 4 Sep 2024, Andrew Cooper wrote:
> Recent additions have undone prior tidying at the top of the file.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Hi Andrew,

I admit I don't understand the value and the meaning of the "Please tidy
above here" comments.

However, you got one just like this one committed with ea59e7d780d9 and
then removed by 5f7606c048f7. It looks like they are useful to you. As
you are the one doing the tiding, and given that you had one already
before:

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien@xen.org>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> CC: Bertrand Marquis <bertrand.marquis@arm.com>
> CC: Michal Orzel <michal.orzel@amd.com>
> CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> CC: Shawn Anastasio <sanastasio@raptorengineering.com>
> ---
>  xen/include/xen/bitops.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h
> index ed6b9ccc724f..6f8e36f1c755 100644
> --- a/xen/include/xen/bitops.h
> +++ b/xen/include/xen/bitops.h
> @@ -210,6 +210,8 @@ static always_inline bool test_bit(int nr, const volatile void *addr)
>      test_bit(nr, addr);                                 \
>  })
>  
> +/* --------------------- Please tidy above here --------------------- */
> +
>  static always_inline attr_const unsigned int ffs(unsigned int x)
>  {
>      if ( __builtin_constant_p(x) )
> -- 
> 2.39.2
> 

On 05.09.2024 00:55, Andrew Cooper wrote:
> Recent additions have undone prior tidying at the top of the file.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Looks like despite not getting any ack (nor comments) for this in earlier
rounds, you still want to keep it and you don't want to re-base subsequent
changes ahead. Which is a pity; I hoped I'd get around opening up another
controversy here. I'm afraid I disagree with the underlying thinking in
this particular case, or perhaps better with what I assume the underlying
thinking is. First: What is it that was undone? I can't spot it.

> --- a/xen/include/xen/bitops.h
> +++ b/xen/include/xen/bitops.h
> @@ -210,6 +210,8 @@ static always_inline bool test_bit(int nr, const volatile void *addr)
>      test_bit(nr, addr);                                 \
>  })
>  
> +/* --------------------- Please tidy above here --------------------- */
> +
>  static always_inline attr_const unsigned int ffs(unsigned int x)
>  {
>      if ( __builtin_constant_p(x) )

Second: How is the code above that marker any better or worse than the code
below the marker? (There are differences, yet that alone doesn't make the
earlier code untidy.)

Taken together: If you really think we need such markers, I'm afraid they
need to come with at least an outline of what wants tidying in which way.
Unless that's (sufficiently) obvious, like imo it is with the somewhat
similar (in intentions) marker in msr-index.h.

Without it being clear what wants doing, I think such markers would better
be omitted.

Jan