From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1764481AbdAKQ4M (ORCPT ); Wed, 11 Jan 2017 11:56:12 -0500 Received: from mga02.intel.com ([134.134.136.20]:16419 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751292AbdAKQ4K (ORCPT ); Wed, 11 Jan 2017 11:56:10 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,346,1477983600"; d="scan'208";a="1081825067" Subject: Re: [PATCH v4 2/4] mm: Add function to support extra actions on swap in/out To: Khalid Aziz , akpm@linux-foundation.org, davem@davemloft.net, arnd@arndb.de References: Cc: kirill.shutemov@linux.intel.com, mhocko@suse.com, jmarchan@redhat.com, vbabka@suse.cz, dan.j.williams@intel.com, lstoakes@gmail.com, hannes@cmpxchg.org, mgorman@suse.de, hughd@google.com, vdavydov.dev@gmail.com, minchan@kernel.org, namit@vmware.com, linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, sparclinux@vger.kernel.org, Khalid Aziz From: Dave Hansen Message-ID: Date: Wed, 11 Jan 2017 08:56:06 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. 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. > @@ -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.