Re: + mm-introduce-reported-pages.patch added to -mm tree

[Nitesh Narayan Lal <nitesh@redhat.com>]

On 11/12/19 11:18 AM, Alexander Duyck wrote:
> On Tue, 2019-11-12 at 10:19 -0500, Nitesh Narayan Lal wrote:
>> On 11/11/19 5:00 PM, Alexander Duyck wrote:
>>> On Mon, Nov 11, 2019 at 10:52 AM Nitesh Narayan Lal <nitesh@redhat.com> wrote:
>>>> On 11/6/19 7:16 AM, Michal Hocko wrote:
>>>>> I didn't have time to read through newer versions of this patch series
>>>>> but I remember there were concerns about this functionality being pulled
>>>>> into the page allocator previously both by me and Mel [1][2]. Have those been
>>>>> addressed? I do not see an ack from Mel or any other MM people. Is there
>>>>> really a consensus that we want something like that living in the
>>>>> allocator?
>>>>>
>>>>> There has also been a different approach discussed and from [3]
>>>>> (referenced by the cover letter) I can only see
>>>>>
>>>>> : Then Nitesh's solution had changed to the bitmap approach[7]. However it
>>>>> : has been pointed out that this solution doesn't deal with sparse memory,
>>>>> : hotplug, and various other issues.
>>>>>
>>>>> which looks more like something to be done than a fundamental
>>>>> roadblocks.
>>>>>
>>>>> [1] http://lkml.kernel.org/r/20190912163525.GV2739@techsingularity.net
>>>>> [2] http://lkml.kernel.org/r/20190912091925.GM4023@dhcp22.suse.cz
>>>>> [3] http://lkml.kernel.org/r/29f43d5796feed0dec8e8bb98b187d9dac03b900.camel@linux.intel.com
>>>>>
>>>> [...]
>>>>
>>>> Hi,
>>>>
>>>> I performed some experiments to find the root cause for the performance
>>>> degradation Alexander reported with my v12 patch-set. [1]
>>>>
>>>> I will try to give a brief background of the previous discussion
>>>> under v12: (Alexander can correct me if I am missing something).
>>>> Alexander suggested two issues with my v12 posting: [2]
>>>> (This is excluding the sparse zone and memory hotplug/hotremove support)
>>>>
>>>> - A crash which was caused because I was not using spinlock_irqsave()
>>>>   (Fix suggestion came from Alexander).
>>>>
>>>> - Performance degradation with Alexander's suggested setup. Where we are using
>>>>   modified will-it-scale/page_fault with THP, CONFIG_SLAB_FREELIST_RANDOM &
>>>>   CONFIG_SHUFFLE_PAGE_ALLOCATOR. When I was using (MAX_ORDER - 2) as the
>>>>   PAGE_REPORTING_MIN_ORDER, I also observed significant performance degradation
>>>>   (around 20% in the number of threads launched on the 16th vCPU). However, on
>>>>   switching the PAGE_REPORTING_MIN_ORDER to (MAX_ORDER - 1), I was able to get
>>>>   the performance similar to what Alexander is reporting.
>>>>
>>>> PAGE_REPORTING_MIN_ORDER: is the minimum order of a page to be captured in the
>>>> bitmap and get reported to the hypervisor.
>>>>
>>>> For the discussion where we are comparing the two series, the performance
>>>> aspect is more relevant and important.
>>>> It turns out that with the current implementation the number of vmexit with
>>>> PAGE_REPORTING_MIN_ORDER as pageblock_order or (MAX_ORDER - 2) are significantly
>>>> large when compared to (MAX_ODER - 1).
>>>>
>>>> One of the reason could be that the lower order pages are not getting sufficient
>>>> time to merge with each other as a result they are somehow getting reported
>>>> with 2 separate reporting requests. Hence, generating more vmexits. Where
>>>> as with (MAX_ORDER - 1) we don't have that kind of situation as I never try
>>>> to report any page which has order < (MAX_ORDER - 1).
>>>>
>>>> To fix this, I might have to further limit the reporting which could allow the
>>>> lower order pages to further merge and hence reduce the VM exits. I will try to
>>>> do some experiments to see if I can fix this. In any case, if anyone has a
>>>> suggestion I would be more than happy to look in that direction.
>>> That doesn't make any sense. My setup using MAX_ORDER - 2, aka
>>> pageblock_order, as the limit doesn't experience the same performance
>>> issues the bitmap solution does. That leads me to believe the issue
>>> isn't that the pages have not had a chance to be merged.
>>>
>> So, I did run your series as well with a few syfs variables to see how many
>> pages of order (MAX_ORDER - 1) or (MAX_ORDER - 2) are reported at the end of
>> will-it-scale/page_fault4 test.
>> What I observed is the number of (MAX_ORDER - 2) pages which were getting
>> reported in your case were lesser than what has been reported in mine with
>> pageblock_order.
>> As you have mentioned below about putting pages in a certain part of the
>> free list might have also an impact.
> Another thing you may want to check is how often your notifier is
> triggering. One thing I did was to intentionally put a fairly significant
> delay from the time the notification is scheduled to when it will start. I
> did this because when an application is freeing memory it will take some
> time to completely free it, and if it is going to reallocate it anyway
> there is no need to rush since it would just invalidate the pages you
> reported anyway.

