Re: [PATCH v2] x86/mm: fix cpu stuck issue in __change_page_attr_set_clr

[Dave Hansen]

On 07/04/2018 10:47 PM, Bin Yang wrote:
> When changing a 4K page attr inside 1G/2M large page range,
> __change_page_attr() will call try_preserve_large_page() to decide
> to split the big page or not. And try_preserve_large_page() will
> call static_protections() to check all 4K pages inside the large page
> range one by one. The check loop is very inefficient. In worst case,
> static_protections() will be called for 1G/4K (262144) times.

I wrote this before I read the entire thread and Thomas's analysis, but
I think we came to the same conclusion.  Here goes anyway:

Let me make sure I got this right.

1. free_init_pages() frees a single 4k page
2. set_memory_rw() tries to set that 4k range read-write
3. try_preserve_large_page()->_lookup_address_cpa() finds a 1GB
   page covering that 4k range.
4. We now go over the 1GB range (outside the 4k one we may or may not be
   changing) to see if it needs to be converted because of
   static_protections()

Correct?

#4 seems insane.  Why is it the new call's job to fix up protections
outside the 4k area which it was called to change?

There's an argument to be made that we need the static_protections()
loop for the area that try_to_preserve_large_page() is being called
over, but by no means do we need that for anything larger.

So, for this patch's purposes, if we do a change_page_attr(), walk down
to the PTE/PMD/PUD/... mapping that virtual address, and find that that
pte fulfills the 'prot' we are going for (adjusted for
static_protections() and for large page prot bits), we are done.
Totally done.  Done-aroo.