It may be useful to be able to access vm_committed_as from device
drivers. On the Hyper-V platform, the host has a policy engine to balance
the available physical memory amongst all competing virtual machines
hosted on a given node. This policy engine is driven by a number of metrics
including the memory pressure reported by the guests. The balloon driver
for Linux on Hyper-V uses this function to report memory pressure back to
the host.
Signed-off-by: K. Y. Srinivasan <[email protected]>
---
include/linux/mman.h | 2 ++
mm/mmap.c | 11 +++++++++++
mm/nommu.c | 11 +++++++++++
3 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/include/linux/mman.h b/include/linux/mman.h
index d09dde1..d53dc3d 100644
--- a/include/linux/mman.h
+++ b/include/linux/mman.h
@@ -11,6 +11,8 @@ extern int sysctl_overcommit_memory;
extern int sysctl_overcommit_ratio;
extern struct percpu_counter vm_committed_as;
+unsigned long read_vm_committed_as(void);
+
static inline void vm_acct_memory(long pages)
{
percpu_counter_add(&vm_committed_as, pages);
diff --git a/mm/mmap.c b/mm/mmap.c
index 2d94235..e527239 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -89,6 +89,17 @@ int sysctl_max_map_count __read_mostly = DEFAULT_MAX_MAP_COUNT;
struct percpu_counter vm_committed_as ____cacheline_aligned_in_smp;
/*
+ * A wrapper to read vm_committed_as that can be used by external modules.
+ */
+
+unsigned long read_vm_committed_as(void)
+{
+ return percpu_counter_read_positive(&vm_committed_as);
+}
+
+EXPORT_SYMBOL_GPL(read_vm_committed_as);
+
+/*
* Check that a process has enough memory to allocate a new virtual
* mapping. 0 means there is enough memory for the allocation to
* succeed and -ENOMEM implies there is not.
diff --git a/mm/nommu.c b/mm/nommu.c
index 45131b4..dbbd0aa 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -66,6 +66,17 @@ int heap_stack_gap = 0;
atomic_long_t mmap_pages_allocated;
+/*
+ * A wrapper to read vm_committed_as that can be used by external modules.
+ */
+
+unsigned long read_vm_committed_as(void)
+{
+ return percpu_counter_read_positive(&vm_committed_as);
+}
+
+EXPORT_SYMBOL_GPL(read_vm_committed_as);
+
EXPORT_SYMBOL(mem_map);
EXPORT_SYMBOL(num_physpages);
--
1.7.4.1
On Sat, 10 Nov 2012, K. Y. Srinivasan wrote:
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 2d94235..e527239 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -89,6 +89,17 @@ int sysctl_max_map_count __read_mostly = DEFAULT_MAX_MAP_COUNT;
> struct percpu_counter vm_committed_as ____cacheline_aligned_in_smp;
>
> /*
> + * A wrapper to read vm_committed_as that can be used by external modules.
> + */
> +
> +unsigned long read_vm_committed_as(void)
> +{
> + return percpu_counter_read_positive(&vm_committed_as);
> +}
> +
> +EXPORT_SYMBOL_GPL(read_vm_committed_as);
> +
> +/*
> * Check that a process has enough memory to allocate a new virtual
> * mapping. 0 means there is enough memory for the allocation to
> * succeed and -ENOMEM implies there is not.
This is precisely what I didn't want to see; I was expecting that this
function was going to have some name that would describe what a hypervisor
would use it for, regardless of its implementation and current use of
vm_committed_as. read_vm_committed_as() misses the entire point of the
suggestion and a few people have mentioned that they think this
implementation will evolve over time.
Please think of what you're trying to determine in the code that will
depend on this and then convert the existing user in
drivers/xen/xen-selfballoon.c.
> -----Original Message-----
> From: David Rientjes [mailto:[email protected]]
> Sent: Saturday, November 10, 2012 9:35 PM
> To: KY Srinivasan
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH 1/1] mm: Export a function to read vm_committed_as
>
> On Sat, 10 Nov 2012, K. Y. Srinivasan wrote:
>
> > diff --git a/mm/mmap.c b/mm/mmap.c
> > index 2d94235..e527239 100644
> > --- a/mm/mmap.c
> > +++ b/mm/mmap.c
> > @@ -89,6 +89,17 @@ int sysctl_max_map_count __read_mostly =
> DEFAULT_MAX_MAP_COUNT;
> > struct percpu_counter vm_committed_as ____cacheline_aligned_in_smp;
> >
> > /*
> > + * A wrapper to read vm_committed_as that can be used by external
> modules.
> > + */
> > +
> > +unsigned long read_vm_committed_as(void)
> > +{
> > + return percpu_counter_read_positive(&vm_committed_as);
> > +}
> > +
> > +EXPORT_SYMBOL_GPL(read_vm_committed_as);
> > +
> > +/*
> > * Check that a process has enough memory to allocate a new virtual
> > * mapping. 0 means there is enough memory for the allocation to
> > * succeed and -ENOMEM implies there is not.
>
> This is precisely what I didn't want to see; I was expecting that this
> function was going to have some name that would describe what a hypervisor
> would use it for, regardless of its implementation and current use of
> vm_committed_as. read_vm_committed_as() misses the entire point of the
> suggestion and a few people have mentioned that they think this
> implementation will evolve over time.
>
> Please think of what you're trying to determine in the code that will
> depend on this and then convert the existing user in
> drivers/xen/xen-selfballoon.c.
David,
Thanks for the prompt response. For the Linux balloon driver for Hyper-V, I need access
to the metric that reflects the system wide memory commitment made by the guest kernel.
In the Hyper-V case, this information is one of the many metrics used to drive the policy engine
on the host. Granted, the interface name I have chosen here could be more generic; how about
read_mem_commit_info(void). I am open to suggestions here.
With regards to making changes to the Xen self ballooning code, I would like to separate that patch
from the patch that implements the exported mechanism to access the memory commitment information.
Once we settle on this patch, I can submit the patch to fix the Xen self ballooning driver to use this new
interface along with the Hyper-V balloon driver that is currently blocked on resolving this issue.
Regards,
K. Y
> -----Original Message-----
> From: KY Srinivasan
> Sent: Sunday, November 11, 2012 4:24 AM
> To: 'David Rientjes'
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: RE: [PATCH 1/1] mm: Export a function to read vm_committed_as
>
>
>
> > -----Original Message-----
> > From: David Rientjes [mailto:[email protected]]
> > Sent: Saturday, November 10, 2012 9:35 PM
> > To: KY Srinivasan
> > Cc: [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]
> > Subject: Re: [PATCH 1/1] mm: Export a function to read vm_committed_as
> >
> > On Sat, 10 Nov 2012, K. Y. Srinivasan wrote:
> >
> > > diff --git a/mm/mmap.c b/mm/mmap.c
> > > index 2d94235..e527239 100644
> > > --- a/mm/mmap.c
> > > +++ b/mm/mmap.c
> > > @@ -89,6 +89,17 @@ int sysctl_max_map_count __read_mostly =
> > DEFAULT_MAX_MAP_COUNT;
> > > struct percpu_counter vm_committed_as ____cacheline_aligned_in_smp;
> > >
> > > /*
> > > + * A wrapper to read vm_committed_as that can be used by external
> > modules.
> > > + */
> > > +
> > > +unsigned long read_vm_committed_as(void)
> > > +{
> > > + return percpu_counter_read_positive(&vm_committed_as);
> > > +}
> > > +
> > > +EXPORT_SYMBOL_GPL(read_vm_committed_as);
> > > +
> > > +/*
> > > * Check that a process has enough memory to allocate a new virtual
> > > * mapping. 0 means there is enough memory for the allocation to
> > > * succeed and -ENOMEM implies there is not.
> >
> > This is precisely what I didn't want to see; I was expecting that this
> > function was going to have some name that would describe what a hypervisor
> > would use it for, regardless of its implementation and current use of
> > vm_committed_as. read_vm_committed_as() misses the entire point of the
> > suggestion and a few people have mentioned that they think this
> > implementation will evolve over time.
> >
> > Please think of what you're trying to determine in the code that will
> > depend on this and then convert the existing user in
> > drivers/xen/xen-selfballoon.c.
>
> David,
>
> Thanks for the prompt response. For the Linux balloon driver for Hyper-V, I need
> access
> to the metric that reflects the system wide memory commitment made by the
> guest kernel.
> In the Hyper-V case, this information is one of the many metrics used to drive the
> policy engine
> on the host. Granted, the interface name I have chosen here could be more
> generic; how about
> read_mem_commit_info(void). I am open to suggestions here.
>
> With regards to making changes to the Xen self ballooning code, I would like to
> separate that patch
> from the patch that implements the exported mechanism to access the memory
> commitment information.
> Once we settle on this patch, I can submit the patch to fix the Xen self ballooning
> driver to use this new
> interface along with the Hyper-V balloon driver that is currently blocked on
> resolving this issue.
David,
I want to finalize this ASAP. Please give me your feedback so I can move this forward.
Regards,
K. Y
On Sun, 11 Nov 2012, KY Srinivasan wrote:
> Thanks for the prompt response. For the Linux balloon driver for Hyper-V, I need access
> to the metric that reflects the system wide memory commitment made by the guest kernel.
> In the Hyper-V case, this information is one of the many metrics used to drive the policy engine
> on the host. Granted, the interface name I have chosen here could be more generic; how about
> read_mem_commit_info(void). I am open to suggestions here.
>
I would suggest vm_memory_committed() and there shouldn't be a comment
describing that this is just a wrapper for modules to read
vm_committed_as, that's apparent from the implementation: it should be
describing exactly what this value represents and why it is a useful
metric (at least in the case that you're concerned about).
> With regards to making changes to the Xen self ballooning code, I would like to separate that patch
> from the patch that implements the exported mechanism to access the memory commitment information.
Why? Is xen using it for a different inference?
> -----Original Message-----
> From: David Rientjes [mailto:[email protected]]
> Sent: Monday, November 12, 2012 4:54 PM
> To: KY Srinivasan
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: RE: [PATCH 1/1] mm: Export a function to read vm_committed_as
>
> On Sun, 11 Nov 2012, KY Srinivasan wrote:
>
> > Thanks for the prompt response. For the Linux balloon driver for Hyper-V, I
> need access
> > to the metric that reflects the system wide memory commitment made by the
> guest kernel.
> > In the Hyper-V case, this information is one of the many metrics used to drive
> the policy engine
> > on the host. Granted, the interface name I have chosen here could be more
> generic; how about
> > read_mem_commit_info(void). I am open to suggestions here.
> >
>
> I would suggest vm_memory_committed() and there shouldn't be a comment
> describing that this is just a wrapper for modules to read
> vm_committed_as, that's apparent from the implementation: it should be
> describing exactly what this value represents and why it is a useful
> metric (at least in the case that you're concerned about).
Will do; thanks.
>
> > With regards to making changes to the Xen self ballooning code, I would like to
> separate that patch
> > from the patch that implements the exported mechanism to access the
> memory commitment information.
>
> Why? Is xen using it for a different inference?
I think it is good to separate these patches. Dan (copied here) wrote the code for the
Xen self balloon driver. If it is ok with him I can submit the patch for Xen as well.
Regards,
K. Y
>
> From: KY Srinivasan [mailto:[email protected]]
> Sent: Monday, November 12, 2012 3:58 PM
> To: David Rientjes
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; [email protected]; Dan Magenheimer
> Subject: RE: [PATCH 1/1] mm: Export a function to read vm_committed_as
>
> > -----Original Message-----
> > From: David Rientjes [mailto:[email protected]]
> > Sent: Monday, November 12, 2012 4:54 PM
> > To: KY Srinivasan
> > Cc: [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]
> > Subject: RE: [PATCH 1/1] mm: Export a function to read vm_committed_as
> >
> > On Sun, 11 Nov 2012, KY Srinivasan wrote:
> >
> > > Thanks for the prompt response. For the Linux balloon driver for Hyper-V, I
> > need access
> > > to the metric that reflects the system wide memory commitment made by the
> > guest kernel.
> > > In the Hyper-V case, this information is one of the many metrics used to drive
> > the policy engine
> > > on the host. Granted, the interface name I have chosen here could be more
> > generic; how about
> > > read_mem_commit_info(void). I am open to suggestions here.
> > >
> >
> > I would suggest vm_memory_committed() and there shouldn't be a comment
> > describing that this is just a wrapper for modules to read
> > vm_committed_as, that's apparent from the implementation: it should be
> > describing exactly what this value represents and why it is a useful
> > metric (at least in the case that you're concerned about).
>
> Will do; thanks.
> >
> > > With regards to making changes to the Xen self ballooning code, I would like to
> > separate that patch
> > > from the patch that implements the exported mechanism to access the
> > memory commitment information.
> >
> > Why? Is xen using it for a different inference?
>
> I think it is good to separate these patches. Dan (copied here) wrote the code for the
> Xen self balloon driver. If it is ok with him I can submit the patch for Xen as well.
Hi KY --
If I understand correctly, this would be only a cosmetic (function renaming) change
to the Xen selfballooning code. If so, then I will be happy to Ack when I
see the patch. However, Konrad ([email protected]) is the maintainer
for all Xen code so you should ask him... and (from previous painful experience)
it can be difficult to sync even very simple interdependent changes going through
different maintainers without breaking linux-next. So I can't offer any
help with that process, only commiseration. :-(
Dan
On Mon, 12 Nov 2012, Dan Magenheimer wrote:
> > > Why? Is xen using it for a different inference?
> >
> > I think it is good to separate these patches. Dan (copied here) wrote the code for the
> > Xen self balloon driver. If it is ok with him I can submit the patch for Xen as well.
>
> Hi KY --
>
> If I understand correctly, this would be only a cosmetic (function renaming) change
> to the Xen selfballooning code. If so, then I will be happy to Ack when I
> see the patch. However, Konrad ([email protected]) is the maintainer
> for all Xen code so you should ask him... and (from previous painful experience)
> it can be difficult to sync even very simple interdependent changes going through
> different maintainers without breaking linux-next. So I can't offer any
> help with that process, only commiseration. :-(
>
I think this should be done in the same patch as the function getting
introduced with a cc to Konrad and routed through -mm; even better,
perhaps he'll have some useful comments for how this is used for xen that
can be included for context.
> -----Original Message-----
> From: Dan Magenheimer [mailto:[email protected]]
> Sent: Monday, November 12, 2012 6:32 PM
> To: KY Srinivasan; David Rientjes; Konrad Wilk
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: RE: [PATCH 1/1] mm: Export a function to read vm_committed_as
>
> > From: KY Srinivasan [mailto:[email protected]]
> > Sent: Monday, November 12, 2012 3:58 PM
> > To: David Rientjes
> > Cc: [email protected]; [email protected];
> [email protected];
> > [email protected]; [email protected]; [email protected]; akpm@linux-
> foundation.org; [email protected];
> > [email protected]; [email protected]; [email protected];
> [email protected]; Dan Magenheimer
> > Subject: RE: [PATCH 1/1] mm: Export a function to read vm_committed_as
> >
> > > -----Original Message-----
> > > From: David Rientjes [mailto:[email protected]]
> > > Sent: Monday, November 12, 2012 4:54 PM
> > > To: KY Srinivasan
> > > Cc: [email protected]; [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > [email protected]
> > > Subject: RE: [PATCH 1/1] mm: Export a function to read vm_committed_as
> > >
> > > On Sun, 11 Nov 2012, KY Srinivasan wrote:
> > >
> > > > Thanks for the prompt response. For the Linux balloon driver for Hyper-V, I
> > > need access
> > > > to the metric that reflects the system wide memory commitment made by
> the
> > > guest kernel.
> > > > In the Hyper-V case, this information is one of the many metrics used to
> drive
> > > the policy engine
> > > > on the host. Granted, the interface name I have chosen here could be more
> > > generic; how about
> > > > read_mem_commit_info(void). I am open to suggestions here.
> > > >
> > >
> > > I would suggest vm_memory_committed() and there shouldn't be a
> comment
> > > describing that this is just a wrapper for modules to read
> > > vm_committed_as, that's apparent from the implementation: it should be
> > > describing exactly what this value represents and why it is a useful
> > > metric (at least in the case that you're concerned about).
> >
> > Will do; thanks.
> > >
> > > > With regards to making changes to the Xen self ballooning code, I would like
> to
> > > separate that patch
> > > > from the patch that implements the exported mechanism to access the
> > > memory commitment information.
> > >
> > > Why? Is xen using it for a different inference?
> >
> > I think it is good to separate these patches. Dan (copied here) wrote the code
> for the
> > Xen self balloon driver. If it is ok with him I can submit the patch for Xen as well.
>
> Hi KY --
>
> If I understand correctly, this would be only a cosmetic (function renaming)
> change
> to the Xen selfballooning code. If so, then I will be happy to Ack when I
> see the patch. However, Konrad ([email protected]) is the maintainer
> for all Xen code so you should ask him... and (from previous painful experience)
> it can be difficult to sync even very simple interdependent changes going through
> different maintainers without breaking linux-next. So I can't offer any
> help with that process, only commiseration. :-(
>
> Dan
>
Dan,
Thank you. I will send the patches out soon.
Regards,
K. Y
> -----Original Message-----
> From: David Rientjes [mailto:[email protected]]
> Sent: Monday, November 12, 2012 6:49 PM
> To: Dan Magenheimer
> Cc: KY Srinivasan; Konrad Wilk; [email protected]; linux-
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: RE: [PATCH 1/1] mm: Export a function to read vm_committed_as
>
> On Mon, 12 Nov 2012, Dan Magenheimer wrote:
>
> > > > Why? Is xen using it for a different inference?
> > >
> > > I think it is good to separate these patches. Dan (copied here) wrote the code
> for the
> > > Xen self balloon driver. If it is ok with him I can submit the patch for Xen as
> well.
> >
> > Hi KY --
> >
> > If I understand correctly, this would be only a cosmetic (function renaming)
> change
> > to the Xen selfballooning code. If so, then I will be happy to Ack when I
> > see the patch. However, Konrad ([email protected]) is the maintainer
> > for all Xen code so you should ask him... and (from previous painful experience)
> > it can be difficult to sync even very simple interdependent changes going
> through
> > different maintainers without breaking linux-next. So I can't offer any
> > help with that process, only commiseration. :-(
> >
>
> I think this should be done in the same patch as the function getting
> introduced with a cc to Konrad and routed through -mm; even better,
> perhaps he'll have some useful comments for how this is used for xen that
> can be included for context.
>
Ok; I will send out a single patch. I am hoping this can be applied soon as Hyper-V balloon
driver is queued behind this.
Regards,
K. Y
> From: KY Srinivasan [mailto:[email protected]]
> Subject: RE: [PATCH 1/1] mm: Export a function to read vm_committed_as
>
> > From: David Rientjes [mailto:[email protected]]
> > Sent: Monday, November 12, 2012 6:49 PM
> > To: Dan Magenheimer
> > Cc: KY Srinivasan; Konrad Wilk; [email protected]; linux-
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected]; linux-
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]
> > Subject: RE: [PATCH 1/1] mm: Export a function to read vm_committed_as
> >
> > On Mon, 12 Nov 2012, Dan Magenheimer wrote:
> >
> > > > > Why? Is xen using it for a different inference?
> > > >
> > > > I think it is good to separate these patches. Dan (copied here) wrote the code
> > for the
> > > > Xen self balloon driver. If it is ok with him I can submit the patch for Xen as
> > well.
> > >
> > > Hi KY --
> > >
> > > If I understand correctly, this would be only a cosmetic (function renaming)
> > change
> > > to the Xen selfballooning code. If so, then I will be happy to Ack when I
> > > see the patch. However, Konrad ([email protected]) is the maintainer
> > > for all Xen code so you should ask him... and (from previous painful experience)
> > > it can be difficult to sync even very simple interdependent changes going
> > through
> > > different maintainers without breaking linux-next. So I can't offer any
> > > help with that process, only commiseration. :-(
> > >
> >
> > I think this should be done in the same patch as the function getting
> > introduced with a cc to Konrad and routed through -mm; even better,
> > perhaps he'll have some useful comments for how this is used for xen that
> > can be included for context.
> >
> Ok; I will send out a single patch. I am hoping this can be applied soon as Hyper-V balloon
> driver is queued behind this.
>
> Regards,
> K. Y
David --
Having caught up on the thread now, I'm a bit confused about your
requirement for KY to patch the Xen selfballooning code.
The data item we are talking about here, committed_as, is defined
by a kernel<->userland ABI, visible to userland via /proc/meminfo.
The Xen selfballoon driver accesses it within the kernel as
a built-in; this driver could potentially be loaded as a
module but currently cannot.
KY is simply asking that the data item be exported so that he can
use it from a new module. No change to the Xen selfballoon driver
is necessary right now and requiring one only gets in the way of the
patch. At some future time, the Xen selfballoon driver can, at its
leisure, switch to use the new exported function but need not
unless/until it is capable of being loaded as a module.
And, IIUC, you are asking that KY's proposed new function include a
comment about how it is used by Xen? How many kernel globals/functions
document at their point of declaration the intent of all the in-kernel
users that use/call them? That seems a bit unreasonable. There is a
very long explanatory comment at the beginning of the Xen
selfballoon driver code already.
So I will ack KY's patch (I see it was just sent) but will leave
it up to Konrad and GregKH and Andrew to decide whether to
include the fragment patching the Xen selfballoon driver.
Dan
On Tue, 13 Nov 2012, Dan Magenheimer wrote:
> KY is simply asking that the data item be exported so that he can
> use it from a new module. No change to the Xen selfballoon driver
> is necessary right now and requiring one only gets in the way of the
> patch. At some future time, the Xen selfballoon driver can, at its
> leisure, switch to use the new exported function but need not
> unless/until it is capable of being loaded as a module.
>
That's obvious.
> And, IIUC, you are asking that KY's proposed new function include a
> comment about how it is used by Xen? How many kernel globals/functions
> document at their point of declaration the intent of all the in-kernel
> users that use/call them? That seems a bit unreasonable. There is a
> very long explanatory comment at the beginning of the Xen
> selfballoon driver code already.
>
Sorry, I don't think it's unreasonable at all: if you're going to be using
a symbol which was always assumed to be internal to the VM for other
purposes and then that usage becomes convoluted with additional usage like
in KY's patch, then no VM hacker will ever know what a change to that
symbol means outside of the VM. There's been a lot of confusion about why
this heuristic is needed outside the VM and whether the symbol is actually
the correct choice, so verbosity as to the intent of what it is to
represent is helpful for a maintainable kernel.
Presumably xen is hijacking that symbol for a similar purpose to KY's
purpose, but perhaps I was too optimistic that others would help to
solidify the semantics in which it is being used and describe it
concisely.