From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932963AbdGLFu3 (ORCPT ); Wed, 12 Jul 2017 01:50:29 -0400 Received: from mail-pg0-f66.google.com ([74.125.83.66]:36137 "EHLO mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756693AbdGLFu0 (ORCPT ); Wed, 12 Jul 2017 01:50:26 -0400 Date: Wed, 12 Jul 2017 15:50:15 +1000 From: Balbir Singh To: Jerome Glisse Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, John Hubbard , David Nellans , Dan Williams , Balbir Singh , Aneesh Kumar , "Paul E . McKenney" , Benjamin Herrenschmidt , Ross Zwisler Subject: Re: [PATCH 2/5] mm/device-public-memory: device memory cache coherent with CPU v2 Message-ID: <20170712155015.2b77f958@firefly.ozlabs.ibm.com> In-Reply-To: <20170711145744.GA5347@redhat.com> References: <20170703211415.11283-1-jglisse@redhat.com> <20170703211415.11283-3-jglisse@redhat.com> <20170711141215.4fd1a972@firefly.ozlabs.ibm.com> <20170711145744.GA5347@redhat.com> X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v6C5p6YJ006229 On Tue, 11 Jul 2017 10:57:44 -0400 Jerome Glisse wrote: > On Tue, Jul 11, 2017 at 02:12:15PM +1000, Balbir Singh wrote: > > On Mon, 3 Jul 2017 17:14:12 -0400 > > Jérôme Glisse wrote: > > > > > Platform with advance system bus (like CAPI or CCIX) allow device > > > memory to be accessible from CPU in a cache coherent fashion. Add > > > a new type of ZONE_DEVICE to represent such memory. The use case > > > are the same as for the un-addressable device memory but without > > > all the corners cases. > > > > > > > Looks good overall, some comments inline. > > > > [...] > > > > /* > > > @@ -92,6 +100,8 @@ enum memory_type { > > > * The page_free() callback is called once the page refcount reaches 1 > > > * (ZONE_DEVICE pages never reach 0 refcount unless there is a refcount bug. > > > * This allows the device driver to implement its own memory management.) > > > + * > > > + * For MEMORY_DEVICE_CACHE_COHERENT only the page_free() callback matter. > > > > Correct, but I wonder if we should in the long term allow for minor faults > > (due to coherency) via this interface? > > This is something we can explore latter on. > > [...] > Agreed > > > diff --git a/kernel/memremap.c b/kernel/memremap.c > > > index e82456c39a6a..da74775f2247 100644 > > > --- a/kernel/memremap.c > > > +++ b/kernel/memremap.c > > > @@ -466,7 +466,7 @@ struct vmem_altmap *to_vmem_altmap(unsigned long memmap_start) > > > > > > > > > #ifdef CONFIG_DEVICE_PRIVATE > > > > Does the #ifdef above need to go as well? > > Good catch i should make that conditional on DEVICE_PUBLIC or whatever > the name endup to be. I will make sure i test without DEVICE_PRIVATE > config before posting again. > > [...] > I've been testing with this off, I should have sent you a patch, but I thought I'd also update in the review. > > > @@ -2541,11 +2551,21 @@ static void migrate_vma_insert_page(struct migrate_vma *migrate, > > > */ > > > __SetPageUptodate(page); > > > > > > - if (is_zone_device_page(page) && is_device_private_page(page)) { > > > - swp_entry_t swp_entry; > > > + if (is_zone_device_page(page)) { > > > + if (is_device_private_page(page)) { > > > + swp_entry_t swp_entry; > > > > > > - swp_entry = make_device_private_entry(page, vma->vm_flags & VM_WRITE); > > > - entry = swp_entry_to_pte(swp_entry); > > > + swp_entry = make_device_private_entry(page, vma->vm_flags & VM_WRITE); > > > + entry = swp_entry_to_pte(swp_entry); > > > + } > > > +#if IS_ENABLED(CONFIG_DEVICE_PUBLIC) > > > > Do we need this #if check? is_device_public_page(page) > > will return false if the config is disabled > > pte_mkdevmap() is not define if ZONE_DEVICE is not enabled hence > i had to protect this with #if/#endif to avoid build error. pte_mkdevmap is always defined, could you please share the build error. Balbir Singh.