Re: [PATCH] x86/mm/pat: fix effective RW computation in lookup_address_in_pgd_attr()

[Mike Rapoport]

On Thu, Jul 16, 2026 at 06:27:34AM -0700, Dave Hansen wrote:
> On 7/16/26 02:18, Mike Rapoport wrote:
> >> I don't really care how the issue is fixed, but this would result in less
> >> code.
> > But it will be slower 🙂 
> 
> What does "slower" mean here?
 
I've seen ~2% difference per set_memory call in instrumented cpa-test.

> Does it matter in _practice_? Does it measurably slow down something an
> end user might see?

Yes, if a user is loading/unloading a BPF program in a tight loop :)

We can also accumulate flags and push the masking to the final assignment
like Jürgen suggested earlier in this thread:

https://lore.kernel.org/all/f0d7dbe5-5472-409c-9f61-eeefe7d08591@xxxxxxxx/
 
I'd prefer to keep the current structure though as I'm planning to pull
lookup_address_in_pgd_attr() to generic code and replace bit checks with
pXd_write()/pXd_exec() helpers.

> Also, I thought we had tests for this gunk. But maybe we're only testing
> the leaf entry permissions or something and not the upper-level
> permissions. I guess we don't often muck with those so this is
> relatively unlikely to have hidden real bugs.
> 
> BTW, my pre-coffee brain struggled with the changelog here:
> 
> 	but _PAGE_RW is bit 1 while *rw only ever holds 0 or 1, so the
> 
> Maybe it was a 0 vs. 1 bit thing, but it still took me way too long.
> Could we do something like:
> 
> _PAGE_RW is 0x2. So consider the accumulation line:
> 
> 	rw &= pXd_flags(*pXd) & _PAGE_RW;
> 
> where rw=0x1 and the right side evaluates down to 0x2. It'll end up doing:
> 
> 	rw = 0x1 & 0x2
> 
> and rw always ends up 0.

Works for me.

-- 
Sincerely yours,
Mike.