Re: [GIT PULL] Bitmap patches for v5.19-rc1

[Linus Torvalds <torvalds@linux-foundation.org>]

On Fri, May 27, 2022 at 8:44 AM Yury Norov <yury.norov@gmail.com> wrote:
>
>       bitmap: add bitmap_weight_{cmp, eq, gt, ge, lt, le} functions

So honestly, I pulled this, looked at the code, and then unpulled it again.

This is not helping.

Making changes like this:

-       if (mm != current->active_mm || cpumask_weight(mm_cpumask(mm)) != 1) {
+       if (mm != current->active_mm || !cpumask_weight_eq(mm_cpumask(mm), 1)) {

only makes the code harder to understand.

And it gets worse:

-               if (cpumask_weight(mask) > cpumask_weight(sibling_mask(cpu)))
+               if (cpumask_weight_gt(mask, cpumask_weight(sibling_mask(cpu))))

is just disgusting. That original line is simple to read and makes
sense. That new replacement really makes you do "Whaa?"

Now, I understand that these kinds of helper functions could make for
slightly more efficient code in that you can break out of the bitmap
scanning early when you have found enough bits set. I get it.

BUT.

 (a) code legibility is really really important

 (b) the places I found this weren't that performance-critical.

 (c) in most cases, the bitmaps in question are one single word

so I'm unpulling this again.

Now, some other parts of the pull were clear improvements. For
example, the hyperv changes like this:

-               if (hc->var_cnt != bitmap_weight((unsigned long
*)&valid_bank_mask, 64))
+               if (hc->var_cnt != hweight64(valid_bank_mask))

were clear improvements where the old code was disgusting, and clearly
improved by the change.

But the "bitmap_weight_cmp()" functions (and the cpumask_weight_cmp()
ones) are just not a direction we want to go.

The special case of zero (ie "cpumask_weight() == 0" ->
"bitmap_empty()") is one thing: making that kind of change tends to
keep the code legible or even make it more understandable. So I didn't
mind that. But I do mind the pointlessly complex new arbitrary weight
comparisons, and the kind of mental cost they have.

There are people in the CS world that think "abstractions are always
good". Those people are very very wrong.

              Linus