Re: [PATCH] GSoC 2010 - Memory hotplug support for Xen guests - secondfully working version - once again

[Jeremy Fitzhardinge]

 On 08/06/2010 09:34 AM, Konrad Rzeszutek Wilk wrote:
> On Fri, Aug 06, 2010 at 01:11:47PM +0200, Daniel Kiper wrote:
> > Hi,
> >
> > I am sending this e-mail once again because it probably
> > has been lost in abyss of Xen-devel/LKLM list.
> >
> > Here is the second version of memory hotplug support
> > for Xen guests patch. This one cleanly applies to
> > git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git
> > repository, xen/memory-hotplug head.
> >
> > Changes:
> >    - /sys/devices/system/memory/probe interface has been removed;
> >      /sys/devices/system/xen_memory/xen_memory0/{target,target_kb}
> >      are much better (I forgot about them),
> >    - most of the code have been moved to drivers/xen/balloon.c,
> >    - this changes forced me to export hotadd_new_pgdat and
> >      rollback_node_hotadd function from mm/memory_hotplug.c;
> >      could it be accepted by mm/memory_hotplug.c maintainers ???
> >    - PV on HVM mode is supported now; it was tested on
> >      git://xenbits.xen.org/people/sstabellini/linux-pvhvm.git
> >      repository, 2.6.34-pvhvm head,
> >    - most of Jeremy suggestions have been applied.
> >
> > On Wed, Jul 28, 2010 at 11:36:29AM +0400, Vasiliy G Tolstov wrote:
> > [...]
> > > Work's fine with opensuse 11.3 (dom0 and domU)
> > Thx.
> >
> > On Thu, Jul 29, 2010 at 12:39:52PM -0700, Jeremy Fitzhardinge wrote:
> > >   On 07/26/2010 05:41 PM, Daniel Kiper wrote:
> > > > Hi,
> > > >
> > > > Currently there is fully working version.
> > > > It has been tested on Xen Ver. 4.0.0 in PV
> > > > guest i386/x86_64 with Linux kernel Ver. 2.6.32.16
> > > > and Ver. 2.6.34.1. This patch cleanly applys
> > > > to Ver. 2.6.34.1
> > > Thanks.  I've pushed this into xen.git as xen/memory-hotplug so people
> > > can play with it more easily (but I haven't merged it into any of the
> > > other branches yet).
> > Thx.
> >
> > > > +#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
> > > > +static inline unsigned long current_target(void)
> > > > +{
> > > > +	return balloon_stats.target_pages;
> > > Why does this need its own version?
> > Because original version return values not bigger
> > then initial memory allocation which does not allow
> > memory hotplug to function.
> >
> > > > +int __ref xen_add_memory(int nid, u64 start, u64 size)
> > > > +{
> > > > +	pg_data_t *pgdat = NULL;
> > > > +	int new_pgdat = 0, ret;
> > > > +
> > > > +	lock_system_sleep();
> > > > +
> > > > +	if (!node_online(nid)) {
> > > > +		pgdat = hotadd_new_pgdat(nid, start);
> > > > +		ret = -ENOMEM;
> > > > +		if (!pgdat)
> > > > +			goto out;
> > > > +		new_pgdat = 1;
> > > > +	}
> > > > +
> > > > +	/* call arch's memory hotadd */
> > > > +	ret = arch_add_memory(nid, start, size);
> > > > +
> > > > +	if (ret<   0)
> > > > +		goto error;
> > > > +
> > > > +	/* we online node here. we can't roll back from here. */
> > > > +	node_set_online(nid);
> > > > +
> > > > +	if (new_pgdat) {
> > > > +		ret = register_one_node(nid);
> > > > +		/*
> > > > +		 * If sysfs file of new node can't create, cpu on the node
> > > > +		 * can't be hot-added. There is no rollback way now.
> > > > +		 * So, check by BUG_ON() to catch it reluctantly..
> > > > +		 */
> > > > +		BUG_ON(ret);
> > > > +	}
> > > This doesn't seem to be doing anything particularly xen-specific.
> > In general it could be generic however I do not know
> > it will be useful for others. If this function would
> > be accepted by mm/memory_hotplug.c maintainers we could
> > move it there. I removed from original add_memory funtion
> > resource allocation (and deallocation after error), which
> > must be done before XENMEM_populate_physmap in Xen. xen_add_memory
> > is called after physmap is fully populated.
> >
> > If you have a questions please drop me a line.
>
>
> Can you repost a patch that is on top of a virgin tree please?

I just thanked him for posting a delta ;)  I've pushed this into 
xen/memory-hotplug so you can easily generate a complete diff with "git 
diff v2.6.34..xen/memory-hotplug".

> + spin_lock_irqsave(&balloon_lock, flags);
> > +
> > +	if (!is_memory_resource_reserved()) {
> > +
> > +		/*
> > +		 * Look for first unused memory region starting at page
> > +		 * boundary. Skip last memory section created at boot time
> > +		 * becuase it may contains unused memory pages with PG_reserved
> > +		 * bit not set (online_pages require PG_reserved bit set).
> > +		 */
> > +
> > +		r = kzalloc(sizeof(struct resource), GFP_KERNEL);
>
>
> You are holding a spinlock here. Kzalloc can sleep

Good point.

    J
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/