| Message ID | 1503629540-26053-3-git-send-email-tianyu.lan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Aug 24, 2017 at 10:52:17PM -0400, Lan Tianyu wrote: > This patch is to increase event channels to support more vcpus in single VM. > > Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> There is no need to bump the default. There is already a configuration option called max_event_channel.
On Fri, Aug 25, 2017 at 10:18:24AM +0100, Wei Liu wrote: > On Thu, Aug 24, 2017 at 10:52:17PM -0400, Lan Tianyu wrote: > > This patch is to increase event channels to support more vcpus in single VM. > > > > Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> > > There is no need to bump the default. There is already a configuration > option called max_event_channel. Maybe make this somehow based on the number of vCPUs assigned to the domain? It's not very used-friendly to allow the creation of a domain with 256 vCPUs for example that would then get stuck during boot. Or at least check max_event_channel and the number of vCPUs and print a warning message to alert the user that things might go wrong with this configuration. Roger.
On Fri, Aug 25, 2017 at 10:57:26AM +0100, Roger Pau Monné wrote: > On Fri, Aug 25, 2017 at 10:18:24AM +0100, Wei Liu wrote: > > On Thu, Aug 24, 2017 at 10:52:17PM -0400, Lan Tianyu wrote: > > > This patch is to increase event channels to support more vcpus in single VM. > > > > > > Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> > > > > There is no need to bump the default. There is already a configuration > > option called max_event_channel. > > Maybe make this somehow based on the number of vCPUs assigned to the > domain? > > It's not very used-friendly to allow the creation of a domain with 256 > vCPUs for example that would then get stuck during boot. > > Or at least check max_event_channel and the number of vCPUs and print > a warning message to alert the user that things might go wrong with > this configuration. > The problem is number of vcpu is only one factor that would affect the number of event channels needed.
On 8/25/2017 6:04 PM, Wei Liu wrote: > On Fri, Aug 25, 2017 at 10:57:26AM +0100, Roger Pau Monné wrote: >> On Fri, Aug 25, 2017 at 10:18:24AM +0100, Wei Liu wrote: >>> On Thu, Aug 24, 2017 at 10:52:17PM -0400, Lan Tianyu wrote: >>>> This patch is to increase event channels to support more vcpus in single VM. >>>> >>>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> >>> >>> There is no need to bump the default. There is already a configuration >>> option called max_event_channel. >> >> Maybe make this somehow based on the number of vCPUs assigned to the >> domain? >> >> It's not very used-friendly to allow the creation of a domain with 256 >> vCPUs for example that would then get stuck during boot. >> >> Or at least check max_event_channel and the number of vCPUs and print >> a warning message to alert the user that things might go wrong with >> this configuration. >> > > The problem is number of vcpu is only one factor that would affect the > number of event channels needed. How about producing a warning about event channel maybe not enough when vcpu number is >128 and still uses default max event channel number?
On Mon, Aug 28, 2017 at 05:11:27PM +0800, Lan, Tianyu wrote: > On 8/25/2017 6:04 PM, Wei Liu wrote: > > On Fri, Aug 25, 2017 at 10:57:26AM +0100, Roger Pau Monné wrote: > > > On Fri, Aug 25, 2017 at 10:18:24AM +0100, Wei Liu wrote: > > > > On Thu, Aug 24, 2017 at 10:52:17PM -0400, Lan Tianyu wrote: > > > > > This patch is to increase event channels to support more vcpus in single VM. > > > > > > > > > > Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> > > > > > > > > There is no need to bump the default. There is already a configuration > > > > option called max_event_channel. > > > > > > Maybe make this somehow based on the number of vCPUs assigned to the > > > domain? > > > > > > It's not very used-friendly to allow the creation of a domain with 256 > > > vCPUs for example that would then get stuck during boot. > > > > > > Or at least check max_event_channel and the number of vCPUs and print > > > a warning message to alert the user that things might go wrong with > > > this configuration. > > > > > > > The problem is number of vcpu is only one factor that would affect the > > number of event channels needed. > > How about producing a warning about event channel maybe not enough when vcpu > number is >128 and still uses default max event channel number? > Maybe. If you're going to do that, please: 1. provide the heuristic in a function so that it can be expand later. 2. make the message system administrator friendly, point them to the max_event_channels option.
>>> On 28.08.17 at 11:11, <tianyu.lan@intel.com> wrote: > On 8/25/2017 6:04 PM, Wei Liu wrote: >> On Fri, Aug 25, 2017 at 10:57:26AM +0100, Roger Pau Monné wrote: >>> On Fri, Aug 25, 2017 at 10:18:24AM +0100, Wei Liu wrote: >>>> On Thu, Aug 24, 2017 at 10:52:17PM -0400, Lan Tianyu wrote: >>>>> This patch is to increase event channels to support more vcpus in single VM. >>>>> >>>>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> >>>> >>>> There is no need to bump the default. There is already a configuration >>>> option called max_event_channel. >>> >>> Maybe make this somehow based on the number of vCPUs assigned to the >>> domain? >>> >>> It's not very used-friendly to allow the creation of a domain with 256 >>> vCPUs for example that would then get stuck during boot. >>> >>> Or at least check max_event_channel and the number of vCPUs and print >>> a warning message to alert the user that things might go wrong with >>> this configuration. >>> >> >> The problem is number of vcpu is only one factor that would affect the >> number of event channels needed. > > How about producing a warning about event channel maybe not enough when > vcpu number is >128 and still uses default max event channel number? There would be nothing wrong with that for a guest not using PV drivers, or not requiring multiple channels per vCPU. Hence I'm not convinced a warning is appropriate here. Jan
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 1158303..3937169 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -210,7 +210,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 = 4095; libxl__arch_domain_build_info_acpi_setdefault(b_info);
This patch is to increase event channels to support more vcpus in single VM. Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> --- tools/libxl/libxl_create.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)