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 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxxx For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>