Re: [PATCH v5] mm/rmap: do not add fully unmapped large folio to deferred split list

[Lance Yang <ioworker0@gmail.com>]

On Sat, Apr 27, 2024 at 4:16 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 26.04.24 21:20, Zi Yan wrote:
> > On 26 Apr 2024, at 15:08, David Hildenbrand wrote:
[...]
> >>> +   bool partially_mapped = false;
[...]
> >>> +
> >>> +           partially_mapped = !!nr && !!atomic_read(mapped);
> >>
> >> Nit: The && should remove the need for both !!.
> >
> > My impression was that !! is needed to convert from int to bool and I do
> > find "!!int && !!int" use in the kernel.
>
> I might be wrong about this, but if you wouldn't write

I think you're correct.

>
>         if (!!nr && !!atomic_read(mapped))
>
> then
>
> bool partially_mapped = nr && atomic_read(mapped);
>
> is sufficient.

+1

>
> && would make sure that the result is either 0 or 1, which
> you can store safely in a bool, no matter which underlying type
> is used to store that value.
>
> But I *think* nowdays, the compiler will always handle that
> correctly, even without the "&&" (ever since C99 added _Bool).
>
> Likely, also
>
>         bool partially_mapped = nr & atomic_read(mapped);
>
> Would nowadays work, but looks stupid.
>
>
> Related: https://lkml.org/lkml/2013/8/31/138
>
> ---
> #include <stdio.h>
> #include <stdbool.h>
> #include <stdint.h>
> #include <inttypes.h>
>
> volatile uint64_t a = 0x8000000000000000ull;
>
> void main (void) {
>          printf("uint64_t a = a: 0x%" PRIx64 "\n", a);
>
>          int i1 = a;
>          printf("int i1 = a: %d\n", i1);
>
>          int i2 = !!a;
>          printf("int i2 = !!a: %d\n", i2);
>
>          bool b1 = a;
>          printf("bool b1 = a: %d\n", b1);
>
>          bool b2 = !!a;
>          printf("bool b2 = !!a: %d\n", b2);
> }
> ---
> $ ./test
> uint64_t a = a: 0x8000000000000000
> int i1 = a: 0
> int i2 = !!a: 1
> bool b1 = a: 1
> bool b2 = !!a: 1
> ---
>
> Note that if bool would be defined as "int", you would need the !!, otherwise you
> would lose information.

Agreed. We need to be careful in this case.

>
> But even for b1, the gcc generates now:
>
>   40118c:       48 8b 05 7d 2e 00 00    mov    0x2e7d(%rip),%rax        # 404010 <a>
>   401193:       48 85 c0                test   %rax,%rax
>   401196:       0f 95 c0                setne  %al
>
>
> My stdbool.h contains
>
> #define bool    _Bool
>
> And I think C99 added _Bool that makes that work.
>
> But I didn't read the standard, and it's time for the weekend :)

I just read the C99 and found some interesting information as follows:

6.3.1.2 Boolean type
    When any *scalar value* is converted to _Bool, the result is 0 if the
    value compares equal to 0; otherwise, the result is 1.

6.2.5 Types
    21. Arithmetic types and pointer types are collectively called *scalar
    types*. Array and structure types are collectively called aggregate types.

6.5.13 Logical AND operator
    Semantics
    The && operator shall yield 1 if both of its operands compare unequal to
    0; otherwise, it yields 0. The result has type int.

6.5.10 Bitwise AND operator
    Constraints
    Each of the operands shall have integer type.
    Semantics
    The result of the binary & operator is the bitwise AND of the operands
    (that is, each bit in the result is set if and only if each of the
corresponding
    bits in the converted operands is set).

&& would ensure that the result is either 0 or 1, as David said, so no worries.

We defined partially_mapped as a bool(_Bool). IIUC, "partially_mapped
= int & int;"
would work correctly as well. However, "partially_mapped = long &
int;" might not.

Using && would be nicer as David suggested :p

Thanks,
Lance

>
> --
> Cheers,
>
> David / dhildenb
>