On Tue, Jul 07, 2020 at 01:47:22PM +0300, Kirill A. Shutemov wrote:
>On Tue, Jul 07, 2020 at 09:38:56AM +0800, Wei Yang wrote:
>> On Mon, Jul 06, 2020 at 01:07:29PM +0300, Kirill A. Shutemov wrote:
>> >On Fri, Jun 26, 2020 at 09:52:15PM +0800, Wei Yang wrote:
>> >> Page tables is moved on the base of PMD. This requires both source
>> >> and destination range should meet the requirement.
>> >>
>> >> Current code works well since move_huge_pmd() and move_normal_pmd()
>> >> would check old_addr and new_addr again. And then return to move_ptes()
>> >> if the either of them is not aligned.
>> >>
>> >> In stead of calculating the extent separately, it is better to calculate
>> >> in one place, so we know it is not necessary to try move pmd. By doing
>> >> so, the logic seems a little clear.
>> >>
>> >> Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxxxxxxxxxx>
>> >> Tested-by: Dmitry Osipenko <digetx@xxxxxxxxx>
>> >> ---
>> >> mm/mremap.c | 6 +++---
>> >> 1 file changed, 3 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/mm/mremap.c b/mm/mremap.c
>> >> index de27b12c8a5a..a30b3e86cc99 100644
>> >> --- a/mm/mremap.c
>> >> +++ b/mm/mremap.c
>> >> @@ -258,6 +258,9 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
>> >> extent = next - old_addr;
>> >> if (extent > old_end - old_addr)
>> >> extent = old_end - old_addr;
>> >> + next = (new_addr + PMD_SIZE) & PMD_MASK;
>> >
>> >Please use round_up() for both 'next' calculations.
>> >
>>
>> I took another close look into this, seems this is not a good suggestion.
>>
>> round_up(new_addr, PMD_SIZE)
>>
>> would be new_addr when new_addr is PMD_SIZE aligned, which is not what we
>> expect.
>
>Maybe round_down(new_addr + PMD_SIZE, PMD_SIZE)?
>
To be honest, I don't like this which makes the code not that straight
forward. And when you look into the definition of pxd_addr_end(), they use the
format of ((addr + PXD_SIZE) & PXD_MASK) too.
I have another alternative to clean up this part with the help of
pmd_addr_end(). If you agree, I would like to append the following change in
next version to cleanup the next/extent staff especially.
Author: Wei Yang <richard.weiyang@xxxxxxxxxxxxxxxxx>
Date: Tue Jul 7 17:42:49 2020 +0800
mm/mremap: use pmd_addr_end to calculate extent
diff --git a/mm/mremap.c b/mm/mremap.c
index f5f17d050617..76e7fdf567c3 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -237,11 +237,12 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
unsigned long new_addr, unsigned long len,
bool need_rmap_locks)
{
- unsigned long extent, next, old_end;
+ unsigned long extent, old_next, new_next, old_end, new_end;
struct mmu_notifier_range range;
pmd_t *old_pmd, *new_pmd;
old_end = old_addr + len;
+ new_end = new_addr + len;
flush_cache_range(vma, old_addr, old_end);
mmu_notifier_range_init(&range, MMU_NOTIFY_UNMAP, 0, vma, vma->vm_mm,
@@ -250,14 +251,11 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
for (; old_addr < old_end; old_addr += extent, new_addr += extent) {
cond_resched();
- next = (old_addr + PMD_SIZE) & PMD_MASK;
- /* even if next overflowed, extent below will be ok */
- extent = next - old_addr;
- if (extent > old_end - old_addr)
- extent = old_end - old_addr;
- next = (new_addr + PMD_SIZE) & PMD_MASK;
- if (extent > next - new_addr)
- extent = next - new_addr;
+
+ old_next = pmd_addr_end(old_addr, old_end);
+ new_next = pmd_addr_end(new_addr, new_end);
+ extent = min((old_next - old_addr), (new_next - new_addr));
+
old_pmd = get_old_pmd(vma->vm_mm, old_addr);
if (!old_pmd)
continue;
>--
> Kirill A. Shutemov
--
Wei Yang
Help you, Help me
