[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2] tools/libxl: make default of max event channels dependant on vcpus



On 06.04.20 12:37, Julien Grall wrote:
Hi Juergen,

On 06/04/2020 11:17, Jürgen Groß wrote:
On 06.04.20 11:24, Julien Grall wrote:
Hi Jurgen,

On 06/04/2020 09:27, Juergen Gross wrote:
Since Xen 4.4 the maximum number of event channels for a guest is
defaulting to 1023. For large guests with lots of vcpus this is not
enough, as e.g. the Linux kernel uses 7 event channels per vcpu,
limiting the guest to about 140 vcpus.

Large guests on which arch? Which type of guests?

I'm pretty sure this applies to x86 only. I'm not aware of event
channels being used on ARM for IPIs.

How about the guest types?

PV and HVM with PV enhancements.




Instead of requiring to specify the allowed number of event channels
via the "event_channels" domain config option, make the default
depend on the maximum number of vcpus of the guest. This will require
to use the "event_channels" domain config option in fewer cases as
before.

As different guests will have differing needs the calculation of the
maximum number of event channels can be a rough estimate only,
currently based on the Linux kernel requirements.

I am not overly happy to extend the default numbers of event channels for everyone based on Linux behavior on a given setup. Yes you have more guests that would be able to run, but at the expense of allowing a guest to use more xen memory.

The resulting number would be larger than today only for guests with
more than 96 vcpus. So I don't think the additional amount of memory
is really that problematic.
This is not a very forward looking argument. For Arm, we limit to 128 vCPUs at the moment but it would be possible to support many more (I think our vGIC implementation support up to 4096 vCPUs).

So even if your change impacts a small subset, each architectures should be able to make the decision on the limit and not imposed by x86 Linux PV.

Okay, what about moving the default setting of b_info->event_channels
into libxl__arch_domain_build_info_setdefault() then?




For instance, I don't think this limit increase is necessary on Arm.

Nevertheless it is
much better than the static upper limit of today as more guests will
boot just fine with the new approach.

In order not to regress current configs use 1023 as the minimum
default setting.

Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
---
V2:
- use max() instead of min()
- clarify commit message a little bit
---
  tools/libxl/libxl_create.c | 2 +-

The documentation should be updated.

Oh, indeed.


  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index e7cb2dbc2b..c025b21894 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -226,7 +226,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
              b_info->iomem[i].gfn = b_info->iomem[i].start;
      if (!b_info->event_channels)
-        b_info->event_channels = 1023;
+        b_info->event_channels = max(1023, b_info->max_vcpus * 8 + 255);

What is the 255 for?

Just some headroom for e.g. pv devices.

That should really be explained in the commit message and a comment.

Okay.


Juergen



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.