| Message ID | 1584395264-22913-1-git-send-email-igor.druzhinin@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/shim: fix ballooning up the guest | expand |
On 16.03.2020 22:47, Igor Druzhinin wrote: > args.preempted as meaningless here and doesn't show if the hypercall > was preempted before. Use start_extent instead which is correct. ... as long as the hypercall was invoked in a "normal" way. > Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com> > --- > This fixes ballooning for 64-bit PV guests inside shim. 32-bit PV guests > require a little bit more work due to compat layer being involved. > --- > xen/common/memory.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/xen/common/memory.c b/xen/common/memory.c > index 444c081..5fdd2a2 100644 > --- a/xen/common/memory.c > +++ b/xen/common/memory.c > @@ -1191,7 +1191,7 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > } > > #ifdef CONFIG_X86 > - if ( pv_shim && op != XENMEM_decrease_reservation && !args.preempted ) > + if ( pv_shim && op != XENMEM_decrease_reservation && !start_extent ) > /* Avoid calling pv_shim_online_memory when preempted. */ > pv_shim_online_memory(args.nr_extents, args.extent_order); The code change can have my R-b, but I'd like the comment to also be changed then - it shouldn't talk about preemption in the way it does. Perhaps "... when this is a continuation"? I'd be okay making this change while committing. As an aside, it would probably have been a good idea to also Cc Roger as the original author of this code. Jan
On 17/03/2020 10:24, Jan Beulich wrote: > On 16.03.2020 22:47, Igor Druzhinin wrote: >> args.preempted as meaningless here and doesn't show if the hypercall >> was preempted before. Use start_extent instead which is correct. > > ... as long as the hypercall was invoked in a "normal" way. > >> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com> >> --- >> This fixes ballooning for 64-bit PV guests inside shim. 32-bit PV guests >> require a little bit more work due to compat layer being involved. >> --- >> xen/common/memory.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/xen/common/memory.c b/xen/common/memory.c >> index 444c081..5fdd2a2 100644 >> --- a/xen/common/memory.c >> +++ b/xen/common/memory.c >> @@ -1191,7 +1191,7 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) >> } >> >> #ifdef CONFIG_X86 >> - if ( pv_shim && op != XENMEM_decrease_reservation && !args.preempted ) >> + if ( pv_shim && op != XENMEM_decrease_reservation && !start_extent ) >> /* Avoid calling pv_shim_online_memory when preempted. */ >> pv_shim_online_memory(args.nr_extents, args.extent_order); > > The code change can have my R-b, but I'd like the comment to also > be changed then - it shouldn't talk about preemption in the way > it does. Perhaps "... when this is a continuation"? I'd be okay > making this change while committing. Sure, if you like. > As an aside, it would probably have been a good idea to also Cc > Roger as the original author of this code. I didn't check "git blame" and he wasn't on a maintainers list. Igor
On Mon, Mar 16, 2020 at 09:47:44PM +0000, Igor Druzhinin wrote: > args.preempted as meaningless here and doesn't show if the hypercall ^ is ^ as it doesn't signal whether the hypercall ... > was preempted before. Use start_extent instead which is correct. > > Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> FWIW, I agree with the change to the comment requested by Jan, I think this can all be done while committing. Thanks, Roger.
diff --git a/xen/common/memory.c b/xen/common/memory.c index 444c081..5fdd2a2 100644 --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -1191,7 +1191,7 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) } #ifdef CONFIG_X86 - if ( pv_shim && op != XENMEM_decrease_reservation && !args.preempted ) + if ( pv_shim && op != XENMEM_decrease_reservation && !start_extent ) /* Avoid calling pv_shim_online_memory when preempted. */ pv_shim_online_memory(args.nr_extents, args.extent_order); #endif
args.preempted as meaningless here and doesn't show if the hypercall was preempted before. Use start_extent instead which is correct. Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com> --- This fixes ballooning for 64-bit PV guests inside shim. 32-bit PV guests require a little bit more work due to compat layer being involved. --- xen/common/memory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)