From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752042AbdFLMUj (ORCPT ); Mon, 12 Jun 2017 08:20:39 -0400 Received: from mx2.suse.de ([195.135.220.15]:36027 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751954AbdFLMUi (ORCPT ); Mon, 12 Jun 2017 08:20:38 -0400 Date: Mon, 12 Jun 2017 14:20:35 +0200 From: Michal Hocko To: Vlastimil Babka Cc: linux-mm@kvack.org, Andrew Morton , Naoya Horiguchi , Xishi Qiu , zhong jiang , Joonsoo Kim , LKML Subject: Re: [RFC PATCH 4/4] hugetlb: add support for preferred node to alloc_huge_page_nodemask Message-ID: <20170612122035.GL7476@dhcp22.suse.cz> References: <20170608074553.22152-1-mhocko@kernel.org> <20170608074553.22152-5-mhocko@kernel.org> <20170612090656.GD7476@dhcp22.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon 12-06-17 13:53:51, Vlastimil Babka wrote: > On 06/12/2017 11:06 AM, Michal Hocko wrote: [...] > > -/* Movability of hugepages depends on migration support. */ > > -static inline gfp_t htlb_alloc_mask(struct hstate *h) > > +static struct page *dequeue_huge_page_node(struct hstate *h, int nid) > > { > > - if (hugepages_treat_as_movable || hugepage_migration_supported(h)) > > - return GFP_HIGHUSER_MOVABLE; > > - else > > - return GFP_HIGHUSER; > > + if (nid != NUMA_NO_NODE) > > + return dequeue_huge_page_node_exact(h, nid); > > + > > + return dequeue_huge_page_nodemask(h, nid, NULL); > > This with nid == NUMA_NO_NODE will break at node_zonelist(nid, > gfp_mask); in dequeue_huge_page_nodemask(). I guess just use the local > node as preferred. You are right. Anyway I have a patch to remove this helper altogether. > > -retry_cpuset: > > - cpuset_mems_cookie = read_mems_allowed_begin(); > > - gfp_mask = htlb_alloc_mask(h); > > - nid = huge_node(vma, address, gfp_mask, &mpol, &nodemask); > > - zonelist = node_zonelist(nid, gfp_mask); > > - > > - for_each_zone_zonelist_nodemask(zone, z, zonelist, > > - MAX_NR_ZONES - 1, nodemask) { > > - if (cpuset_zone_allowed(zone, gfp_mask)) { > > - page = dequeue_huge_page_node(h, zone_to_nid(zone)); > > - if (page) { > > - if (avoid_reserve) > > - break; > > - if (!vma_has_reserves(vma, chg)) > > - break; > > - > > - SetPagePrivate(page); > > - h->resv_huge_pages--; > > - break; > > - } > > - } > > + nid = huge_node(vma, address, htlb_alloc_mask(h), &mpol, &nodemask); > > + page = dequeue_huge_page_nodemask(h, nid, nodemask); > > + if (page && !(avoid_reserve || (!vma_has_reserves(vma, chg)))) { > > Ugh that's hard to parse. > What about: if (page && !avoid_reserve && vma_has_reserves(...)) ? Yeah, I have just translated the two breaks into a single condition without scratching my head to much. If you think that this face of De Morgan is nicer I can use it. -- Michal Hocko SUSE Labs