From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752391AbdFODKn (ORCPT ); Wed, 14 Jun 2017 23:10:43 -0400 Received: from mail-pf0-f195.google.com ([209.85.192.195]:34988 "EHLO mail-pf0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752053AbdFODKl (ORCPT ); Wed, 14 Jun 2017 23:10:41 -0400 Date: Thu, 15 Jun 2017 13:10:30 +1000 From: Balbir Singh To: Jerome Glisse Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, John Hubbard , David Nellans , Johannes Weiner , Michal Hocko , Vladimir Davydov , cgroups@vger.kernel.org Subject: Re: [HMM-CDM 4/5] mm/memcontrol: support MEMORY_DEVICE_PRIVATE and MEMORY_DEVICE_PUBLIC Message-ID: <20170615131030.35fe8d57@firefly.ozlabs.ibm.com> In-Reply-To: <20170615020454.GA4666@redhat.com> References: <20170614201144.9306-1-jglisse@redhat.com> <20170614201144.9306-5-jglisse@redhat.com> <20170615114159.11a1eece@firefly.ozlabs.ibm.com> <20170615020454.GA4666@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 mail.home.local id v5F3BGYY007212 On Wed, 14 Jun 2017 22:04:55 -0400 Jerome Glisse wrote: > On Thu, Jun 15, 2017 at 11:41:59AM +1000, Balbir Singh wrote: > > On Wed, 14 Jun 2017 16:11:43 -0400 > > Jérôme Glisse wrote: > > > > > HMM pages (private or public device pages) are ZONE_DEVICE page and > > > thus need special handling when it comes to lru or refcount. This > > > patch make sure that memcontrol properly handle those when it face > > > them. Those pages are use like regular pages in a process address > > > space either as anonymous page or as file back page. So from memcg > > > point of view we want to handle them like regular page for now at > > > least. > > > > > > Signed-off-by: Jérôme Glisse > > > Cc: Johannes Weiner > > > Cc: Michal Hocko > > > Cc: Vladimir Davydov > > > Cc: cgroups@vger.kernel.org > > > --- > > > kernel/memremap.c | 2 ++ > > > mm/memcontrol.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++----- > > > 2 files changed, 55 insertions(+), 5 deletions(-) > > > > > > diff --git a/kernel/memremap.c b/kernel/memremap.c > > > index da74775..584984c 100644 > > > --- a/kernel/memremap.c > > > +++ b/kernel/memremap.c > > > @@ -479,6 +479,8 @@ void put_zone_device_private_or_public_page(struct page *page) > > > __ClearPageActive(page); > > > __ClearPageWaiters(page); > > > > > > + mem_cgroup_uncharge(page); > > > + > > > > A zone device page could have a mem_cgroup charge if > > > > 1. The old page was charged to a cgroup and the new page from ZONE_DEVICE then > > gets the charge that we need to drop here > > > > And should not be charged > > > > 2. If the driver allowed mmap based allocation (these pages are not on LRU > > > > > > Since put_zone_device_private_or_public_page() is called from release_pages(), > > I think the assumption is that 2 is not a problem? I've not tested the mmap > > bits yet. > > Well that is one of the big question. Do we care about memory cgroup despite > page not being on lru and thus not being reclaimable through the usual path ? > > I believe we do want to keep charging ZONE_DEVICE page against memory cgroup > so that userspace limit are enforced. This is important especialy for device > private when migrating back to system memory due to CPU page fault. We do not > want the migration back to fail because of memory cgroup limit. > > Hence why i do want to charge ZONE_DEVICE page just like regular page. If we > have people that run into OOM because of this then we can start thinking about > how to account those pages slightly differently inside the memory cgroup. > > For now i believe we do want this patch. > Yes, we do need the patch, I was trying to check if we'll end up trying to uncharge a page that is not charged, just double checking > > [...] > > > > @@ -4610,6 +4637,9 @@ static enum mc_target_type get_mctgt_type(struct vm_area_struct *vma, > > > */ > > > if (page->mem_cgroup == mc.from) { > > > ret = MC_TARGET_PAGE; > > > + if (is_device_private_page(page) || > > > + is_device_public_page(page)) > > > + ret = MC_TARGET_DEVICE; > > > if (target) > > > target->page = page; > > > } > > > @@ -4669,6 +4699,11 @@ static int mem_cgroup_count_precharge_pte_range(pmd_t *pmd, > > > > > > ptl = pmd_trans_huge_lock(pmd, vma); > > > if (ptl) { > > > + /* > > > + * Note their can not be MC_TARGET_DEVICE for now as we do not > > there > > > + * support transparent huge page with MEMORY_DEVICE_PUBLIC or > > > + * MEMORY_DEVICE_PRIVATE but this might change. > > > > I am trying to remind myself why THP and MEMORY_DEVICE_* pages don't work well > > together today, the driver could allocate a THP size set of pages and migrate it. > > There are patches to do THP migration, not upstream yet. Could you remind me > > of any other limitations? > > No there is nothing that would be problematic AFAICT. Persistent memory already > use huge page so we should be in the clear. But i would rather enable that as > a separate patchset alltogether and have proper testing specificaly for such > scenario. Agreed Balbir Singh.