From: Andrew Morton on
On Thu, 15 Apr 2010 14:00:31 -0700
Dmitry Torokhov <dtor(a)vmware.com> wrote:

> This is standalone version of VMware Balloon driver. Ballooning is a
> technique that allows hypervisor dynamically limit the amount of memory
> available to the guest (with guest cooperation). In the overcommit
> scenario, when hypervisor set detects that it needs to shuffle some memory,
> it instructs the driver to allocate certain number of pages, and the
> underlying memory gets returned to the hypervisor. Later hypervisor may
> return memory to the guest by reattaching memory to the pageframes and
> instructing the driver to "deflate" balloon.
>
> Signed-off-by: Dmitry Torokhov <dtor(a)vmware.com>
> ---
>
> Unlike previous version, that tried to integrate VMware ballooning transport
> into virtio subsystem, and use stock virtio_ballon driver, this one implements
> both controlling thread/algorithm and hypervisor transport.
>
> We are submitting standalone driver because KVM maintainer (Avi Kivity)
> expressed opinion (rightly) that our transport does not fit well into
> virtqueue paradigm and thus it does not make much sense to integrate
> with virtio.
>
> There were also some concerns whether current ballooning technique is
> the right thing. If there appears a better framework to achieve this we
> are prepared to evaluate and switch to using it, but in the meantime
> we'd like to get this driver upstream.
>
> Changes since v1:
> - added comments throughout the code;
> - exported stats moved from /proc to debugfs;
> - better changelog.
>
>
> ...
>
> +#define VMW_BALLOON_NOSLEEP_ALLOC_MAX 16384U
> +
> +#define VMW_BALLOON_RATE_ALLOC_MIN 512U
> +#define VMW_BALLOON_RATE_ALLOC_MAX 2048U
> +#define VMW_BALLOON_RATE_ALLOC_INC 16U
> +
> +#define VMW_BALLOON_RATE_FREE_MIN 512U
> +#define VMW_BALLOON_RATE_FREE_MAX 16384U
> +#define VMW_BALLOON_RATE_FREE_INC 16U

hum. What do these do and what units are they in? Needs a comment?

>
> ...
>
> +#define VMWARE_BALLOON_CMD(cmd, data, result) \
> +({ \
> + unsigned long __stat, __dummy1, __dummy2; \
> + __asm__ __volatile__ ("inl (%%dx)" : \
> + "=a"(__stat), \
> + "=c"(__dummy1), \
> + "=d"(__dummy2), \
> + "=b"(result) : \
> + "0"(VMW_BALLOON_HV_MAGIC), \
> + "1"(VMW_BALLOON_CMD_##cmd), \
> + "2"(VMW_BALLOON_HV_PORT), \
> + "3"(data) : \
> + "memory"); \
> + result &= -1UL; \
> + __stat & -1UL; \
> +})

This is OK for both x86_32 and x86_64?

Was it actually intended that this driver be enabled for 32-bit?

> +#define STATS_INC(stat) (stat)++
> +
> +struct vmballoon_stats {
> + unsigned int timer;
> +
> + /* allocation statustics */
> + unsigned int alloc;
> + unsigned int alloc_fail;
> + unsigned int sleep_alloc;
> + unsigned int sleep_alloc_fail;
> + unsigned int refused_alloc;
> + unsigned int refused_free;
> + unsigned int free;
> +
> + /* monitor operations */
> + unsigned int lock;
> + unsigned int lock_fail;
> + unsigned int unlock;
> + unsigned int unlock_fail;
> + unsigned int target;
> + unsigned int target_fail;
> + unsigned int start;
> + unsigned int start_fail;
> + unsigned int guest_type;
> + unsigned int guest_type_fail;
> +};
> +
> +struct vmballoon {
> +
> + /* list of reserved physical pages */
> + struct list_head pages;
> +
> + /* transient list of non-balloonable pages */
> + struct list_head refused_pages;
> +
> + /* balloon size in pages */
> + unsigned int size;
> + unsigned int target;
> +
> + /* reset flag */
> + bool reset_required;
> +
> + /* adjustment rates (pages per second) */
> + unsigned int rate_alloc;
> + unsigned int rate_free;
> +
> + /* slowdown page allocations for next few cycles */
> + unsigned int slow_allocation_cycles;
> +
> + /* statistics */
> + struct vmballoon_stats stats;
> +
> + /* debugfs file exporting statistics */
> + struct dentry *dbg_entry;
> +
> + struct sysinfo sysinfo;
> +
> + struct delayed_work dwork;
> +};

afaict all the stats stuff is useless if CONFIG_DEBUG_FS=n. Perhaps in
that case the vmballoon.stats field should be omitted and STATS_INC
be made a no-op?

>
> ...
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Dmitry Torokhov on
On Wed, Apr 21, 2010 at 02:13:25PM -0700, Andrew Morton wrote:
> On Wed, 21 Apr 2010 13:52:08 -0700
> Dmitry Torokhov <dtor(a)vmware.com> wrote:
>
> > On Wed, Apr 21, 2010 at 01:18:36PM -0700, Andrew Morton wrote:
> > > On Wed, 21 Apr 2010 12:59:35 -0700
> > > Dmitry Torokhov <dtor(a)vmware.com> wrote:
> > >
> > > > Do you see any issues with the driver?
> > >
> > > Only my near-complete cluelessness on the whole topic.
> > >
> > > > Will you be the one picking it
> > > > up and queueing for mainline?
> > >
> > > Spose so.
> >
> > Good. I don't suppose we have a chance making into .34? Being a
> > completely new driver and all...
>
> It's foggy. Is there a good-sounding reason for pushing it in this
> late?
>

We want to get the driver accepted in distributions so that users do not
have to deal with an out-of-tree module and many distributions have
"upstream first" requirement.

The driver has been shipping for a number of years and users running on
VMware platform will have it installed as part of VMware Tools even if
it will not come from a distribution, thus there should not be
additional risk in pulling the driver into mainline. The driver will
only activate if host is VMware so everyone else should not be affected
at all.

Thanks,

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