diff mbox series

tools/libxl: make default of max event channels dependant on vcpus

Message ID 20200326094557.13822-1-jgross@suse.com (mailing list archive)
State Superseded
Headers show
Series tools/libxl: make default of max event channels dependant on vcpus | expand

Commit Message

Jürgen Groß March 26, 2020, 9:45 a.m. UTC
Today 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.

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.

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

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/libxl/libxl_create.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jan Beulich March 26, 2020, 9:54 a.m. UTC | #1
On 26.03.2020 10:45, Juergen Gross wrote:
> Today 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.

I don't think any particular guest OS should "dictate" the default
here. What if another OS (or even a future version of Linux) requires
more than 8? And why would the value depend on number of vCPU-s but
not number and kind of virtual devices?

Jan
Jürgen Groß March 26, 2020, 10 a.m. UTC | #2
On 26.03.20 10:54, Jan Beulich wrote:
> On 26.03.2020 10:45, Juergen Gross wrote:
>> Today 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.
> 
> I don't think any particular guest OS should "dictate" the default
> here. What if another OS (or even a future version of Linux) requires
> more than 8? And why would the value depend on number of vCPU-s but
> not number and kind of virtual devices?

It is a rough estimate. And with HVM guests being limited to 128 cpus
anyway, this problem is more relevant for PV guests, limiting the
available options for guest OS's.

You can always specify a larger value in the guest config after all.


Juergen
Jan Beulich March 26, 2020, 10:05 a.m. UTC | #3
On 26.03.2020 11:00, Jürgen Groß wrote:
> On 26.03.20 10:54, Jan Beulich wrote:
>> On 26.03.2020 10:45, Juergen Gross wrote:
>>> Today 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.
>>
>> I don't think any particular guest OS should "dictate" the default
>> here. What if another OS (or even a future version of Linux) requires
>> more than 8? And why would the value depend on number of vCPU-s but
>> not number and kind of virtual devices?
> 
> It is a rough estimate. And with HVM guests being limited to 128 cpus
> anyway, this problem is more relevant for PV guests, limiting the
> available options for guest OS's.

How many evtchns would e.g. mini-os require? I.e. wouldn't such stubdoms
get a much larger than necessary allowance this way?

Jan
Jürgen Groß March 26, 2020, 10:11 a.m. UTC | #4
On 26.03.20 11:05, Jan Beulich wrote:
> On 26.03.2020 11:00, Jürgen Groß wrote:
>> On 26.03.20 10:54, Jan Beulich wrote:
>>> On 26.03.2020 10:45, Juergen Gross wrote:
>>>> Today 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.
>>>
>>> I don't think any particular guest OS should "dictate" the default
>>> here. What if another OS (or even a future version of Linux) requires
>>> more than 8? And why would the value depend on number of vCPU-s but
>>> not number and kind of virtual devices?
>>
>> It is a rough estimate. And with HVM guests being limited to 128 cpus
>> anyway, this problem is more relevant for PV guests, limiting the
>> available options for guest OS's.
> 
> How many evtchns would e.g. mini-os require? I.e. wouldn't such stubdoms
> get a much larger than necessary allowance this way?

mini-os doesn't support SMP. So (if not configured with lots of vcpus,
which doesn't make sense) they still get 1023 event channels per
default. Which is much more as they need, but the same as today.

Xenstore stubdom is unlimited like dom0, BTW.


Juergen
Jürgen Groß March 26, 2020, 10:29 a.m. UTC | #5
On 26.03.20 10:45, Juergen Gross wrote:
> Today 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.
> 
> 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.
> 
> In order not to regress current configs use 1023 as the minimum
> default setting.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>   tools/libxl/libxl_create.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index e7cb2dbc2b..eddd0e98e5 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 = min(1023, b_info->max_vcpus * 8 + 255);

Sorry, forgot to do "stg refresh", this should be max(), of course.


Juergen
Andrew Cooper March 26, 2020, 12:36 p.m. UTC | #6
On 26/03/2020 09:45, Juergen Gross wrote:
> Today 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.
>
> 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.
>
> In order not to regress current configs use 1023 as the minimum
> default setting.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>

:(

I suspect I regressed this by making it not actually dead code.

1023 is the 32bit EVT 2L ABI limit, but until I moved max_evtchn into
the createdomain hypercall, I'm pretty sure a limit never used to get set.

If this is the case, then unlimited should be the default.

~Andrew
Jürgen Groß March 26, 2020, 12:42 p.m. UTC | #7
On 26.03.20 13:36, Andrew Cooper wrote:
> On 26/03/2020 09:45, Juergen Gross wrote:
>> Today 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.
>>
>> 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.
>>
>> In order not to regress current configs use 1023 as the minimum
>> default setting.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> :(
> 
> I suspect I regressed this by making it not actually dead code.
> 
> 1023 is the 32bit EVT 2L ABI limit, but until I moved max_evtchn into
> the createdomain hypercall, I'm pretty sure a limit never used to get set.

In 4.11 I'm seeing it being set via xc_domain_set_max_evtchn().


Juergen
diff mbox series

Patch

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index e7cb2dbc2b..eddd0e98e5 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 = min(1023, b_info->max_vcpus * 8 + 255);
 
     libxl__arch_domain_build_info_setdefault(gc, b_info);
     libxl_defbool_setdefault(&b_info->dm_restrict, false);