[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87fuy168wa.fsf@vitty.brq.redhat.com>
Date: Wed, 13 Jan 2016 12:01:09 +0100
From: Vitaly Kuznetsov <vkuznets@...hat.com>
To: David Rientjes <rientjes@...gle.com>
Cc: linux-mm@...ck.org, Jonathan Corbet <corbet@....net>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Daniel Kiper <daniel.kiper@...cle.com>,
Dan Williams <dan.j.williams@...el.com>,
Tang Chen <tangchen@...fujitsu.com>,
David Vrabel <david.vrabel@...rix.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
Xishi Qiu <qiuxishi@...wei.com>,
Mel Gorman <mgorman@...hsingularity.net>,
"K. Y. Srinivasan" <kys@...rosoft.com>,
Igor Mammedov <imammedo@...hat.com>,
Kay Sievers <kay@...y.org>,
Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
Boris Ostrovsky <boris.ostrovsky@...cle.com>,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
xen-devel@...ts.xenproject.org
Subject: Re: [PATCH v4 1/2] memory-hotplug: add automatic onlining policy for the newly added memory
David Rientjes <rientjes@...gle.com> writes:
> On Tue, 12 Jan 2016, Vitaly Kuznetsov wrote:
>
>> diff --git a/Documentation/memory-hotplug.txt b/Documentation/memory-hotplug.txt
>> index ce2cfcf..ceaf40c 100644
>> --- a/Documentation/memory-hotplug.txt
>> +++ b/Documentation/memory-hotplug.txt
>> @@ -254,12 +254,23 @@ If the memory block is online, you'll read "online".
>> If the memory block is offline, you'll read "offline".
>>
>>
>> -5.2. How to online memory
>> +5.2. Memory onlining
>
> Idk why you're changing this title since you didn't change it in the table
> of contents and it already pairs with "6.2. How to offline memory".
>
> This makes it seem like you're covering all memory onlining operations in
> the kernel (including xen onlining) rather than just memory onlined by
> root. It doesn't cover the fact that xen onlining can be done without
> automatic onlining, so I would leave this section's title as it is and
> only cover aspects of memory onlining that users are triggering
> themselves.
Ok, I changed the title to reflect the fact that a special action to
online memory is not always required any more but as the global policy
stays 'offline' by default for now let's keep the original title.
[skip]
>>
>> + /* online pages if requested */
>> + if (online)
>> + online_pages(start >> PAGE_SHIFT, size >> PAGE_SHIFT,
>> + MMOP_ONLINE_KEEP);
>> +
>> goto out;
>>
>> error:
>
> Well, shucks, what happens if online_pages() fails, such as if a memory
> hot-add notifier returns an errno for MEMORY_GOING_ONLINE? The memory was
> added but not subsequently onlined, although auto onlining was set, so how
> does userspace know the state it is in?
Bad ... we could have checked the return value but I don't see a proper
way to handling it here: if we managed to online some blocks we can't
revert back. We'll probably have to online pages block-by-block (e.g. by
utilizing memory_block_change_state()) handling possible failures.
>
>> @@ -1315,7 +1327,7 @@ int __ref add_memory(int nid, u64 start, u64 size)
>> if (!res)
>> return -EEXIST;
>>
>> - ret = add_memory_resource(nid, res);
>> + ret = add_memory_resource(nid, res, memhp_auto_online);
>> if (ret < 0)
>> release_memory_resource(res);
>> return ret;
--
Vitaly
Powered by blists - more mailing lists