Message-ID: <8951b82b-d95b-5e22-9846-c78a330f57b7@redhat.com>
Date:   Tue, 7 Dec 2021 16:40:57 -0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.2.0
Subject: Re: [RFC PATCH 2/2] mm/vmscan.c: Prevent allocating shrinker_info on
 offlined nodes
Content-Language: en-US
To:     David Hildenbrand <david@redhat.com>,
        Michal Hocko <mhocko@suse.com>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        akpm@linux-foundation.org, shakeelb@google.com,
        ktkhai@virtuozzo.com, shy828301@gmail.com, guro@fb.com,
        vbabka@suse.cz, vdavydov.dev@gmail.com, raquini@redhat.com
References: <20211206033338.743270-1-npache@redhat.com>
 <20211206033338.743270-3-npache@redhat.com> <Ya3WcYKcej8XEI0W@dhcp22.suse.cz>
 <d9d14beb-ee20-7ebb-e007-fbf58fb28535@redhat.com>
From:   Nico Pache <npache@redhat.com>
In-Reply-To: <d9d14beb-ee20-7ebb-e007-fbf58fb28535@redhat.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 12/6/21 05:45, David Hildenbrand wrote:
>> This doesn't seen complete. Slab shrinkers are used in the reclaim
>> context. Previously offline nodes could be onlined later and this would
>> lead to NULL ptr because there is no hook to allocate new shrinker
>> infos. This would be also really impractical because this would have to
>> update all existing memcgs...
> 
> Instead of going through the trouble of updating...
> 
> ...  maybe just keep for_each_node() and check if the target node is
> offline. If it's offline, just allocate from the first online node.
> After all, we're not using __GFP_THISNODE, so there are no guarantees
> either way ...

Thanks for your reviews David :)

Yes, I believe this would be a viable solution. At this point it looks like our
short term solution would be something along this line. Long term we would want
to either allocate all pgdats (which will waste memory) or update the memcgs
with the appropriate node information once a node is onlined. Im currently
working a solution that does the latter.