lkml.org 
[lkml]   [2011]   [May]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH V2 2/2] mm: Extend memory hotplug API to allow memory hotplug in virtual machines
From
Date
On Mon, 2011-05-02 at 23:49 +0200, Daniel Kiper wrote:
> +int register_online_page_callback(online_page_callback_t callback)
> +{
> + int rc = -EPERM;
> +
> + lock_memory_hotplug();
> +
> + if (online_page_callback == generic_online_page) {
> + online_page_callback = callback;
> + rc = 0;
> + }
> +
> + unlock_memory_hotplug();
> +
> + return rc;
> +}
> +EXPORT_SYMBOL_GPL(register_online_page_callback);

-EPERM is a bit uninformative here. How about -EEXIST, plus a printk?

I also don't seen the real use behind having a "register" that can only
take a single callback. At worst, it should be
"set_online_page_callback()" so it's more apparent that there can only
be one of these.

> +int unregister_online_page_callback(online_page_callback_t callback)
> +{
> + int rc = -EPERM;
> +
> + lock_memory_hotplug();
> +
> + if (online_page_callback == callback) {
> + online_page_callback = generic_online_page;
> + rc = 0;
> + }
> +
> + unlock_memory_hotplug();
> +
> + return rc;
> +}
> +EXPORT_SYMBOL_GPL(unregister_online_page_callback);

Again, -EPERM is a bad code here. -EEXIST, perhaps? It also deserves a
WARN_ON() or a printk on failure here.

Your changelog doesn't mention, but what ever happened to doing
something dirt-simple like this? I have a short memory.

> void arch_free_hotplug_page(struct page *page)
> {
> if (xen_need_to_inflate_balloon())
> put_page_in_balloon(page);
> else
> __free_page(page);
> }

-- Dave



\
 
 \ /
  Last update: 2011-05-03 18:29    [W:0.100 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site