On 3/6/25 11:34, Lorenzo Stoakes wrote: > Finish refactoring the page table logic by threading the PMC state > throughout the operation, allowing us to control the operation as we go. > > Additionally, update the old_addr, new_addr fields in move_page_tables() > as we progress through the process making use of the fact we have this > state object now to track this. > > With these changes made, not only is the code far more readable, but we > can finally transmit state throughout the entire operation, which lays the > groundwork for sensibly making changes in future to how the mremap() > operation is performed. > > Additionally take the opportunity to refactor the means of determining the > progress of the operation, abstracting this to pmc_progress() and > simplifying the logic to make it clearer what's going on. > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> Reviewed-by: Vlastimil Babka <vbabka@xxxxxxx> Small nit: > > +/* > + * Should move_pgt_entry() acquire the rmap locks? This is either expressed in > + * the PMC, or overridden in the case of normal, larger page tables. > + */ > +static bool should_take_rmap_locks(struct pagetable_move_control *pmc, > + enum pgt_entry entry) > +{ > + if (pmc->need_rmap_locks) > + return true; > + > + switch (entry) { > + case NORMAL_PMD: > + case NORMAL_PUD: > + return true; > + default: > + return false; > + } > +} > + IMHO the "overriden" logic would be more obvious if we removed the initial "if" and just had default: return pmc->need_rmap_locks