Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] mm/memory_hotplug: drain per-cpu pages again during
 memory offline
To:     Pavel Tatashin <pasha.tatashin@soleen.com>,
        Michal Hocko <mhocko@suse.com>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        linux-mm <linux-mm@kvack.org>, Mel Gorman <mgorman@suse.de>
References: <20200901124615.137200-1-pasha.tatashin@soleen.com>
 <20200902140116.GI4617@dhcp22.suse.cz> <20200902141057.GK4617@dhcp22.suse.cz>
 <CA+CK2bChfLLDikSzaD0kCkchxiRd0fzpyDEEpbzEh_7MEqhNgw@mail.gmail.com>
From:   Vlastimil Babka <vbabka@suse.cz>
Message-ID: <e20d9fec-c417-7415-72fb-ee44bf83ed67@suse.cz>
Date:   Wed, 2 Sep 2020 16:49:19 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.11.0
MIME-Version: 1.0
In-Reply-To: <CA+CK2bChfLLDikSzaD0kCkchxiRd0fzpyDEEpbzEh_7MEqhNgw@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 9/2/20 4:31 PM, Pavel Tatashin wrote:
>> > > The fix is to try to drain per-cpu lists again after
>> > > check_pages_isolated_cb() fails.
>>
>> Still trying to wrap my head around this but I think this is not a
>> proper fix. It should be the page isolation to make sure no races are
>> possible with the page freeing path.
>>
> 
> As Bharata B Rao found in another thread, the problem was introduced
> by this change:
> c52e75935f8d: mm: remove extra drain pages on pcp list
> 
> So, the drain used to be tried every time with lru_add_drain_all();
> Which, I think is excessive, as we start a thread per cpu to try to
> drain and catch a rare race condition. With the proposed change we
> drain again only when we find such a condition. Fixing it in
> start_isolate_page_range means that we must somehow synchronize it
> with the release_pages() which adds costs to runtime code, instead of
> to hot-remove code.

Agreed. Isolation was always racy wrt freeing to pcplists, and it was simply
acceptable to do some extra drains if needed. Removing that race would be indeed
acceptable only if it didn't affect alloc/free fastpaths.

> Pasha
>