Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755904AbbB0WTs (ORCPT ); Fri, 27 Feb 2015 17:19:48 -0500 Received: from cantor2.suse.de ([195.135.220.15]:58302 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754956AbbB0WTq (ORCPT ); Fri, 27 Feb 2015 17:19:46 -0500 Message-ID: <54F0ED7E.6010900@suse.cz> Date: Fri, 27 Feb 2015 23:19:42 +0100 From: Vlastimil Babka User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: David Rientjes CC: Andrew Morton , Christoph Lameter , Pekka Enberg , Joonsoo Kim , Johannes Weiner , Mel Gorman , Pravin Shelar , Jarno Rajahalme , Greg Thelen , linux-kernel@vger.kernel.org, linux-mm@kvack.org, netdev@vger.kernel.org, dev@openvswitch.org Subject: Re: [patch 1/2] mm: remove GFP_THISNODE References: <54EED9A7.5010505@suse.cz> <54F01E02.1090007@suse.cz> In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2800 Lines: 57 On 02/27/2015 11:03 PM, David Rientjes wrote: >> With both >> patches they won't bail out and __GFP_NO_KSWAPD will prevent most of the stuff >> described above, including clearing ALLOC_CPUSET. > > Yeah, ALLOC_CPUSET is never cleared for thp allocations because atomic == > false for thp, regardless of this series. > >> But __cpuset_node_allowed() >> will allow it to allocate anywhere anyway thanks to the newly passed >> __GFP_THISNODE, which would be a regression of what b104a35d32 fixed... unless >> I'm missing something else that prevents it, which wouldn't surprise me at all. >> >> There's this outdated comment: >> >> * The __GFP_THISNODE placement logic is really handled elsewhere, >> * by forcibly using a zonelist starting at a specified node, and by >> * (in get_page_from_freelist()) refusing to consider the zones for >> * any node on the zonelist except the first. By the time any such >> * calls get to this routine, we should just shut up and say 'yes'. >> >> AFAIK the __GFP_THISNODE zonelist contains *only* zones from the single node and >> there's no other "refusing". > > Yes, __cpuset_node_allowed() is never called for a zone from any other > node when __GFP_THISNODE is passed because of node_zonelist(). It's > pointless to iterate over those zones since the allocation wants to fail > instead of allocate on them. > > Do you see any issues with either patch 1/2 or patch 2/2 besides the > s/GFP_TRANSHUGE/GFP_THISNODE/ that is necessary on the changelog? Well, my point is, what if the node we are explicitly trying to allocate hugepage on, is in fact not allowed by our cpuset? This could happen in the page fault case, no? Although in a weird configuration when process can (and really gets scheduled to run) on a node where it is not allowed to allocate from... >> And I don't really see why __GFP_THISNODE should >> have this exception, it feels to me like "well we shouldn't reach this but we >> are not sure, so let's play it safe". So maybe we could just remove this >> exception? I don't think any other user of __GFP_THISNODE | __GFP_WAIT user >> relies on this allowed memset violation? >> > > Since this function was written, there were other callers to > cpuset_{node,zone}_allowed_{soft,hard}wall() that may have required it. I > looked at all the current callers of cpuset_zone_allowed() and they don't > appear to need this "exception" (slub calls node_zonelist() itself for the > iteration and slab never calls it for __GFP_THISNODE). So, yeah, I think > it can be removed. > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/