lkml.org 
[lkml]   [2019]   [Jan]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v7] mm/page_alloc.c: memory_hotplug: free pages as higher order
On 2019-01-09 21:39, Alexander Duyck wrote:
> On Wed, 2019-01-09 at 11:51 +0530, Arun KS wrote:
>> On 2019-01-09 03:47, Alexander Duyck wrote:
>> > On Fri, 2019-01-04 at 10:31 +0530, Arun KS wrote:
>> > > When freeing pages are done with higher order, time spent on
>> > > coalescing
>> > > pages by buddy allocator can be reduced. With section size of 256MB,
>> > > hot
>> > > add latency of a single section shows improvement from 50-60 ms to
>> > > less
>> > > than 1 ms, hence improving the hot add latency by 60 times. Modify
>> > > external providers of online callback to align with the change.
>> > >
>> > > Signed-off-by: Arun KS <arunks@codeaurora.org>
>> > > Acked-by: Michal Hocko <mhocko@suse.com>
>> > > Reviewed-by: Oscar Salvador <osalvador@suse.de>
>> >
>> > Sorry, ended up encountering a couple more things that have me a bit
>> > confused.
>> >
>> > [...]
>> >
>> > > diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
>> > > index 5301fef..211f3fe 100644
>> > > --- a/drivers/hv/hv_balloon.c
>> > > +++ b/drivers/hv/hv_balloon.c
>> > > @@ -771,7 +771,7 @@ static void hv_mem_hot_add(unsigned long start,
>> > > unsigned long size,
>> > > }
>> > > }
>> > >
>> > > -static void hv_online_page(struct page *pg)
>> > > +static int hv_online_page(struct page *pg, unsigned int order)
>> > > {
>> > > struct hv_hotadd_state *has;
>> > > unsigned long flags;
>> > > @@ -783,10 +783,12 @@ static void hv_online_page(struct page *pg)
>> > > if ((pfn < has->start_pfn) || (pfn >= has->end_pfn))
>> > > continue;
>> > >
>> > > - hv_page_online_one(has, pg);
>> > > + hv_bring_pgs_online(has, pfn, (1UL << order));
>> > > break;
>> > > }
>> > > spin_unlock_irqrestore(&dm_device.ha_lock, flags);
>> > > +
>> > > + return 0;
>> > > }
>> > >
>> > > static int pfn_covered(unsigned long start_pfn, unsigned long
>> > > pfn_cnt)
>> >
>> > So the question I have is why was a return value added to these
>> > functions? They were previously void types and now they are int. What
>> > is the return value expected other than 0?
>>
>> Earlier with returning a void there was now way for an arch code to
>> denying onlining of this particular page. By using an int as return
>> type, we can implement this. In one of the boards I was using, there
>> are
>> some pages which should not be onlined because they are used for other
>> purposes(like secure trust zone or hypervisor).
>
> So where is the code using that? I don't see any functions in the
> kernel that are returning anything other than 0. Maybe you should hold
> off on changing the return type and make that a separate patch to be
> enabled when you add the new functions that can return non-zero values.
>
> That way if someone wants to backport this they are just getting the
> bits needed to enable the improved hot-plug times without adding the
> extra overhead for changing the return type.

The implementation was in our downstream code. I thought this might be
useful for someone else in similar situations.
Considering the above mentioned reasons, I ll remove changing the return
type.

Regards,
Arun

\
 
 \ /
  Last update: 2019-01-10 05:40    [W:0.297 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site