Subject: Re: [HMM v13 06/18] mm/ZONE_DEVICE/unaddressable: add special swap
 for unaddressable
To: Jerome Glisse <jglisse@redhat.com>
References: <1479493107-982-1-git-send-email-jglisse@redhat.com>
 <1479493107-982-7-git-send-email-jglisse@redhat.com>
 <5832D33C.6030403@linux.vnet.ibm.com> <20161121124218.GF2392@redhat.com>
Cc: akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org, John Hubbard <jhubbard@nvidia.com>,
        Dan Williams <dan.j.williams@intel.com>,
        Ross Zwisler <ross.zwisler@linux.intel.com>
From: Anshuman Khandual <khandual@linux.vnet.ibm.com>
Date: Tue, 22 Nov 2016 10:18:27 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.1.0
MIME-Version: 1.0
In-Reply-To: <20161121124218.GF2392@redhat.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 8bit
Message-Id: <5833CE1B.6030104@linux.vnet.ibm.com>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2066
Lines: 50

On 11/21/2016 06:12 PM, Jerome Glisse wrote:
> On Mon, Nov 21, 2016 at 04:28:04PM +0530, Anshuman Khandual wrote:
>> On 11/18/2016 11:48 PM, J?r?me Glisse wrote:
>>> To allow use of device un-addressable memory inside a process add a
>>> special swap type. Also add a new callback to handle page fault on
>>> such entry.
>>
>> IIUC this swap type is required only for the mirror cases and its
>> not a requirement for migration. If it's required for mirroring
>> purpose where we intercept each page fault, the commit message
>> here should clearly elaborate on that more.
> 
> It is only require for un-addressable memory. The mirroring has nothing to do
> with it. I will clarify commit message.

One thing though. I dont recall how persistent memory ZONE_DEVICE
pages are handled inside the page tables, point here is it should
be part of the same code block. We should catch that its a device
memory page and then figure out addressable or not and act
accordingly. Because persistent memory are CPU addressable, there
might not been special code block but dealing with device pages 
should be handled in a more holistic manner.

> 
> [...]
> 
>>> diff --git a/include/linux/memremap.h b/include/linux/memremap.h
>>> index b6f03e9..d584c74 100644
>>> --- a/include/linux/memremap.h
>>> +++ b/include/linux/memremap.h
>>> @@ -47,6 +47,11 @@ static inline struct vmem_altmap *to_vmem_altmap(unsigned long memmap_start)
>>>   */
>>>  struct dev_pagemap {
>>>  	void (*free_devpage)(struct page *page, void *data);
>>> +	int (*fault)(struct vm_area_struct *vma,
>>> +		     unsigned long addr,
>>> +		     struct page *page,
>>> +		     unsigned flags,
>>> +		     pmd_t *pmdp);
>>
>> We are extending the dev_pagemap once again to accommodate device driver
>> specific fault routines for these pages. Wondering if this extension and
>> the new swap type should be in the same patch.
> 
> It make sense to have it in one single patch as i also change page fault code
> path to deal with the new special swap entry and those make use of this new
> callback.
> 

Okay.