Yes, I agree with this. This could have an impact on the performance.

>
>>>> Following are the numbers I gathered on a 30GB single NUMA, 16 vCPU guest
>>>> affined to a single host-NUMA:
>>>>
>>>> On 16th vCPU:
>>>> With PAGE_REPORTING_MIN_ORDER as (MAX_ORDER - 1):
>>>> % Dip on the number of Processes = 1.3 %
>>>> % Dip on the number of  Threads  = 5.7 %
>>>>
>>>> With PAGE_REPORTING_MIN_ORDER as With (pageblock_order):
>>>> % Dip on the number of Processes = 5 %
>>>> % Dip on the number of  Threads  = 20 %
>>> So I don't hold much faith in the threads numbers. I have seen the
>>> variability be as high as 14% between runs.
>> That's interesting. Do you see the variability even with an unmodified kernel?
>> Somehow, for me it seems pretty consistent. However, if you are running with
>> multiple NUMA nodes it might have a significant impact on the numbers.
>>
>> For now, I am only running a single NUMA guest affined to a single NUMA
>> of host.
> My guest should be running in a single node, and yes I saw it with just
> the unmodified kernel. I am running on the linux-next 20191031 kernel.

I am using Linus linux tree and working on top of Linux 5.4-rc5.
Not sure how much difference will that make.

>  It
> did occur to me that it seems like the performance for the threads number
> recently increased. There might be a guest config option impacting things
> as well since I know I have changed a number of variables since then.

This is quite interesting because if I remember correctly then you reported a
huge degradation of over 30% with my patch-set.
So far, I was able to reproduce significant degradation with the number of
threads launched on the 16th vcpu but not in the number of processes which you
are observing.
I am wondering if I am still missing something in my test-setup.

>
>>>> Michal's suggestion:
>>>> I was able to get the prototype which could use page-isolation API:
>>>> start_isolate_page_range()/undo_isolate_page_range() to work.
>>>> But the issue mentioned above was also evident with it.
>>>>
>>>> Hence, I think before moving to the decision whether I want to use
>>>> __isolate_free_page() which isolates pages from the buddy or
>>>> start/undo_isolate_page_range() which just marks the page as MIGRATE_ISOLATE,
>>>> it is important for me to resolve the above-mentioned issue.
>>> I'd be curious how you are avoiding causing memory starvation if you
>>> are isolating ranges of memory that have been recently freed.
>> I would still be marking only 32 pages as MIGRATE_ISOLATE at a time. It is
>> exactly same as that of isolating limited chunk of pages from the buddy.
>> For example if I have a pfn:x of order y then I pass
>> start_isolate_page_range(x, x+y, mt, 0). So at the end we
>> will have 32 such entries marked as MIGRATE_ISOLATE.
> I get that you are isolating the same amount of memory. What I was getting
> at is that __isolate_free_page has a check in it to make certain you are
> not pulling memory that would put you below the minimum watermark. As far
> as I know there isn't anything like that for the page isolation framework
> since it is normally used for offlining memory before it is hotplugged
> away.

Yes, that is correct. I will have to take care of that explicitly.

>
>>>> Previous discussions:
>>>> More about how we ended up with these two approaches could be found at [3] &
>>>> [4] explained by Alexander & David.
>>>>
>>>> [1] https://lore.kernel.org/lkml/20190812131235.27244-1-nitesh@redhat.com/
>>>> [2] https://lkml.org/lkml/2019/10/2/425
>>>> [3] https://lkml.org/lkml/2019/10/23/1166
>>>> [4] https://lkml.org/lkml/2019/9/12/48
>>>>
>>> So one thing you may want to consider would be how placement of the
>>> buffers will impact your performance.
>>>
>>> One thing I realized I was doing wrong with my approach was scanning
>>> for pages starting at the tail and then working up. It greatly hurt
>>> the efficiency of my search since in the standard case most of the
>>> free memory will be placed at the head and only with shuffling enabled
>>> do I really need to worry about things getting mixed up with the tail.
>>>
>>> I suspect you may be similarly making things more difficult for
>>> yourself by placing the reported pages back on the head of the list
>>> instead of placing them at the tail where they will not be reallocated
>>> immediately.
>> hmm, I see. I will try and explore this.
>>
>
-- 
Thanks
Nitesh