All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/1] RFC: memcg support for KVM/s390
@ 2019-10-31 15:19 Christian Borntraeger
  2019-10-31 15:19 ` [PATCH 1/1] KVM: s390: Add memcg accounting to KVM allocations Christian Borntraeger
  0 siblings, 1 reply; 5+ messages in thread
From: Christian Borntraeger @ 2019-10-31 15:19 UTC (permalink / raw)
  To: Janosch Frank
  Cc: KVM, Cornelia Huck, Christian Borntraeger, David Hildenbrand,
	linux-s390, Thomas Huth

As a followup to my keynote where I talked about memcg support in
kvm/x86, here is the s390 equivalent.

I think all of those allocations can be really attributed to a specific
host process but a 2nd (or 3rd) pair of eyes is certainly welcome.

Christian Borntraeger (1):
  KVM: s390:  Add memcg accounting to KVM allocations

 arch/s390/kvm/guestdbg.c  |  8 ++++----
 arch/s390/kvm/intercept.c |  2 +-
 arch/s390/kvm/interrupt.c | 12 ++++++------
 arch/s390/kvm/kvm-s390.c  | 22 +++++++++++-----------
 arch/s390/kvm/priv.c      |  4 ++--
 arch/s390/kvm/vsie.c      |  4 ++--
 6 files changed, 26 insertions(+), 26 deletions(-)

-- 
2.21.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/1] KVM: s390:  Add memcg accounting to KVM allocations
  2019-10-31 15:19 [PATCH 0/1] RFC: memcg support for KVM/s390 Christian Borntraeger
@ 2019-10-31 15:19 ` Christian Borntraeger
  2019-10-31 15:22   ` David Hildenbrand
  0 siblings, 1 reply; 5+ messages in thread
From: Christian Borntraeger @ 2019-10-31 15:19 UTC (permalink / raw)
  To: Janosch Frank
  Cc: KVM, Cornelia Huck, Christian Borntraeger, David Hildenbrand,
	linux-s390, Thomas Huth

While I propared my KVM Forum talk about whats new in KVM including
memcg, I realized that the s390 code does not take care of memcg.

As far as I can tell, almost all kvm allocations in the s390x KVM code
can be attributed to process that triggers the allocation (in other
words, no global allocation for other guests). This will help the memcg
controller to do the right decisions.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/kvm/guestdbg.c  |  8 ++++----
 arch/s390/kvm/intercept.c |  2 +-
 arch/s390/kvm/interrupt.c | 12 ++++++------
 arch/s390/kvm/kvm-s390.c  | 22 +++++++++++-----------
 arch/s390/kvm/priv.c      |  4 ++--
 arch/s390/kvm/vsie.c      |  4 ++--
 6 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/arch/s390/kvm/guestdbg.c b/arch/s390/kvm/guestdbg.c
index 394a5f53805b..3765c4223bf9 100644
--- a/arch/s390/kvm/guestdbg.c
+++ b/arch/s390/kvm/guestdbg.c
@@ -184,7 +184,7 @@ static int __import_wp_info(struct kvm_vcpu *vcpu,
 	if (wp_info->len < 0 || wp_info->len > MAX_WP_SIZE)
 		return -EINVAL;
 
-	wp_info->old_data = kmalloc(bp_data->len, GFP_KERNEL);
+	wp_info->old_data = kmalloc(bp_data->len, GFP_KERNEL_ACCOUNT);
 	if (!wp_info->old_data)
 		return -ENOMEM;
 	/* try to backup the original value */
@@ -234,7 +234,7 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu,
 	if (nr_wp > 0) {
 		wp_info = kmalloc_array(nr_wp,
 					sizeof(*wp_info),
-					GFP_KERNEL);
+					GFP_KERNEL_ACCOUNT);
 		if (!wp_info) {
 			ret = -ENOMEM;
 			goto error;
@@ -243,7 +243,7 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu,
 	if (nr_bp > 0) {
 		bp_info = kmalloc_array(nr_bp,
 					sizeof(*bp_info),
-					GFP_KERNEL);
+					GFP_KERNEL_ACCOUNT);
 		if (!bp_info) {
 			ret = -ENOMEM;
 			goto error;
@@ -349,7 +349,7 @@ static struct kvm_hw_wp_info_arch *any_wp_changed(struct kvm_vcpu *vcpu)
 		if (!wp_info || !wp_info->old_data || wp_info->len <= 0)
 			continue;
 
-		temp = kmalloc(wp_info->len, GFP_KERNEL);
+		temp = kmalloc(wp_info->len, GFP_KERNEL_ACCOUNT);
 		if (!temp)
 			continue;
 
diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c
index a389fa85cca2..fb2daae88105 100644
--- a/arch/s390/kvm/intercept.c
+++ b/arch/s390/kvm/intercept.c
@@ -387,7 +387,7 @@ int handle_sthyi(struct kvm_vcpu *vcpu)
 	if (addr & ~PAGE_MASK)
 		return kvm_s390_inject_program_int(vcpu, PGM_SPECIFICATION);
 
-	sctns = (void *)get_zeroed_page(GFP_KERNEL);
+	sctns = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
 	if (!sctns)
 		return -ENOMEM;
 
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 165dea4c7f19..7fe8896a82dd 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -1668,7 +1668,7 @@ struct kvm_s390_interrupt_info *kvm_s390_get_io_int(struct kvm *kvm,
 		goto out;
 	}
 gisa_out:
-	tmp_inti = kzalloc(sizeof(*inti), GFP_KERNEL);
+	tmp_inti = kzalloc(sizeof(*inti), GFP_KERNEL_ACCOUNT);
 	if (tmp_inti) {
 		tmp_inti->type = KVM_S390_INT_IO(1, 0, 0, 0);
 		tmp_inti->io.io_int_word = isc_to_int_word(isc);
@@ -1881,7 +1881,7 @@ int kvm_s390_inject_vm(struct kvm *kvm,
 	struct kvm_s390_interrupt_info *inti;
 	int rc;
 
-	inti = kzalloc(sizeof(*inti), GFP_KERNEL);
+	inti = kzalloc(sizeof(*inti), GFP_KERNEL_ACCOUNT);
 	if (!inti)
 		return -ENOMEM;
 
@@ -2275,7 +2275,7 @@ static int enqueue_floating_irq(struct kvm_device *dev,
 		return -EINVAL;
 
 	while (len >= sizeof(struct kvm_s390_irq)) {
-		inti = kzalloc(sizeof(*inti), GFP_KERNEL);
+		inti = kzalloc(sizeof(*inti), GFP_KERNEL_ACCOUNT);
 		if (!inti)
 			return -ENOMEM;
 
@@ -2323,7 +2323,7 @@ static int register_io_adapter(struct kvm_device *dev,
 	if (dev->kvm->arch.adapters[adapter_info.id] != NULL)
 		return -EINVAL;
 
-	adapter = kzalloc(sizeof(*adapter), GFP_KERNEL);
+	adapter = kzalloc(sizeof(*adapter), GFP_KERNEL_ACCOUNT);
 	if (!adapter)
 		return -ENOMEM;
 
@@ -2363,7 +2363,7 @@ static int kvm_s390_adapter_map(struct kvm *kvm, unsigned int id, __u64 addr)
 	if (!adapter || !addr)
 		return -EINVAL;
 
-	map = kzalloc(sizeof(*map), GFP_KERNEL);
+	map = kzalloc(sizeof(*map), GFP_KERNEL_ACCOUNT);
 	if (!map) {
 		ret = -ENOMEM;
 		goto out;
@@ -3223,7 +3223,7 @@ int kvm_s390_gib_init(u8 nisc)
 		goto out;
 	}
 
-	gib = (struct kvm_s390_gib *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
+	gib = (struct kvm_s390_gib *)get_zeroed_page(GFP_KERNEL_ACCOUNT | GFP_DMA);
 	if (!gib) {
 		rc = -ENOMEM;
 		goto out;
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index d9e6bf3d54f0..373e182fd8e8 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -1243,7 +1243,7 @@ static int kvm_s390_set_processor(struct kvm *kvm, struct kvm_device_attr *attr)
 		ret = -EBUSY;
 		goto out;
 	}
-	proc = kzalloc(sizeof(*proc), GFP_KERNEL);
+	proc = kzalloc(sizeof(*proc), GFP_KERNEL_ACCOUNT);
 	if (!proc) {
 		ret = -ENOMEM;
 		goto out;
@@ -1405,7 +1405,7 @@ static int kvm_s390_get_processor(struct kvm *kvm, struct kvm_device_attr *attr)
 	struct kvm_s390_vm_cpu_processor *proc;
 	int ret = 0;
 
-	proc = kzalloc(sizeof(*proc), GFP_KERNEL);
+	proc = kzalloc(sizeof(*proc), GFP_KERNEL_ACCOUNT);
 	if (!proc) {
 		ret = -ENOMEM;
 		goto out;
@@ -1433,7 +1433,7 @@ static int kvm_s390_get_machine(struct kvm *kvm, struct kvm_device_attr *attr)
 	struct kvm_s390_vm_cpu_machine *mach;
 	int ret = 0;
 
-	mach = kzalloc(sizeof(*mach), GFP_KERNEL);
+	mach = kzalloc(sizeof(*mach), GFP_KERNEL_ACCOUNT);
 	if (!mach) {
 		ret = -ENOMEM;
 		goto out;
@@ -1801,7 +1801,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, struct kvm_s390_skeys *args)
 	if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
 		return -EINVAL;
 
-	keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL);
+	keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL_ACCOUNT);
 	if (!keys)
 		return -ENOMEM;
 
@@ -1846,7 +1846,7 @@ static long kvm_s390_set_skeys(struct kvm *kvm, struct kvm_s390_skeys *args)
 	if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
 		return -EINVAL;
 
-	keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL);
+	keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL_ACCOUNT);
 	if (!keys)
 		return -ENOMEM;
 
@@ -2393,7 +2393,7 @@ static void sca_dispose(struct kvm *kvm)
 
 int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 {
-	gfp_t alloc_flags = GFP_KERNEL;
+	gfp_t alloc_flags = GFP_KERNEL_ACCOUNT;
 	int i, rc;
 	char debug_name[16];
 	static unsigned long sca_offset;
@@ -2438,7 +2438,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 
 	BUILD_BUG_ON(sizeof(struct sie_page2) != 4096);
 	kvm->arch.sie_page2 =
-	     (struct sie_page2 *) get_zeroed_page(GFP_KERNEL | GFP_DMA);
+	     (struct sie_page2 *) get_zeroed_page(GFP_KERNEL_ACCOUNT | GFP_DMA);
 	if (!kvm->arch.sie_page2)
 		goto out_err;
 
@@ -2652,7 +2652,7 @@ static int sca_switch_to_extended(struct kvm *kvm)
 	unsigned int vcpu_idx;
 	u32 scaol, scaoh;
 
-	new_sca = alloc_pages_exact(sizeof(*new_sca), GFP_KERNEL|__GFP_ZERO);
+	new_sca = alloc_pages_exact(sizeof(*new_sca), GFP_KERNEL_ACCOUNT | __GFP_ZERO);
 	if (!new_sca)
 		return -ENOMEM;
 
@@ -2947,7 +2947,7 @@ void kvm_s390_vcpu_unsetup_cmma(struct kvm_vcpu *vcpu)
 
 int kvm_s390_vcpu_setup_cmma(struct kvm_vcpu *vcpu)
 {
-	vcpu->arch.sie_block->cbrlo = get_zeroed_page(GFP_KERNEL);
+	vcpu->arch.sie_block->cbrlo = get_zeroed_page(GFP_KERNEL_ACCOUNT);
 	if (!vcpu->arch.sie_block->cbrlo)
 		return -ENOMEM;
 	return 0;
@@ -3047,12 +3047,12 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
 
 	rc = -ENOMEM;
 
-	vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL);
+	vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL_ACCOUNT);
 	if (!vcpu)
 		goto out;
 
 	BUILD_BUG_ON(sizeof(struct sie_page) != 4096);
-	sie_page = (struct sie_page *) get_zeroed_page(GFP_KERNEL);
+	sie_page = (struct sie_page *) get_zeroed_page(GFP_KERNEL_ACCOUNT);
 	if (!sie_page)
 		goto out_free_cpu;
 
diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
index ed52ffa8d5d4..536fcd599665 100644
--- a/arch/s390/kvm/priv.c
+++ b/arch/s390/kvm/priv.c
@@ -878,7 +878,7 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
 	switch (fc) {
 	case 1: /* same handling for 1 and 2 */
 	case 2:
-		mem = get_zeroed_page(GFP_KERNEL);
+		mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
 		if (!mem)
 			goto out_no_data;
 		if (stsi((void *) mem, fc, sel1, sel2))
@@ -887,7 +887,7 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
 	case 3:
 		if (sel1 != 2 || sel2 != 2)
 			goto out_no_data;
-		mem = get_zeroed_page(GFP_KERNEL);
+		mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
 		if (!mem)
 			goto out_no_data;
 		handle_stsi_3_2_2(vcpu, (void *) mem);
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index 076090f9e666..f55fca8f94f8 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -1236,7 +1236,7 @@ static struct vsie_page *get_vsie_page(struct kvm *kvm, unsigned long addr)
 
 	mutex_lock(&kvm->arch.vsie.mutex);
 	if (kvm->arch.vsie.page_count < nr_vcpus) {
-		page = alloc_page(GFP_KERNEL | __GFP_ZERO | GFP_DMA);
+		page = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO | GFP_DMA);
 		if (!page) {
 			mutex_unlock(&kvm->arch.vsie.mutex);
 			return ERR_PTR(-ENOMEM);
@@ -1338,7 +1338,7 @@ int kvm_s390_handle_vsie(struct kvm_vcpu *vcpu)
 void kvm_s390_vsie_init(struct kvm *kvm)
 {
 	mutex_init(&kvm->arch.vsie.mutex);
-	INIT_RADIX_TREE(&kvm->arch.vsie.addr_to_page, GFP_KERNEL);
+	INIT_RADIX_TREE(&kvm->arch.vsie.addr_to_page, GFP_KERNEL_ACCOUNT);
 }
 
 /* Destroy the vsie data structures. To be called when a vm is destroyed. */
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] KVM: s390: Add memcg accounting to KVM allocations
  2019-10-31 15:19 ` [PATCH 1/1] KVM: s390: Add memcg accounting to KVM allocations Christian Borntraeger
@ 2019-10-31 15:22   ` David Hildenbrand
  2019-10-31 15:27     ` Christian Borntraeger
  0 siblings, 1 reply; 5+ messages in thread
From: David Hildenbrand @ 2019-10-31 15:22 UTC (permalink / raw)
  To: Christian Borntraeger, Janosch Frank
  Cc: KVM, Cornelia Huck, linux-s390, Thomas Huth

On 31.10.19 16:19, Christian Borntraeger wrote:
> While I propared my KVM Forum talk about whats new in KVM including
> memcg, I realized that the s390 code does not take care of memcg.
> 
> As far as I can tell, almost all kvm allocations in the s390x KVM code
> can be attributed to process that triggers the allocation (in other
> words, no global allocation for other guests). This will help the memcg
> controller to do the right decisions.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>   arch/s390/kvm/guestdbg.c  |  8 ++++----
>   arch/s390/kvm/intercept.c |  2 +-
>   arch/s390/kvm/interrupt.c | 12 ++++++------
>   arch/s390/kvm/kvm-s390.c  | 22 +++++++++++-----------
>   arch/s390/kvm/priv.c      |  4 ++--
>   arch/s390/kvm/vsie.c      |  4 ++--
>   6 files changed, 26 insertions(+), 26 deletions(-)
> 
> diff --git a/arch/s390/kvm/guestdbg.c b/arch/s390/kvm/guestdbg.c
> index 394a5f53805b..3765c4223bf9 100644
> --- a/arch/s390/kvm/guestdbg.c
> +++ b/arch/s390/kvm/guestdbg.c
> @@ -184,7 +184,7 @@ static int __import_wp_info(struct kvm_vcpu *vcpu,
>   	if (wp_info->len < 0 || wp_info->len > MAX_WP_SIZE)
>   		return -EINVAL;
>   
> -	wp_info->old_data = kmalloc(bp_data->len, GFP_KERNEL);
> +	wp_info->old_data = kmalloc(bp_data->len, GFP_KERNEL_ACCOUNT);
>   	if (!wp_info->old_data)
>   		return -ENOMEM;
>   	/* try to backup the original value */
> @@ -234,7 +234,7 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu,
>   	if (nr_wp > 0) {
>   		wp_info = kmalloc_array(nr_wp,
>   					sizeof(*wp_info),
> -					GFP_KERNEL);
> +					GFP_KERNEL_ACCOUNT);
>   		if (!wp_info) {
>   			ret = -ENOMEM;
>   			goto error;
> @@ -243,7 +243,7 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu,
>   	if (nr_bp > 0) {
>   		bp_info = kmalloc_array(nr_bp,
>   					sizeof(*bp_info),
> -					GFP_KERNEL);
> +					GFP_KERNEL_ACCOUNT);
>   		if (!bp_info) {
>   			ret = -ENOMEM;
>   			goto error;
> @@ -349,7 +349,7 @@ static struct kvm_hw_wp_info_arch *any_wp_changed(struct kvm_vcpu *vcpu)
>   		if (!wp_info || !wp_info->old_data || wp_info->len <= 0)
>   			continue;
>   
> -		temp = kmalloc(wp_info->len, GFP_KERNEL);
> +		temp = kmalloc(wp_info->len, GFP_KERNEL_ACCOUNT);
>   		if (!temp)
>   			continue;
>   
> diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c
> index a389fa85cca2..fb2daae88105 100644
> --- a/arch/s390/kvm/intercept.c
> +++ b/arch/s390/kvm/intercept.c
> @@ -387,7 +387,7 @@ int handle_sthyi(struct kvm_vcpu *vcpu)
>   	if (addr & ~PAGE_MASK)
>   		return kvm_s390_inject_program_int(vcpu, PGM_SPECIFICATION);
>   
> -	sctns = (void *)get_zeroed_page(GFP_KERNEL);
> +	sctns = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
>   	if (!sctns)
>   		return -ENOMEM;
>   
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 165dea4c7f19..7fe8896a82dd 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -1668,7 +1668,7 @@ struct kvm_s390_interrupt_info *kvm_s390_get_io_int(struct kvm *kvm,
>   		goto out;
>   	}
>   gisa_out:
> -	tmp_inti = kzalloc(sizeof(*inti), GFP_KERNEL);
> +	tmp_inti = kzalloc(sizeof(*inti), GFP_KERNEL_ACCOUNT);
>   	if (tmp_inti) {
>   		tmp_inti->type = KVM_S390_INT_IO(1, 0, 0, 0);
>   		tmp_inti->io.io_int_word = isc_to_int_word(isc);
> @@ -1881,7 +1881,7 @@ int kvm_s390_inject_vm(struct kvm *kvm,
>   	struct kvm_s390_interrupt_info *inti;
>   	int rc;
>   
> -	inti = kzalloc(sizeof(*inti), GFP_KERNEL);
> +	inti = kzalloc(sizeof(*inti), GFP_KERNEL_ACCOUNT);
>   	if (!inti)
>   		return -ENOMEM;
>   
> @@ -2275,7 +2275,7 @@ static int enqueue_floating_irq(struct kvm_device *dev,
>   		return -EINVAL;
>   
>   	while (len >= sizeof(struct kvm_s390_irq)) {
> -		inti = kzalloc(sizeof(*inti), GFP_KERNEL);
> +		inti = kzalloc(sizeof(*inti), GFP_KERNEL_ACCOUNT);
>   		if (!inti)
>   			return -ENOMEM;
>   
> @@ -2323,7 +2323,7 @@ static int register_io_adapter(struct kvm_device *dev,
>   	if (dev->kvm->arch.adapters[adapter_info.id] != NULL)
>   		return -EINVAL;
>   
> -	adapter = kzalloc(sizeof(*adapter), GFP_KERNEL);
> +	adapter = kzalloc(sizeof(*adapter), GFP_KERNEL_ACCOUNT);
>   	if (!adapter)
>   		return -ENOMEM;
>   
> @@ -2363,7 +2363,7 @@ static int kvm_s390_adapter_map(struct kvm *kvm, unsigned int id, __u64 addr)
>   	if (!adapter || !addr)
>   		return -EINVAL;
>   
> -	map = kzalloc(sizeof(*map), GFP_KERNEL);
> +	map = kzalloc(sizeof(*map), GFP_KERNEL_ACCOUNT);
>   	if (!map) {
>   		ret = -ENOMEM;
>   		goto out;
> @@ -3223,7 +3223,7 @@ int kvm_s390_gib_init(u8 nisc)
>   		goto out;
>   	}
>   
> -	gib = (struct kvm_s390_gib *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
> +	gib = (struct kvm_s390_gib *)get_zeroed_page(GFP_KERNEL_ACCOUNT | GFP_DMA);
>   	if (!gib) {
>   		rc = -ENOMEM;
>   		goto out;
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index d9e6bf3d54f0..373e182fd8e8 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -1243,7 +1243,7 @@ static int kvm_s390_set_processor(struct kvm *kvm, struct kvm_device_attr *attr)
>   		ret = -EBUSY;
>   		goto out;
>   	}
> -	proc = kzalloc(sizeof(*proc), GFP_KERNEL);
> +	proc = kzalloc(sizeof(*proc), GFP_KERNEL_ACCOUNT);
>   	if (!proc) {
>   		ret = -ENOMEM;
>   		goto out;
> @@ -1405,7 +1405,7 @@ static int kvm_s390_get_processor(struct kvm *kvm, struct kvm_device_attr *attr)
>   	struct kvm_s390_vm_cpu_processor *proc;
>   	int ret = 0;
>   
> -	proc = kzalloc(sizeof(*proc), GFP_KERNEL);
> +	proc = kzalloc(sizeof(*proc), GFP_KERNEL_ACCOUNT);
>   	if (!proc) {
>   		ret = -ENOMEM;
>   		goto out;
> @@ -1433,7 +1433,7 @@ static int kvm_s390_get_machine(struct kvm *kvm, struct kvm_device_attr *attr)
>   	struct kvm_s390_vm_cpu_machine *mach;
>   	int ret = 0;
>   
> -	mach = kzalloc(sizeof(*mach), GFP_KERNEL);
> +	mach = kzalloc(sizeof(*mach), GFP_KERNEL_ACCOUNT);
>   	if (!mach) {
>   		ret = -ENOMEM;
>   		goto out;
> @@ -1801,7 +1801,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, struct kvm_s390_skeys *args)
>   	if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
>   		return -EINVAL;
>   
> -	keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL);
> +	keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL_ACCOUNT);
>   	if (!keys)
>   		return -ENOMEM;
>   
> @@ -1846,7 +1846,7 @@ static long kvm_s390_set_skeys(struct kvm *kvm, struct kvm_s390_skeys *args)
>   	if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
>   		return -EINVAL;
>   
> -	keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL);
> +	keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL_ACCOUNT);
>   	if (!keys)
>   		return -ENOMEM;
>   
> @@ -2393,7 +2393,7 @@ static void sca_dispose(struct kvm *kvm)
>   
>   int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>   {
> -	gfp_t alloc_flags = GFP_KERNEL;
> +	gfp_t alloc_flags = GFP_KERNEL_ACCOUNT;
>   	int i, rc;
>   	char debug_name[16];
>   	static unsigned long sca_offset;
> @@ -2438,7 +2438,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>   
>   	BUILD_BUG_ON(sizeof(struct sie_page2) != 4096);
>   	kvm->arch.sie_page2 =
> -	     (struct sie_page2 *) get_zeroed_page(GFP_KERNEL | GFP_DMA);
> +	     (struct sie_page2 *) get_zeroed_page(GFP_KERNEL_ACCOUNT | GFP_DMA);
>   	if (!kvm->arch.sie_page2)
>   		goto out_err;
>   
> @@ -2652,7 +2652,7 @@ static int sca_switch_to_extended(struct kvm *kvm)
>   	unsigned int vcpu_idx;
>   	u32 scaol, scaoh;
>   
> -	new_sca = alloc_pages_exact(sizeof(*new_sca), GFP_KERNEL|__GFP_ZERO);
> +	new_sca = alloc_pages_exact(sizeof(*new_sca), GFP_KERNEL_ACCOUNT | __GFP_ZERO);
>   	if (!new_sca)
>   		return -ENOMEM;
>   
> @@ -2947,7 +2947,7 @@ void kvm_s390_vcpu_unsetup_cmma(struct kvm_vcpu *vcpu)
>   
>   int kvm_s390_vcpu_setup_cmma(struct kvm_vcpu *vcpu)
>   {
> -	vcpu->arch.sie_block->cbrlo = get_zeroed_page(GFP_KERNEL);
> +	vcpu->arch.sie_block->cbrlo = get_zeroed_page(GFP_KERNEL_ACCOUNT);
>   	if (!vcpu->arch.sie_block->cbrlo)
>   		return -ENOMEM;
>   	return 0;
> @@ -3047,12 +3047,12 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
>   
>   	rc = -ENOMEM;
>   
> -	vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL);
> +	vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL_ACCOUNT);
>   	if (!vcpu)
>   		goto out;
>   
>   	BUILD_BUG_ON(sizeof(struct sie_page) != 4096);
> -	sie_page = (struct sie_page *) get_zeroed_page(GFP_KERNEL);
> +	sie_page = (struct sie_page *) get_zeroed_page(GFP_KERNEL_ACCOUNT);
>   	if (!sie_page)
>   		goto out_free_cpu;
>   
> diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
> index ed52ffa8d5d4..536fcd599665 100644
> --- a/arch/s390/kvm/priv.c
> +++ b/arch/s390/kvm/priv.c
> @@ -878,7 +878,7 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
>   	switch (fc) {
>   	case 1: /* same handling for 1 and 2 */
>   	case 2:
> -		mem = get_zeroed_page(GFP_KERNEL);
> +		mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
>   		if (!mem)
>   			goto out_no_data;
>   		if (stsi((void *) mem, fc, sel1, sel2))
> @@ -887,7 +887,7 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
>   	case 3:
>   		if (sel1 != 2 || sel2 != 2)
>   			goto out_no_data;
> -		mem = get_zeroed_page(GFP_KERNEL);
> +		mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
>   		if (!mem)
>   			goto out_no_data;
>   		handle_stsi_3_2_2(vcpu, (void *) mem);
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index 076090f9e666..f55fca8f94f8 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
> @@ -1236,7 +1236,7 @@ static struct vsie_page *get_vsie_page(struct kvm *kvm, unsigned long addr)
>   
>   	mutex_lock(&kvm->arch.vsie.mutex);
>   	if (kvm->arch.vsie.page_count < nr_vcpus) {
> -		page = alloc_page(GFP_KERNEL | __GFP_ZERO | GFP_DMA);
> +		page = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO | GFP_DMA);
>   		if (!page) {
>   			mutex_unlock(&kvm->arch.vsie.mutex);
>   			return ERR_PTR(-ENOMEM);
> @@ -1338,7 +1338,7 @@ int kvm_s390_handle_vsie(struct kvm_vcpu *vcpu)
>   void kvm_s390_vsie_init(struct kvm *kvm)
>   {
>   	mutex_init(&kvm->arch.vsie.mutex);
> -	INIT_RADIX_TREE(&kvm->arch.vsie.addr_to_page, GFP_KERNEL);
> +	INIT_RADIX_TREE(&kvm->arch.vsie.addr_to_page, GFP_KERNEL_ACCOUNT);
>   }
>   
>   /* Destroy the vsie data structures. To be called when a vm is destroyed. */
> 

I was wondering about the gmap, especially also page tables for nested 
guests. Did you consider that already?

-- 

Thanks,

David / dhildenb

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] KVM: s390: Add memcg accounting to KVM allocations
  2019-10-31 15:22   ` David Hildenbrand
@ 2019-10-31 15:27     ` Christian Borntraeger
  2019-10-31 15:31       ` David Hildenbrand
  0 siblings, 1 reply; 5+ messages in thread
From: Christian Borntraeger @ 2019-10-31 15:27 UTC (permalink / raw)
  To: David Hildenbrand, Janosch Frank
  Cc: KVM, Cornelia Huck, linux-s390, Thomas Huth



On 31.10.19 16:22, David Hildenbrand wrote:
> On 31.10.19 16:19, Christian Borntraeger wrote:
>> While I propared my KVM Forum talk about whats new in KVM including
>> memcg, I realized that the s390 code does not take care of memcg.
>>
>> As far as I can tell, almost all kvm allocations in the s390x KVM code
>> can be attributed to process that triggers the allocation (in other
>> words, no global allocation for other guests). This will help the memcg
>> controller to do the right decisions.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>   arch/s390/kvm/guestdbg.c  |  8 ++++----
>>   arch/s390/kvm/intercept.c |  2 +-
>>   arch/s390/kvm/interrupt.c | 12 ++++++------
>>   arch/s390/kvm/kvm-s390.c  | 22 +++++++++++-----------
>>   arch/s390/kvm/priv.c      |  4 ++--
>>   arch/s390/kvm/vsie.c      |  4 ++--
>>   6 files changed, 26 insertions(+), 26 deletions(-)
>>
>> diff --git a/arch/s390/kvm/guestdbg.c b/arch/s390/kvm/guestdbg.c
>> index 394a5f53805b..3765c4223bf9 100644
>> --- a/arch/s390/kvm/guestdbg.c
>> +++ b/arch/s390/kvm/guestdbg.c
>> @@ -184,7 +184,7 @@ static int __import_wp_info(struct kvm_vcpu *vcpu,
>>       if (wp_info->len < 0 || wp_info->len > MAX_WP_SIZE)
>>           return -EINVAL;
>>   -    wp_info->old_data = kmalloc(bp_data->len, GFP_KERNEL);
>> +    wp_info->old_data = kmalloc(bp_data->len, GFP_KERNEL_ACCOUNT);
>>       if (!wp_info->old_data)
>>           return -ENOMEM;
>>       /* try to backup the original value */
>> @@ -234,7 +234,7 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu,
>>       if (nr_wp > 0) {
>>           wp_info = kmalloc_array(nr_wp,
>>                       sizeof(*wp_info),
>> -                    GFP_KERNEL);
>> +                    GFP_KERNEL_ACCOUNT);
>>           if (!wp_info) {
>>               ret = -ENOMEM;
>>               goto error;
>> @@ -243,7 +243,7 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu,
>>       if (nr_bp > 0) {
>>           bp_info = kmalloc_array(nr_bp,
>>                       sizeof(*bp_info),
>> -                    GFP_KERNEL);
>> +                    GFP_KERNEL_ACCOUNT);
>>           if (!bp_info) {
>>               ret = -ENOMEM;
>>               goto error;
>> @@ -349,7 +349,7 @@ static struct kvm_hw_wp_info_arch *any_wp_changed(struct kvm_vcpu *vcpu)
>>           if (!wp_info || !wp_info->old_data || wp_info->len <= 0)
>>               continue;
>>   -        temp = kmalloc(wp_info->len, GFP_KERNEL);
>> +        temp = kmalloc(wp_info->len, GFP_KERNEL_ACCOUNT);
>>           if (!temp)
>>               continue;
>>   diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c
>> index a389fa85cca2..fb2daae88105 100644
>> --- a/arch/s390/kvm/intercept.c
>> +++ b/arch/s390/kvm/intercept.c
>> @@ -387,7 +387,7 @@ int handle_sthyi(struct kvm_vcpu *vcpu)
>>       if (addr & ~PAGE_MASK)
>>           return kvm_s390_inject_program_int(vcpu, PGM_SPECIFICATION);
>>   -    sctns = (void *)get_zeroed_page(GFP_KERNEL);
>> +    sctns = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
>>       if (!sctns)
>>           return -ENOMEM;
>>   diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
>> index 165dea4c7f19..7fe8896a82dd 100644
>> --- a/arch/s390/kvm/interrupt.c
>> +++ b/arch/s390/kvm/interrupt.c
>> @@ -1668,7 +1668,7 @@ struct kvm_s390_interrupt_info *kvm_s390_get_io_int(struct kvm *kvm,
>>           goto out;
>>       }
>>   gisa_out:
>> -    tmp_inti = kzalloc(sizeof(*inti), GFP_KERNEL);
>> +    tmp_inti = kzalloc(sizeof(*inti), GFP_KERNEL_ACCOUNT);
>>       if (tmp_inti) {
>>           tmp_inti->type = KVM_S390_INT_IO(1, 0, 0, 0);
>>           tmp_inti->io.io_int_word = isc_to_int_word(isc);
>> @@ -1881,7 +1881,7 @@ int kvm_s390_inject_vm(struct kvm *kvm,
>>       struct kvm_s390_interrupt_info *inti;
>>       int rc;
>>   -    inti = kzalloc(sizeof(*inti), GFP_KERNEL);
>> +    inti = kzalloc(sizeof(*inti), GFP_KERNEL_ACCOUNT);
>>       if (!inti)
>>           return -ENOMEM;
>>   @@ -2275,7 +2275,7 @@ static int enqueue_floating_irq(struct kvm_device *dev,
>>           return -EINVAL;
>>         while (len >= sizeof(struct kvm_s390_irq)) {
>> -        inti = kzalloc(sizeof(*inti), GFP_KERNEL);
>> +        inti = kzalloc(sizeof(*inti), GFP_KERNEL_ACCOUNT);
>>           if (!inti)
>>               return -ENOMEM;
>>   @@ -2323,7 +2323,7 @@ static int register_io_adapter(struct kvm_device *dev,
>>       if (dev->kvm->arch.adapters[adapter_info.id] != NULL)
>>           return -EINVAL;
>>   -    adapter = kzalloc(sizeof(*adapter), GFP_KERNEL);
>> +    adapter = kzalloc(sizeof(*adapter), GFP_KERNEL_ACCOUNT);
>>       if (!adapter)
>>           return -ENOMEM;
>>   @@ -2363,7 +2363,7 @@ static int kvm_s390_adapter_map(struct kvm *kvm, unsigned int id, __u64 addr)
>>       if (!adapter || !addr)
>>           return -EINVAL;
>>   -    map = kzalloc(sizeof(*map), GFP_KERNEL);
>> +    map = kzalloc(sizeof(*map), GFP_KERNEL_ACCOUNT);
>>       if (!map) {
>>           ret = -ENOMEM;
>>           goto out;
>> @@ -3223,7 +3223,7 @@ int kvm_s390_gib_init(u8 nisc)
>>           goto out;
>>       }
>>   -    gib = (struct kvm_s390_gib *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
>> +    gib = (struct kvm_s390_gib *)get_zeroed_page(GFP_KERNEL_ACCOUNT | GFP_DMA);
>>       if (!gib) {
>>           rc = -ENOMEM;
>>           goto out;
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index d9e6bf3d54f0..373e182fd8e8 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -1243,7 +1243,7 @@ static int kvm_s390_set_processor(struct kvm *kvm, struct kvm_device_attr *attr)
>>           ret = -EBUSY;
>>           goto out;
>>       }
>> -    proc = kzalloc(sizeof(*proc), GFP_KERNEL);
>> +    proc = kzalloc(sizeof(*proc), GFP_KERNEL_ACCOUNT);
>>       if (!proc) {
>>           ret = -ENOMEM;
>>           goto out;
>> @@ -1405,7 +1405,7 @@ static int kvm_s390_get_processor(struct kvm *kvm, struct kvm_device_attr *attr)
>>       struct kvm_s390_vm_cpu_processor *proc;
>>       int ret = 0;
>>   -    proc = kzalloc(sizeof(*proc), GFP_KERNEL);
>> +    proc = kzalloc(sizeof(*proc), GFP_KERNEL_ACCOUNT);
>>       if (!proc) {
>>           ret = -ENOMEM;
>>           goto out;
>> @@ -1433,7 +1433,7 @@ static int kvm_s390_get_machine(struct kvm *kvm, struct kvm_device_attr *attr)
>>       struct kvm_s390_vm_cpu_machine *mach;
>>       int ret = 0;
>>   -    mach = kzalloc(sizeof(*mach), GFP_KERNEL);
>> +    mach = kzalloc(sizeof(*mach), GFP_KERNEL_ACCOUNT);
>>       if (!mach) {
>>           ret = -ENOMEM;
>>           goto out;
>> @@ -1801,7 +1801,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, struct kvm_s390_skeys *args)
>>       if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
>>           return -EINVAL;
>>   -    keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL);
>> +    keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL_ACCOUNT);
>>       if (!keys)
>>           return -ENOMEM;
>>   @@ -1846,7 +1846,7 @@ static long kvm_s390_set_skeys(struct kvm *kvm, struct kvm_s390_skeys *args)
>>       if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
>>           return -EINVAL;
>>   -    keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL);
>> +    keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL_ACCOUNT);
>>       if (!keys)
>>           return -ENOMEM;
>>   @@ -2393,7 +2393,7 @@ static void sca_dispose(struct kvm *kvm)
>>     int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>>   {
>> -    gfp_t alloc_flags = GFP_KERNEL;
>> +    gfp_t alloc_flags = GFP_KERNEL_ACCOUNT;
>>       int i, rc;
>>       char debug_name[16];
>>       static unsigned long sca_offset;
>> @@ -2438,7 +2438,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>>         BUILD_BUG_ON(sizeof(struct sie_page2) != 4096);
>>       kvm->arch.sie_page2 =
>> -         (struct sie_page2 *) get_zeroed_page(GFP_KERNEL | GFP_DMA);
>> +         (struct sie_page2 *) get_zeroed_page(GFP_KERNEL_ACCOUNT | GFP_DMA);
>>       if (!kvm->arch.sie_page2)
>>           goto out_err;
>>   @@ -2652,7 +2652,7 @@ static int sca_switch_to_extended(struct kvm *kvm)
>>       unsigned int vcpu_idx;
>>       u32 scaol, scaoh;
>>   -    new_sca = alloc_pages_exact(sizeof(*new_sca), GFP_KERNEL|__GFP_ZERO);
>> +    new_sca = alloc_pages_exact(sizeof(*new_sca), GFP_KERNEL_ACCOUNT | __GFP_ZERO);
>>       if (!new_sca)
>>           return -ENOMEM;
>>   @@ -2947,7 +2947,7 @@ void kvm_s390_vcpu_unsetup_cmma(struct kvm_vcpu *vcpu)
>>     int kvm_s390_vcpu_setup_cmma(struct kvm_vcpu *vcpu)
>>   {
>> -    vcpu->arch.sie_block->cbrlo = get_zeroed_page(GFP_KERNEL);
>> +    vcpu->arch.sie_block->cbrlo = get_zeroed_page(GFP_KERNEL_ACCOUNT);
>>       if (!vcpu->arch.sie_block->cbrlo)
>>           return -ENOMEM;
>>       return 0;
>> @@ -3047,12 +3047,12 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
>>         rc = -ENOMEM;
>>   -    vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL);
>> +    vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL_ACCOUNT);
>>       if (!vcpu)
>>           goto out;
>>         BUILD_BUG_ON(sizeof(struct sie_page) != 4096);
>> -    sie_page = (struct sie_page *) get_zeroed_page(GFP_KERNEL);
>> +    sie_page = (struct sie_page *) get_zeroed_page(GFP_KERNEL_ACCOUNT);
>>       if (!sie_page)
>>           goto out_free_cpu;
>>   diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
>> index ed52ffa8d5d4..536fcd599665 100644
>> --- a/arch/s390/kvm/priv.c
>> +++ b/arch/s390/kvm/priv.c
>> @@ -878,7 +878,7 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
>>       switch (fc) {
>>       case 1: /* same handling for 1 and 2 */
>>       case 2:
>> -        mem = get_zeroed_page(GFP_KERNEL);
>> +        mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
>>           if (!mem)
>>               goto out_no_data;
>>           if (stsi((void *) mem, fc, sel1, sel2))
>> @@ -887,7 +887,7 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
>>       case 3:
>>           if (sel1 != 2 || sel2 != 2)
>>               goto out_no_data;
>> -        mem = get_zeroed_page(GFP_KERNEL);
>> +        mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
>>           if (!mem)
>>               goto out_no_data;
>>           handle_stsi_3_2_2(vcpu, (void *) mem);
>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
>> index 076090f9e666..f55fca8f94f8 100644
>> --- a/arch/s390/kvm/vsie.c
>> +++ b/arch/s390/kvm/vsie.c
>> @@ -1236,7 +1236,7 @@ static struct vsie_page *get_vsie_page(struct kvm *kvm, unsigned long addr)
>>         mutex_lock(&kvm->arch.vsie.mutex);
>>       if (kvm->arch.vsie.page_count < nr_vcpus) {
>> -        page = alloc_page(GFP_KERNEL | __GFP_ZERO | GFP_DMA);
>> +        page = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO | GFP_DMA);
>>           if (!page) {
>>               mutex_unlock(&kvm->arch.vsie.mutex);
>>               return ERR_PTR(-ENOMEM);
>> @@ -1338,7 +1338,7 @@ int kvm_s390_handle_vsie(struct kvm_vcpu *vcpu)
>>   void kvm_s390_vsie_init(struct kvm *kvm)
>>   {
>>       mutex_init(&kvm->arch.vsie.mutex);
>> -    INIT_RADIX_TREE(&kvm->arch.vsie.addr_to_page, GFP_KERNEL);
>> +    INIT_RADIX_TREE(&kvm->arch.vsie.addr_to_page, GFP_KERNEL_ACCOUNT);
>>   }
>>     /* Destroy the vsie data structures. To be called when a vm is destroyed. */
>>
> 
> I was wondering about the gmap, especially also page tables for nested guests. Did you consider that already?

No not yet. gmap would be an extra patch. I then also have to be careful if there  are
some data structures that are shared between different guests. I think not, but I have 
not yet looked completely through that code.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] KVM: s390: Add memcg accounting to KVM allocations
  2019-10-31 15:27     ` Christian Borntraeger
@ 2019-10-31 15:31       ` David Hildenbrand
  0 siblings, 0 replies; 5+ messages in thread
From: David Hildenbrand @ 2019-10-31 15:31 UTC (permalink / raw)
  To: Christian Borntraeger, Janosch Frank
  Cc: KVM, Cornelia Huck, linux-s390, Thomas Huth

On 31.10.19 16:27, Christian Borntraeger wrote:
> 
> 
> On 31.10.19 16:22, David Hildenbrand wrote:
>> On 31.10.19 16:19, Christian Borntraeger wrote:
>>> While I propared my KVM Forum talk about whats new in KVM including
>>> memcg, I realized that the s390 code does not take care of memcg.
>>>
>>> As far as I can tell, almost all kvm allocations in the s390x KVM code
>>> can be attributed to process that triggers the allocation (in other
>>> words, no global allocation for other guests). This will help the memcg
>>> controller to do the right decisions.
>>>
>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> ---
>>>    arch/s390/kvm/guestdbg.c  |  8 ++++----
>>>    arch/s390/kvm/intercept.c |  2 +-
>>>    arch/s390/kvm/interrupt.c | 12 ++++++------
>>>    arch/s390/kvm/kvm-s390.c  | 22 +++++++++++-----------
>>>    arch/s390/kvm/priv.c      |  4 ++--
>>>    arch/s390/kvm/vsie.c      |  4 ++--
>>>    6 files changed, 26 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/arch/s390/kvm/guestdbg.c b/arch/s390/kvm/guestdbg.c
>>> index 394a5f53805b..3765c4223bf9 100644
>>> --- a/arch/s390/kvm/guestdbg.c
>>> +++ b/arch/s390/kvm/guestdbg.c
>>> @@ -184,7 +184,7 @@ static int __import_wp_info(struct kvm_vcpu *vcpu,
>>>        if (wp_info->len < 0 || wp_info->len > MAX_WP_SIZE)
>>>            return -EINVAL;
>>>    -    wp_info->old_data = kmalloc(bp_data->len, GFP_KERNEL);
>>> +    wp_info->old_data = kmalloc(bp_data->len, GFP_KERNEL_ACCOUNT);
>>>        if (!wp_info->old_data)
>>>            return -ENOMEM;
>>>        /* try to backup the original value */
>>> @@ -234,7 +234,7 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu,
>>>        if (nr_wp > 0) {
>>>            wp_info = kmalloc_array(nr_wp,
>>>                        sizeof(*wp_info),
>>> -                    GFP_KERNEL);
>>> +                    GFP_KERNEL_ACCOUNT);
>>>            if (!wp_info) {
>>>                ret = -ENOMEM;
>>>                goto error;
>>> @@ -243,7 +243,7 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu,
>>>        if (nr_bp > 0) {
>>>            bp_info = kmalloc_array(nr_bp,
>>>                        sizeof(*bp_info),
>>> -                    GFP_KERNEL);
>>> +                    GFP_KERNEL_ACCOUNT);
>>>            if (!bp_info) {
>>>                ret = -ENOMEM;
>>>                goto error;
>>> @@ -349,7 +349,7 @@ static struct kvm_hw_wp_info_arch *any_wp_changed(struct kvm_vcpu *vcpu)
>>>            if (!wp_info || !wp_info->old_data || wp_info->len <= 0)
>>>                continue;
>>>    -        temp = kmalloc(wp_info->len, GFP_KERNEL);
>>> +        temp = kmalloc(wp_info->len, GFP_KERNEL_ACCOUNT);
>>>            if (!temp)
>>>                continue;
>>>    diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c
>>> index a389fa85cca2..fb2daae88105 100644
>>> --- a/arch/s390/kvm/intercept.c
>>> +++ b/arch/s390/kvm/intercept.c
>>> @@ -387,7 +387,7 @@ int handle_sthyi(struct kvm_vcpu *vcpu)
>>>        if (addr & ~PAGE_MASK)
>>>            return kvm_s390_inject_program_int(vcpu, PGM_SPECIFICATION);
>>>    -    sctns = (void *)get_zeroed_page(GFP_KERNEL);
>>> +    sctns = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
>>>        if (!sctns)
>>>            return -ENOMEM;
>>>    diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
>>> index 165dea4c7f19..7fe8896a82dd 100644
>>> --- a/arch/s390/kvm/interrupt.c
>>> +++ b/arch/s390/kvm/interrupt.c
>>> @@ -1668,7 +1668,7 @@ struct kvm_s390_interrupt_info *kvm_s390_get_io_int(struct kvm *kvm,
>>>            goto out;
>>>        }
>>>    gisa_out:
>>> -    tmp_inti = kzalloc(sizeof(*inti), GFP_KERNEL);
>>> +    tmp_inti = kzalloc(sizeof(*inti), GFP_KERNEL_ACCOUNT);
>>>        if (tmp_inti) {
>>>            tmp_inti->type = KVM_S390_INT_IO(1, 0, 0, 0);
>>>            tmp_inti->io.io_int_word = isc_to_int_word(isc);
>>> @@ -1881,7 +1881,7 @@ int kvm_s390_inject_vm(struct kvm *kvm,
>>>        struct kvm_s390_interrupt_info *inti;
>>>        int rc;
>>>    -    inti = kzalloc(sizeof(*inti), GFP_KERNEL);
>>> +    inti = kzalloc(sizeof(*inti), GFP_KERNEL_ACCOUNT);
>>>        if (!inti)
>>>            return -ENOMEM;
>>>    @@ -2275,7 +2275,7 @@ static int enqueue_floating_irq(struct kvm_device *dev,
>>>            return -EINVAL;
>>>          while (len >= sizeof(struct kvm_s390_irq)) {
>>> -        inti = kzalloc(sizeof(*inti), GFP_KERNEL);
>>> +        inti = kzalloc(sizeof(*inti), GFP_KERNEL_ACCOUNT);
>>>            if (!inti)
>>>                return -ENOMEM;
>>>    @@ -2323,7 +2323,7 @@ static int register_io_adapter(struct kvm_device *dev,
>>>        if (dev->kvm->arch.adapters[adapter_info.id] != NULL)
>>>            return -EINVAL;
>>>    -    adapter = kzalloc(sizeof(*adapter), GFP_KERNEL);
>>> +    adapter = kzalloc(sizeof(*adapter), GFP_KERNEL_ACCOUNT);
>>>        if (!adapter)
>>>            return -ENOMEM;
>>>    @@ -2363,7 +2363,7 @@ static int kvm_s390_adapter_map(struct kvm *kvm, unsigned int id, __u64 addr)
>>>        if (!adapter || !addr)
>>>            return -EINVAL;
>>>    -    map = kzalloc(sizeof(*map), GFP_KERNEL);
>>> +    map = kzalloc(sizeof(*map), GFP_KERNEL_ACCOUNT);
>>>        if (!map) {
>>>            ret = -ENOMEM;
>>>            goto out;
>>> @@ -3223,7 +3223,7 @@ int kvm_s390_gib_init(u8 nisc)
>>>            goto out;
>>>        }
>>>    -    gib = (struct kvm_s390_gib *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
>>> +    gib = (struct kvm_s390_gib *)get_zeroed_page(GFP_KERNEL_ACCOUNT | GFP_DMA);
>>>        if (!gib) {
>>>            rc = -ENOMEM;
>>>            goto out;
>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>> index d9e6bf3d54f0..373e182fd8e8 100644
>>> --- a/arch/s390/kvm/kvm-s390.c
>>> +++ b/arch/s390/kvm/kvm-s390.c
>>> @@ -1243,7 +1243,7 @@ static int kvm_s390_set_processor(struct kvm *kvm, struct kvm_device_attr *attr)
>>>            ret = -EBUSY;
>>>            goto out;
>>>        }
>>> -    proc = kzalloc(sizeof(*proc), GFP_KERNEL);
>>> +    proc = kzalloc(sizeof(*proc), GFP_KERNEL_ACCOUNT);
>>>        if (!proc) {
>>>            ret = -ENOMEM;
>>>            goto out;
>>> @@ -1405,7 +1405,7 @@ static int kvm_s390_get_processor(struct kvm *kvm, struct kvm_device_attr *attr)
>>>        struct kvm_s390_vm_cpu_processor *proc;
>>>        int ret = 0;
>>>    -    proc = kzalloc(sizeof(*proc), GFP_KERNEL);
>>> +    proc = kzalloc(sizeof(*proc), GFP_KERNEL_ACCOUNT);
>>>        if (!proc) {
>>>            ret = -ENOMEM;
>>>            goto out;
>>> @@ -1433,7 +1433,7 @@ static int kvm_s390_get_machine(struct kvm *kvm, struct kvm_device_attr *attr)
>>>        struct kvm_s390_vm_cpu_machine *mach;
>>>        int ret = 0;
>>>    -    mach = kzalloc(sizeof(*mach), GFP_KERNEL);
>>> +    mach = kzalloc(sizeof(*mach), GFP_KERNEL_ACCOUNT);
>>>        if (!mach) {
>>>            ret = -ENOMEM;
>>>            goto out;
>>> @@ -1801,7 +1801,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, struct kvm_s390_skeys *args)
>>>        if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
>>>            return -EINVAL;
>>>    -    keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL);
>>> +    keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL_ACCOUNT);
>>>        if (!keys)
>>>            return -ENOMEM;
>>>    @@ -1846,7 +1846,7 @@ static long kvm_s390_set_skeys(struct kvm *kvm, struct kvm_s390_skeys *args)
>>>        if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
>>>            return -EINVAL;
>>>    -    keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL);
>>> +    keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL_ACCOUNT);
>>>        if (!keys)
>>>            return -ENOMEM;
>>>    @@ -2393,7 +2393,7 @@ static void sca_dispose(struct kvm *kvm)
>>>      int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>>>    {
>>> -    gfp_t alloc_flags = GFP_KERNEL;
>>> +    gfp_t alloc_flags = GFP_KERNEL_ACCOUNT;
>>>        int i, rc;
>>>        char debug_name[16];
>>>        static unsigned long sca_offset;
>>> @@ -2438,7 +2438,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>>>          BUILD_BUG_ON(sizeof(struct sie_page2) != 4096);
>>>        kvm->arch.sie_page2 =
>>> -         (struct sie_page2 *) get_zeroed_page(GFP_KERNEL | GFP_DMA);
>>> +         (struct sie_page2 *) get_zeroed_page(GFP_KERNEL_ACCOUNT | GFP_DMA);
>>>        if (!kvm->arch.sie_page2)
>>>            goto out_err;
>>>    @@ -2652,7 +2652,7 @@ static int sca_switch_to_extended(struct kvm *kvm)
>>>        unsigned int vcpu_idx;
>>>        u32 scaol, scaoh;
>>>    -    new_sca = alloc_pages_exact(sizeof(*new_sca), GFP_KERNEL|__GFP_ZERO);
>>> +    new_sca = alloc_pages_exact(sizeof(*new_sca), GFP_KERNEL_ACCOUNT | __GFP_ZERO);
>>>        if (!new_sca)
>>>            return -ENOMEM;
>>>    @@ -2947,7 +2947,7 @@ void kvm_s390_vcpu_unsetup_cmma(struct kvm_vcpu *vcpu)
>>>      int kvm_s390_vcpu_setup_cmma(struct kvm_vcpu *vcpu)
>>>    {
>>> -    vcpu->arch.sie_block->cbrlo = get_zeroed_page(GFP_KERNEL);
>>> +    vcpu->arch.sie_block->cbrlo = get_zeroed_page(GFP_KERNEL_ACCOUNT);
>>>        if (!vcpu->arch.sie_block->cbrlo)
>>>            return -ENOMEM;
>>>        return 0;
>>> @@ -3047,12 +3047,12 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
>>>          rc = -ENOMEM;
>>>    -    vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL);
>>> +    vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL_ACCOUNT);
>>>        if (!vcpu)
>>>            goto out;
>>>          BUILD_BUG_ON(sizeof(struct sie_page) != 4096);
>>> -    sie_page = (struct sie_page *) get_zeroed_page(GFP_KERNEL);
>>> +    sie_page = (struct sie_page *) get_zeroed_page(GFP_KERNEL_ACCOUNT);
>>>        if (!sie_page)
>>>            goto out_free_cpu;
>>>    diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
>>> index ed52ffa8d5d4..536fcd599665 100644
>>> --- a/arch/s390/kvm/priv.c
>>> +++ b/arch/s390/kvm/priv.c
>>> @@ -878,7 +878,7 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
>>>        switch (fc) {
>>>        case 1: /* same handling for 1 and 2 */
>>>        case 2:
>>> -        mem = get_zeroed_page(GFP_KERNEL);
>>> +        mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
>>>            if (!mem)
>>>                goto out_no_data;
>>>            if (stsi((void *) mem, fc, sel1, sel2))
>>> @@ -887,7 +887,7 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
>>>        case 3:
>>>            if (sel1 != 2 || sel2 != 2)
>>>                goto out_no_data;
>>> -        mem = get_zeroed_page(GFP_KERNEL);
>>> +        mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
>>>            if (!mem)
>>>                goto out_no_data;
>>>            handle_stsi_3_2_2(vcpu, (void *) mem);
>>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
>>> index 076090f9e666..f55fca8f94f8 100644
>>> --- a/arch/s390/kvm/vsie.c
>>> +++ b/arch/s390/kvm/vsie.c
>>> @@ -1236,7 +1236,7 @@ static struct vsie_page *get_vsie_page(struct kvm *kvm, unsigned long addr)
>>>          mutex_lock(&kvm->arch.vsie.mutex);
>>>        if (kvm->arch.vsie.page_count < nr_vcpus) {
>>> -        page = alloc_page(GFP_KERNEL | __GFP_ZERO | GFP_DMA);
>>> +        page = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO | GFP_DMA);
>>>            if (!page) {
>>>                mutex_unlock(&kvm->arch.vsie.mutex);
>>>                return ERR_PTR(-ENOMEM);
>>> @@ -1338,7 +1338,7 @@ int kvm_s390_handle_vsie(struct kvm_vcpu *vcpu)
>>>    void kvm_s390_vsie_init(struct kvm *kvm)
>>>    {
>>>        mutex_init(&kvm->arch.vsie.mutex);
>>> -    INIT_RADIX_TREE(&kvm->arch.vsie.addr_to_page, GFP_KERNEL);
>>> +    INIT_RADIX_TREE(&kvm->arch.vsie.addr_to_page, GFP_KERNEL_ACCOUNT);
>>>    }
>>>      /* Destroy the vsie data structures. To be called when a vm is destroyed. */
>>>
>>
>> I was wondering about the gmap, especially also page tables for nested guests. Did you consider that already?
> 
> No not yet. gmap would be an extra patch. I then also have to be careful if there  are
> some data structures that are shared between different guests. I think not, but I have
> not yet looked completely through that code.

Everything that is shared should apply to the user page tables only. But 
all datastructures we allocate in the gmap (especially tables, radix 
tress ...) should be for this very process only. At least that's what I 
hope :D

Agreed that this should go to a separate patch.

-- 

Thanks,

David / dhildenb

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-10-31 15:31 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-31 15:19 [PATCH 0/1] RFC: memcg support for KVM/s390 Christian Borntraeger
2019-10-31 15:19 ` [PATCH 1/1] KVM: s390: Add memcg accounting to KVM allocations Christian Borntraeger
2019-10-31 15:22   ` David Hildenbrand
2019-10-31 15:27     ` Christian Borntraeger
2019-10-31 15:31       ` David Hildenbrand

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.