Re: [PATCH v3 1/5] xen/bitops: Reinstate the please tidy message

[Jan Beulich <jbeulich@xxxxxxxx>]

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@xxxxxxxxxx>

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