| Message ID | 1438180163-275465-3-git-send-email-imammedo@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 29, 2015 at 04:29:23PM +0200, Igor Mammedov wrote: > although now there is vhost module max_mem_regions option > to set custom limit it doesn't help for default setups, > since it requires administrator manually set a higher > limit on each host. Which complicates servers deployments > and management. > Rise limit to the same value as KVM has (509 slots max), > so that default deployments would work out of box. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > PS: > Users that would want to lock down vhost could still > use max_mem_regions option to set lower limit, but > I expect it would be minority. I'm not inclined to merge this. Once we change this we can't take it back. It's not a decision to be taken lightly. And memory hotplug users are a minority. Out of these, users with a heavily fragmented PA space due to hotplug abuse are an even smaller minority. > --- > include/uapi/linux/vhost.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h > index 2511954..92657bf 100644 > --- a/include/uapi/linux/vhost.h > +++ b/include/uapi/linux/vhost.h > @@ -140,7 +140,7 @@ struct vhost_memory { > #define VHOST_MEM_MAX_NREGIONS_NONE 0 > /* We support at least as many nregions in VHOST_SET_MEM_TABLE: > * for use on legacy kernels without VHOST_GET_MEM_MAX_NREGIONS support. */ > -#define VHOST_MEM_MAX_NREGIONS_DEFAULT 64 > +#define VHOST_MEM_MAX_NREGIONS_DEFAULT 509 > > /* VHOST_NET specific defines */ > > -- > 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 29 Jul 2015 18:28:26 +0300 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Wed, Jul 29, 2015 at 04:29:23PM +0200, Igor Mammedov wrote: > > although now there is vhost module max_mem_regions option > > to set custom limit it doesn't help for default setups, > > since it requires administrator manually set a higher > > limit on each host. Which complicates servers deployments > > and management. > > Rise limit to the same value as KVM has (509 slots max), > > so that default deployments would work out of box. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > PS: > > Users that would want to lock down vhost could still > > use max_mem_regions option to set lower limit, but > > I expect it would be minority. > > I'm not inclined to merge this. > > Once we change this we can't take it back. It's not a decision > to be taken lightly. considering that continuous HVA idea has failed, why would you want to take limit back in the future if we rise it now? > > And memory hotplug users are a minority. Out of these, users with a > heavily fragmented PA space due to hotplug abuse are an even smaller > minority. > > > --- > > include/uapi/linux/vhost.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h > > index 2511954..92657bf 100644 > > --- a/include/uapi/linux/vhost.h > > +++ b/include/uapi/linux/vhost.h > > @@ -140,7 +140,7 @@ struct vhost_memory { > > #define VHOST_MEM_MAX_NREGIONS_NONE 0 > > /* We support at least as many nregions in VHOST_SET_MEM_TABLE: > > * for use on legacy kernels without VHOST_GET_MEM_MAX_NREGIONS support. */ > > -#define VHOST_MEM_MAX_NREGIONS_DEFAULT 64 > > +#define VHOST_MEM_MAX_NREGIONS_DEFAULT 509 > > > > /* VHOST_NET specific defines */ > > > > -- > > 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 30, 2015 at 08:26:03AM +0200, Igor Mammedov wrote: > On Wed, 29 Jul 2015 18:28:26 +0300 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Wed, Jul 29, 2015 at 04:29:23PM +0200, Igor Mammedov wrote: > > > although now there is vhost module max_mem_regions option > > > to set custom limit it doesn't help for default setups, > > > since it requires administrator manually set a higher > > > limit on each host. Which complicates servers deployments > > > and management. > > > Rise limit to the same value as KVM has (509 slots max), > > > so that default deployments would work out of box. > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > --- > > > PS: > > > Users that would want to lock down vhost could still > > > use max_mem_regions option to set lower limit, but > > > I expect it would be minority. > > > > I'm not inclined to merge this. > > > > Once we change this we can't take it back. It's not a decision > > to be taken lightly. > considering that continuous HVA idea has failed, why would you > want to take limit back in the future if we rise it now? I'm not sure. I think you merely demonstrated it's a big change for userspace - not that it's unfeasible. Alternatively, if we want an unlimited size table, we should keep it in userspace memory. > > > > And memory hotplug users are a minority. Out of these, users with a > > heavily fragmented PA space due to hotplug abuse are an even smaller > > minority. > > > > > --- > > > include/uapi/linux/vhost.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h > > > index 2511954..92657bf 100644 > > > --- a/include/uapi/linux/vhost.h > > > +++ b/include/uapi/linux/vhost.h > > > @@ -140,7 +140,7 @@ struct vhost_memory { > > > #define VHOST_MEM_MAX_NREGIONS_NONE 0 > > > /* We support at least as many nregions in VHOST_SET_MEM_TABLE: > > > * for use on legacy kernels without VHOST_GET_MEM_MAX_NREGIONS support. */ > > > -#define VHOST_MEM_MAX_NREGIONS_DEFAULT 64 > > > +#define VHOST_MEM_MAX_NREGIONS_DEFAULT 509 > > > > > > /* VHOST_NET specific defines */ > > > > > > -- > > > 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 30 Jul 2015 09:33:57 +0300 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Thu, Jul 30, 2015 at 08:26:03AM +0200, Igor Mammedov wrote: > > On Wed, 29 Jul 2015 18:28:26 +0300 > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > On Wed, Jul 29, 2015 at 04:29:23PM +0200, Igor Mammedov wrote: > > > > although now there is vhost module max_mem_regions option > > > > to set custom limit it doesn't help for default setups, > > > > since it requires administrator manually set a higher > > > > limit on each host. Which complicates servers deployments > > > > and management. > > > > Rise limit to the same value as KVM has (509 slots max), > > > > so that default deployments would work out of box. > > > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > > --- > > > > PS: > > > > Users that would want to lock down vhost could still > > > > use max_mem_regions option to set lower limit, but > > > > I expect it would be minority. > > > > > > I'm not inclined to merge this. > > > > > > Once we change this we can't take it back. It's not a decision > > > to be taken lightly. in addition, you already gave out control on limit allowing to rise it via module parameter. Rising default is just a way to reduce pain for users that would try to use more than 64 slots. > > considering that continuous HVA idea has failed, why would you > > want to take limit back in the future if we rise it now? > > I'm not sure. > > I think you merely demonstrated it's a big change for userspace - > not that it's unfeasible. It's not a big change but rather ugly one, being unportable, enforcing unnecessary (and to really reasonable) restrictions on memory backends and changing memory unplug mgmt workflow depending on if HVA used or not. > Alternatively, if we want an unlimited size table, we should keep it > in userspace memory. this patch doesn't propose unlimited table size. with proposed limit we are talking about max order-4 allocations that can fallback to vmalloc if needed. And it makes vhost consistent with KVM's limit, which has similar table. Proposed limit also neatly works around corner cases of existing old userspace that can't deal with it when it hits limit. > > > > > > > And memory hotplug users are a minority. Out of these, users with a > > > heavily fragmented PA space due to hotplug abuse are an even smaller > > > minority. > > > > > > > --- > > > > include/uapi/linux/vhost.h | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h > > > > index 2511954..92657bf 100644 > > > > --- a/include/uapi/linux/vhost.h > > > > +++ b/include/uapi/linux/vhost.h > > > > @@ -140,7 +140,7 @@ struct vhost_memory { > > > > #define VHOST_MEM_MAX_NREGIONS_NONE 0 > > > > /* We support at least as many nregions in VHOST_SET_MEM_TABLE: > > > > * for use on legacy kernels without VHOST_GET_MEM_MAX_NREGIONS support. */ > > > > -#define VHOST_MEM_MAX_NREGIONS_DEFAULT 64 > > > > +#define VHOST_MEM_MAX_NREGIONS_DEFAULT 509 > > > > > > > > /* VHOST_NET specific defines */ > > > > > > > > -- > > > > 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 30 Jul 2015 09:33:57 +0300 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Thu, Jul 30, 2015 at 08:26:03AM +0200, Igor Mammedov wrote: > > On Wed, 29 Jul 2015 18:28:26 +0300 > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > On Wed, Jul 29, 2015 at 04:29:23PM +0200, Igor Mammedov wrote: > > > > although now there is vhost module max_mem_regions option > > > > to set custom limit it doesn't help for default setups, > > > > since it requires administrator manually set a higher > > > > limit on each host. Which complicates servers deployments > > > > and management. > > > > Rise limit to the same value as KVM has (509 slots max), > > > > so that default deployments would work out of box. > > > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > > --- > > > > PS: > > > > Users that would want to lock down vhost could still > > > > use max_mem_regions option to set lower limit, but > > > > I expect it would be minority. > > > > > > I'm not inclined to merge this. > > > > > > Once we change this we can't take it back. It's not a decision > > > to be taken lightly. > > considering that continuous HVA idea has failed, why would you > > want to take limit back in the future if we rise it now? > > I'm not sure. > > I think you merely demonstrated it's a big change for userspace - > not that it's unfeasible. > > Alternatively, if we want an unlimited size table, we should keep it > in userspace memory. btw: if table were a simple array and kernel will do inefficient linear scan to do translation then I guess we could use userspace memory. But I'm afraid we can't trust userspace in case of more elaborate structure. Even if it's just binary search over sorted array, it would be possible for userspace to hung kernel thread in translate_desc() by providing corrupted or wrongly sorted table. And we can't afford table validation on hot path. > > > > > > > And memory hotplug users are a minority. Out of these, users with a > > > heavily fragmented PA space due to hotplug abuse are an even smaller > > > minority. > > > > > > > --- > > > > include/uapi/linux/vhost.h | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h > > > > index 2511954..92657bf 100644 > > > > --- a/include/uapi/linux/vhost.h > > > > +++ b/include/uapi/linux/vhost.h > > > > @@ -140,7 +140,7 @@ struct vhost_memory { > > > > #define VHOST_MEM_MAX_NREGIONS_NONE 0 > > > > /* We support at least as many nregions in VHOST_SET_MEM_TABLE: > > > > * for use on legacy kernels without VHOST_GET_MEM_MAX_NREGIONS support. */ > > > > -#define VHOST_MEM_MAX_NREGIONS_DEFAULT 64 > > > > +#define VHOST_MEM_MAX_NREGIONS_DEFAULT 509 > > > > > > > > /* VHOST_NET specific defines */ > > > > > > > > -- > > > > 1.8.3.1 > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h index 2511954..92657bf 100644 --- a/include/uapi/linux/vhost.h +++ b/include/uapi/linux/vhost.h @@ -140,7 +140,7 @@ struct vhost_memory { #define VHOST_MEM_MAX_NREGIONS_NONE 0 /* We support at least as many nregions in VHOST_SET_MEM_TABLE: * for use on legacy kernels without VHOST_GET_MEM_MAX_NREGIONS support. */ -#define VHOST_MEM_MAX_NREGIONS_DEFAULT 64 +#define VHOST_MEM_MAX_NREGIONS_DEFAULT 509 /* VHOST_NET specific defines */
although now there is vhost module max_mem_regions option to set custom limit it doesn't help for default setups, since it requires administrator manually set a higher limit on each host. Which complicates servers deployments and management. Rise limit to the same value as KVM has (509 slots max), so that default deployments would work out of box. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- PS: Users that would want to lock down vhost could still use max_mem_regions option to set lower limit, but I expect it would be minority. --- include/uapi/linux/vhost.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)