Re: [PATCH 3/3] powerpc: mm: optimize for the correctly placed page

2013-12-05 Thread Aneesh Kumar K.V
Liu Ping Fan kernelf...@gmail.com writes:

 The period check of _PAGE_NUMA can probably trigger the check on
 the correctly placed page. For this case, we can just insert hpte and
 do fast exception return.

I still don't understand why we need to handle numa faults in hash
page ? Are you trying to optimize the code path ? If so can you explain
the benefits ? Some numbers showing it is helping  ?



 Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
 ---
  arch/powerpc/mm/hash_utils_64.c | 34 +-
  1 file changed, 33 insertions(+), 1 deletion(-)

 diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
 index 9bf1195..735678c 100644
 --- a/arch/powerpc/mm/hash_utils_64.c
 +++ b/arch/powerpc/mm/hash_utils_64.c
 @@ -965,6 +965,10 @@ int hash_page(unsigned long ea, unsigned long access, 
 unsigned long trap)
   const struct cpumask *tmp;
   int rc, user_region = 0, local = 0;
   int psize, ssize;
 + pte_t old, new;
 + struct vm_area_struct *vma;
 + int page_nid, target_nid;
 + struct page *test_page;

   DBG_LOW(hash_page(ea=%016lx, access=%lx, trap=%lx\n,
   ea, access, trap);
 @@ -1033,12 +1037,40 @@ int hash_page(unsigned long ea, unsigned long access, 
 unsigned long trap)

   /* Get PTE and page size from page tables */
   ptep = find_linux_pte_or_hugepte(pgdir, ea, hugeshift);
 - if (ptep == NULL || !pte_present(*ptep) || pte_numa(*ptep)) {
 + if (ptep == NULL || !pte_present(*ptep)) {
   DBG_LOW( no PTE !\n);
   rc = 1;
   goto bail;
   }

 + old = pte_val(*ptep);
 + if (pte_numa(old)) {
 + /* If fail to lock, let do_page_fault() to handle it */
 + if (down_read_trylock(mm-mmap_sem)) {

hmm is that something we want to do in hash_page ?

 + vma = find_vma(mm, ea);
 + up_read(mm-mmap_sem);
 + test_page = pte_page(old);
 + page_nid = page_to_nid(test_page);
 + target_nid = numa_migrate_prep(test_page, vma, ea,
 + page_nid);
 + if (target_nid  0) {
 + new = pte_mknonnuma(old);
 + /* If ptep is modified under us,
 +  * just retry the access
 +  */
 + if (unlikely(cmpxchg(ptep, old, new) != old)) {
 + put_page(test_page);
 + return 0;
 + }
 + put_page(test_page);
 + }
 + } else {
 + put_page(test_page);
 + rc = 1;
 + goto bail;
 + }
 + }
 +
   /* Add _PAGE_PRESENT to the required access perm */
   access |= _PAGE_PRESENT;


-aneesh

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 3/3] powerpc: mm: optimize for the correctly placed page

2013-12-05 Thread Liu ping fan
On Thu, Dec 5, 2013 at 6:58 PM, Aneesh Kumar K.V
aneesh.ku...@linux.vnet.ibm.com wrote:
 Liu Ping Fan kernelf...@gmail.com writes:

 The period check of _PAGE_NUMA can probably trigger the check on
 the correctly placed page. For this case, we can just insert hpte and
 do fast exception return.

 I still don't understand why we need to handle numa faults in hash
 page ? Are you trying to optimize the code path ? If so can you explain
 the benefits ? Some numbers showing it is helping  ?

When return from hash_page(), we will take fast_exc_return_irq, while
from do_page_fault(), we take ret_from_except.
As the fast implies that there are more complicated logic to sync
the interrupt states in ret_from_except, which cost much.
Do you think so?


 Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
 ---
  arch/powerpc/mm/hash_utils_64.c | 34 +-
  1 file changed, 33 insertions(+), 1 deletion(-)

 diff --git a/arch/powerpc/mm/hash_utils_64.c 
 b/arch/powerpc/mm/hash_utils_64.c
 index 9bf1195..735678c 100644
 --- a/arch/powerpc/mm/hash_utils_64.c
 +++ b/arch/powerpc/mm/hash_utils_64.c
 @@ -965,6 +965,10 @@ int hash_page(unsigned long ea, unsigned long access, 
 unsigned long trap)
   const struct cpumask *tmp;
   int rc, user_region = 0, local = 0;
   int psize, ssize;
 + pte_t old, new;
 + struct vm_area_struct *vma;
 + int page_nid, target_nid;
 + struct page *test_page;

   DBG_LOW(hash_page(ea=%016lx, access=%lx, trap=%lx\n,
   ea, access, trap);
 @@ -1033,12 +1037,40 @@ int hash_page(unsigned long ea, unsigned long 
 access, unsigned long trap)

   /* Get PTE and page size from page tables */
   ptep = find_linux_pte_or_hugepte(pgdir, ea, hugeshift);
 - if (ptep == NULL || !pte_present(*ptep) || pte_numa(*ptep)) {
 + if (ptep == NULL || !pte_present(*ptep)) {
   DBG_LOW( no PTE !\n);
   rc = 1;
   goto bail;
   }

 + old = pte_val(*ptep);
 + if (pte_numa(old)) {
 + /* If fail to lock, let do_page_fault() to handle it */
 + if (down_read_trylock(mm-mmap_sem)) {

 hmm is that something we want to do in hash_page ?

Yes, the function has no relation with hash. But I think it depends on
whether it is worth to optimize or not.

Thanks and regards,
Pingfan
 + vma = find_vma(mm, ea);
 + up_read(mm-mmap_sem);
 + test_page = pte_page(old);
 + page_nid = page_to_nid(test_page);
 + target_nid = numa_migrate_prep(test_page, vma, ea,
 + page_nid);
 + if (target_nid  0) {
 + new = pte_mknonnuma(old);
 + /* If ptep is modified under us,
 +  * just retry the access
 +  */
 + if (unlikely(cmpxchg(ptep, old, new) != old)) {
 + put_page(test_page);
 + return 0;
 + }
 + put_page(test_page);
 + }
 + } else {
 + put_page(test_page);
 + rc = 1;
 + goto bail;
 + }
 + }
 +
   /* Add _PAGE_PRESENT to the required access perm */
   access |= _PAGE_PRESENT;


 -aneesh

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev