Re: [PATCH v4 2/4] mm: Add function to support extra actions on swap in/out

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 01/11/2017 09:56 AM, Dave Hansen wrote:
On 01/11/2017 08:12 AM, Khalid Aziz wrote:
+#ifndef set_swp_pte_at
+#define set_swp_pte_at(mm, addr, ptep, pte, oldpte)	\
+		set_pte_at(mm, addr, ptep, pte)
+#endif

BTW, thanks for the *much* improved description of the series.  This is
way easier to understand.

Your feedback was very helpful in getting me there.


I really don't think this is the interface we want, though.
set_swp_pte_at() is really doing *two* things:
1. Detecting _PAGE_MCD_4V and squirreling the MCD data away at swap-out
2. Reading back in the MCD data at swap-on

You're effectively using (!pte_none(pte) && !pte_present(pte)) to
determine whether you're at swap in or swap out time.  That's goofy, IMNHO.

It isn't obvious from the context, but this hunk is creating a migration
PTE.  Why is ADI tag manipulation needed?  We're just changing the
physical address of the underlying memory, but neither the
application-visible contents nor the tags are changing.

Memory controller associates tags with physical memory. MMU verifies these tags against the tag embedded in VA. If physical memory is moved, tags must move with it.


@@ -1539,7 +1539,7 @@ static int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 		swp_pte = swp_entry_to_pte(entry);
 		if (pte_soft_dirty(pteval))
 			swp_pte = pte_swp_mksoft_dirty(swp_pte);
-		set_pte_at(mm, address, pte, swp_pte);
+		set_swp_pte_at(mm, address, pte, swp_pte, pteval);
 	} else if (PageAnon(page)) {
 		swp_entry_t entry = { .val = page_private(page) };
 		pte_t swp_pte;

Which means you're down to a single call that does swap-out, and a
single call that does swap-in.  There's no reason to hide all your code
behind set_pte_at().

Just add a new arch-specific call that takes the VMA and the swap PTE
and stores the ADI bit in there, here:

@@ -1572,7 +1572,7 @@ static int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 		swp_pte = swp_entry_to_pte(entry);
 		if (pte_soft_dirty(pteval))
 			swp_pte = pte_swp_mksoft_dirty(swp_pte);
-		set_pte_at(mm, address, pte, swp_pte);
+		set_swp_pte_at(mm, address, pte, swp_pte, pteval);
 	} else

and in do_swap_page(), do the opposite with a second, new call.

That does sound simpler. I will give it more thought.

Thanks,
Khalid

--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel]     [Kernel Newbies]     [x86 Platform Driver]     [Netdev]     [Linux Wireless]     [Netfilter]     [Bugtraq]     [Linux Filesystems]     [Yosemite Discussion]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux