On 2/25/21 9:49 AM, Axel Rasmussen wrote: > On Wed, Feb 24, 2021 at 4:26 PM Mike Kravetz <mike.kravetz@xxxxxxxxxx> 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
