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: [v5] mm: khugepaged: recalculate min_free_kbytes after memory
 hotplug as expected by khugepaged
To:     Michal Hocko <mhocko@suse.com>
Cc:     Vijay Balakrishna <vijayb@linux.microsoft.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
        Oleg Nesterov <oleg@redhat.com>,
        Song Liu <songliubraving@fb.com>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Pavel Tatashin <pasha.tatashin@soleen.com>,
        Allen Pais <apais@microsoft.com>, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org
References: <1601398153-5517-1-git-send-email-vijayb@linux.microsoft.com>
 <2a380b84-4fee-fa4e-e862-8a8577961088@oracle.com>
 <8cdb105c-2b7b-1997-ed82-22f4bb25638c@linux.microsoft.com>
 <638ebb7a-72e3-a219-ee2b-55f1c028efad@oracle.com>
 <20201002112516.GD4555@dhcp22.suse.cz>
From:   Mike Kravetz <mike.kravetz@oracle.com>
Message-ID: <9bc3d446-ea25-6abf-bd9d-0c24009c8a19@oracle.com>
Date:   Fri, 2 Oct 2020 11:31:10 -0700
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: <20201002112516.GD4555@dhcp22.suse.cz>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 10/2/20 4:25 AM, Michal Hocko wrote:
> On Wed 30-09-20 15:03:11, Mike Kravetz wrote:
>> On 9/30/20 1:47 PM, Vijay Balakrishna wrote:
>>> On 9/30/2020 11:20 AM, Mike Kravetz wrote:
>>>> On 9/29/20 9:49 AM, Vijay Balakrishna wrote:
>>>>
>>>> Sorry for jumping in so late.  Should we use this as an opportunity to
>>>> also fix up the messages logged when (re)calculating mfk?  They are wrong
>>>> and could be quite confusing.
>>>
>>>
>>> Sure.  Please share your thoughts regarding appropriate message.  Here is what I'm thinking
>>>
>>> pr_warn("min_free_kbytes is not updated to %d because current value %d is preferred\n", new_min_free_kbytes, min_free_kbytes);
>>>
>>> If above message is reasonable I can post a new revision (v6).
>>
>> Just considering the below example,
>>
>>>> For example consider the following sequence
>>>> of operations and corresponding log messages produced.
>>>>
>>>> Freshly booted VM with 2 nodes and 8GB memory:
>>>> # cat /proc/sys/vm/min_free_kbytes
>>>> 90112
>>>> # echo 90000 > /proc/sys/vm/min_free_kbytes
>>>> # cat /proc/sys/vm/min_free_kbytes
>>>> 90000
>>>> # echo 0 > /sys/devices/system/node/node1/memory56/online
>>>> [  135.099947] Offlined Pages 32768
>>>> [  135.102362] min_free_kbytes is not updated to 11241 because user defined value 90000 is preferred
>>
>> I am not sure if there is any value in printing the above line.  Especially
>> in this context as it becomes obsolete with the printing of the next line.
> 
> The original intention was to make it explicit that auto-tuning is
> influenced by the user provided configuration.
> 
>>>> [  135.109070] khugepaged: raising min_free_kbytes from 90000 to 90112 to help t
>>>> ransparent hugepage allocations
>>
>> IMO, the above line is the only one that should be output as a result of the
>> recalculation.
> 
> Well, but khugepaged could be disabled and then the above might not get
> printed. Sure the code could get reorganized and all that but is this
> really worth that?
> 
>> I guess that brings up the question of 'should we continue to track the user
>> defined value if we overwrite it?".  If we quit tracking it may help with the
>> next message.
> 
> Auto tuning and user provided override is quite tricky to get sensible.
> Especially in the case here. Admin has provided an override but has the
> potential memory hotplug been considered? Or to make it even more
> complicated, consider that the hotplug happens without admin involvement
> - e.g. memory gets hotremoved due to HW problems. Is the admin provided
> value still meaningful? To be honest I do not have a good answer and I
> am not sure we should care all that much until we see practical
> problems.

I am not insisting that this be cleaned up.  The change in this patch to
ensure THP related calculations are performed during hotplug is the most
important.

I became aware of the logging issues when looking at a customer issue with
an older kernel.  The min_free_kbytes setting was integral to the issue we
were investigating, and it was unclear whether or not the customer had
changed the value.  I knew the system log should contain evidence of manually
setting min_free_kbytes.  However, there was no evidence in the log.  Turns
out the customer did not change the value, but it did cause me to do a deep
dive into the logging code.
-- 
Mike Kravetz