From: David Hildenbrand <david@redhat.com>
To: Lance Yang <ioworker0@gmail.com>
Cc: Zi Yan <ziy@nvidia.com>,
Andrew Morton <akpm@linux-foundation.org>,
linux-mm@kvack.org,
"Matthew Wilcox (Oracle)" <willy@infradead.org>,
Yang Shi <shy828301@gmail.com>,
Ryan Roberts <ryan.roberts@arm.com>,
Barry Song <21cnbao@gmail.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5] mm/rmap: do not add fully unmapped large folio to deferred split list
Date: Sat, 27 Apr 2024 08:51:04 +0200 [thread overview]
Message-ID: <436316d3-7fee-4843-93f3-1c07860566c8@redhat.com> (raw)
In-Reply-To: <CAK1f24=PnZ-Q=TrG9+SKWhwNHF_=EbVBWVcexqsEU-dctrAOFA@mail.gmail.com>
On 27.04.24 06:06, Lance Yang wrote:
> 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.
>
My example was flawed: I wanted to express that "if any bit is set, the
bool value will be 1. That works for "| vs ||" but not for "& vs &&",
obviously :)
> 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.
Implicit type conversion would convert "long & int" to "long & long"
first, which should work just fine. I think really most concerns
regarding the bool type are due to < C99 not supporting _Bool.
Great weekend everybody!
--
Cheers,
David / dhildenb
next prev parent reply other threads:[~2024-04-27 6:51 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-26 19:02 [PATCH v5] mm/rmap: do not add fully unmapped large folio to deferred split list Zi Yan
2024-04-26 19:08 ` David Hildenbrand
2024-04-26 19:20 ` Zi Yan
2024-04-26 20:15 ` David Hildenbrand
2024-04-26 20:22 ` Zi Yan
2024-04-27 4:06 ` Lance Yang
2024-04-27 6:51 ` David Hildenbrand [this message]
2024-04-27 9:32 ` Barry Song
2024-04-26 20:42 ` Yang Shi
2024-04-27 4:09 ` Lance Yang
2024-05-01 13:24 ` Alexander Gordeev
2024-05-01 13:38 ` Zi Yan
2024-05-01 15:54 ` David Hildenbrand
2024-05-02 13:18 ` Alexander Gordeev
2024-05-02 13:20 ` Zi Yan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=436316d3-7fee-4843-93f3-1c07860566c8@redhat.com \
--to=david@redhat.com \
--cc=21cnbao@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=ioworker0@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=ryan.roberts@arm.com \
--cc=shy828301@gmail.com \
--cc=willy@infradead.org \
--cc=ziy@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.