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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK1f24=PnZ-Q=TrG9+SKWhwNHF_=EbVBWVcexqsEU-dctrAOFA@mail.gmail.com>
Date: Sat, 27 Apr 2024 12:06:43 +0800
From: Lance Yang <ioworker0@...il.com>
To: David Hildenbrand <david@...hat.com>
Cc: Zi Yan <ziy@...dia.com>, Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org, 
	"Matthew Wilcox (Oracle)" <willy@...radead.org>, Yang Shi <shy828301@...il.com>, 
	Ryan Roberts <ryan.roberts@....com>, Barry Song <21cnbao@...il.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5] mm/rmap: do not add fully unmapped large folio to
 deferred split list

On Sat, Apr 27, 2024 at 4:16 AM David Hildenbrand <david@...hat.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
>