From: David Rientjes <rientjes@google.com>
To: Daniel Kiper <dkiper@net-space.pl>
Cc: ian.campbell@citrix.com, akpm@linux-foundation.org,
andi.kleen@intel.com, haicheng.li@linux.intel.com,
fengguang.wu@intel.com, jeremy@goop.org, konrad.wilk@oracle.com,
dan.magenheimer@oracle.com, v.tolstov@selfip.ru, pasik@iki.fi,
dave@linux.vnet.ibm.com, wdauchy@gmail.com,
xen-devel@lists.xensource.com, linux-kernel@vger.kernel.org,
linux-mm@kvack.org
Subject: Re: [PATCH V3 2/2] mm: Extend memory hotplug API to allow memory hotplug in virtual machines
Date: Wed, 18 May 2011 20:36:02 -0700 (PDT) [thread overview]
Message-ID: <alpine.DEB.2.00.1105182026390.20651@chino.kir.corp.google.com> (raw)
In-Reply-To: <20110517213858.GC30232@router-fw-old.local.net-space.pl>
On Tue, 17 May 2011, Daniel Kiper wrote:
> This patch contains online_page_callback and apropriate functions for
> setting/restoring online page callbacks. It allows to do some machine
> specific tasks during online page stage which is required to implement
> memory hotplug in virtual machines. Additionally, __online_page_set_limits(),
> __online_page_increment_counters() and __online_page_free() function
> was added to ease generic hotplug operation.
>
There are several issues with this.
First, this is completely racy and only allows one global callback to be
in use at a time without looping, which is probably why you had to pass an
argument to restore_online_page_callback(). Your implementation also
requires that a callback must be synchronized with itself for the
comparison to generic_online_page to make any sense. Nobody knows which
callback is effective at any given moment and has no guarantees that when
they've set the callback that it will be the one called, otherwise.
Second, there's no explanation offered about why you have to split
online_page() into three separate functions. In addition, you've exported
all of them so presumably modules will need to be doing this when loading
or unloading and that further complicates the race mentioned above.
Third, there are no followup patches that use this interface or show how
you plan on using it (other than eluding that it will be used for virtual
machines in the changelog) so we're left guessing as to why we need it
implemented in this fashion and restricts the amount of help I can offer
because I don't know the problem you're facing.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. 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@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2011-05-19 3:36 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-17 21:38 Daniel Kiper
2011-05-19 3:36 ` David Rientjes [this message]
2011-05-19 20:45 ` Daniel Kiper
2011-05-19 23:01 ` Andrew Morton
2011-05-19 23:25 ` Daniel Kiper
2011-05-20 0:04 ` Andrew Morton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.00.1105182026390.20651@chino.kir.corp.google.com \
--to=rientjes@google.com \
--cc=akpm@linux-foundation.org \
--cc=andi.kleen@intel.com \
--cc=dan.magenheimer@oracle.com \
--cc=dave@linux.vnet.ibm.com \
--cc=dkiper@net-space.pl \
--cc=fengguang.wu@intel.com \
--cc=haicheng.li@linux.intel.com \
--cc=ian.campbell@citrix.com \
--cc=jeremy@goop.org \
--cc=konrad.wilk@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=pasik@iki.fi \
--cc=v.tolstov@selfip.ru \
--cc=wdauchy@gmail.com \
--cc=xen-devel@lists.xensource.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox