On 2/25/21 9:49 AM, Axel Rasmussen wrote:
> On Wed, Feb 24, 2021 at 4:26 PM Mike Kravetz <mike.krav...@oracle.com> wrote:
>>
>> On 2/18/21 4:48 PM, Axel Rasmussen wrote:
>> <snip>
>>> @@ -401,8 +398,10 @@ vm_fault_t handle_userfault(struct vm_fault *vmf,
>>> unsigned long reason)
>>>
>>> BUG_ON(ctx->mm != mm);
>>>
>>> - VM_BUG_ON(reason & ~(VM_UFFD_MISSING|VM_UFFD_WP));
>>> - VM_BUG_ON(!(reason & VM_UFFD_MISSING) ^ !!(reason & VM_UFFD_WP));
>>> + /* Any unrecognized flag is a bug. */
>>> + VM_BUG_ON(reason & ~__VM_UFFD_FLAGS);
>>> + /* 0 or > 1 flags set is a bug; we expect exactly 1. */
>>> + VM_BUG_ON(!reason || !!(reason & (reason - 1)));
>>
>> I may be confused, but that seems to be checking for a flag value of 1
>> as opposed to one flag being set?
>
> (Assuming I implemented it correctly!) It's the logical negation of
> this trick:
> https://graphics.stanford.edu/~seander/bithacks.html#DetermineIfPowerOf2
> So, it's "VM_BUG_ON(reason is *not* a power of 2)".
>
> Maybe the double negation makes it overly confusing? It ought to be
> equivalent if we remove it and just say:
> VM_BUG_ON(!reason || (reason & (reason - 1)));
Sorry, my bad. In my mind I was thinking,
VM_BUG_ON(!reason || !!(reason && (reason - 1)));
--
Mike Kravetz
