| Message ID | 20240904225530.3888315-2-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | xen/bitops: hweight() cleanup/improvements | expand |
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
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 >
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) )
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(+)