Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1471403 > unrolled thread

[PATCH 0/6] KVM: PPC: e500: Fine-tuning for two function implementations

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 0/6] KVM: PPC: e500: Fine-tuning for two function  implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-28 19:20 +0200
    [PATCH 5/6] KVM: PPC: e500: Use kmalloc_array() in  kvmppc_e500_tlb_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-28 19:20 +0200
      Re: [PATCH 5/6] KVM: PPC: e500: Use kmalloc_array() in  kvmppc_e500_tlb_init() Julia Lawall <julia.lawall@lip6.fr> - 2016-08-28 19:50 +0200
    [PATCH 2/6] KVM: PPC: e500: Less function calls in  kvm_vcpu_ioctl_config_tlb() after error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-28 19:20 +0200
    [PATCH 4/6] KVM: PPC: e500: Replace kzalloc() calls by kcalloc() in  two functions SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-28 19:20 +0200
    [PATCH 1/6] KVM: PPC: e500: Use kmalloc_array() in  kvm_vcpu_ioctl_config_tlb() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-28 19:20 +0200
    [PATCH 3/6] KVM: PPC: e500: Delete an unnecessary initialisation in  kvm_vcpu_ioctl_config_tlb() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-28 19:30 +0200
    [PATCH 6/6] KVM: PPC: e500: Rename jump labels in  kvmppc_e500_tlb_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-28 19:30 +0200
      Re: [PATCH 6/6] KVM: PPC: e500: Rename jump labels in  kvmppc_e500_tlb_init() Julia Lawall <julia.lawall@lip6.fr> - 2016-08-28 19:50 +0200

#1471403 — [PATCH 0/6] KVM: PPC: e500: Fine-tuning for two function implementations

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 28 Aug 2016 19:01:02 +0200

Several update suggestions were taken into account
from static source code analysis.

Markus Elfring (6):
  Use kmalloc_array() in kvm_vcpu_ioctl_config_tlb()
  Less function calls in kvm_vcpu_ioctl_config_tlb() after error detection
  Delete an unnecessary initialisation in kvm_vcpu_ioctl_config_tlb()
  Replace kzalloc() calls by kcalloc() in two functions
  Use kmalloc_array() in kvmppc_e500_tlb_init()
  Rename jump labels in kvmppc_e500_tlb_init()

 arch/powerpc/kvm/e500_mmu.c | 71 +++++++++++++++++++++++----------------------
 1 file changed, 36 insertions(+), 35 deletions(-)

-- 
2.9.3

[toc] | [next] | [standalone]

#1471405 — [PATCH 5/6] KVM: PPC: e500: Use kmalloc_array() in kvmppc_e500_tlb_init()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 28 Aug 2016 18:40:08 +0200

* A multiplication for the size determination of a memory allocation
  indicated that an array data structure should be processed.
  Thus use the corresponding function "kmalloc_array".

* Replace the specification of a data structure by a pointer dereference
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/powerpc/kvm/e500_mmu.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c
index 2be2afc4..0a2eeb1 100644
--- a/arch/powerpc/kvm/e500_mmu.c
+++ b/arch/powerpc/kvm/e500_mmu.c
@@ -905,8 +905,6 @@ static int vcpu_mmu_init(struct kvm_vcpu *vcpu,
 int kvmppc_e500_tlb_init(struct kvmppc_vcpu_e500 *vcpu_e500)
 {
 	struct kvm_vcpu *vcpu = &vcpu_e500->vcpu;
-	int entry_size = sizeof(struct kvm_book3e_206_tlb_entry);
-	int entries = KVM_E500_TLB0_SIZE + KVM_E500_TLB1_SIZE;
 
 	if (e500_mmu_host_init(vcpu_e500))
 		goto err;
@@ -921,7 +919,10 @@ int kvmppc_e500_tlb_init(struct kvmppc_vcpu_e500 *vcpu_e500)
 	vcpu_e500->gtlb_params[1].ways = KVM_E500_TLB1_SIZE;
 	vcpu_e500->gtlb_params[1].sets = 1;
 
-	vcpu_e500->gtlb_arch = kmalloc(entries * entry_size, GFP_KERNEL);
+	vcpu_e500->gtlb_arch = kmalloc_array(KVM_E500_TLB0_SIZE +
+					     KVM_E500_TLB1_SIZE,
+					     sizeof(*vcpu_e500->gtlb_arch),
+					     GFP_KERNEL);
 	if (!vcpu_e500->gtlb_arch)
 		return -ENOMEM;
 
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1471415 — Re: [PATCH 5/6] KVM: PPC: e500: Use kmalloc_array() in kvmppc_e500_tlb_init()

On Sun, 28 Aug 2016, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 28 Aug 2016 18:40:08 +0200
>
> * A multiplication for the size determination of a memory allocation
>   indicated that an array data structure should be processed.
>   Thus use the corresponding function "kmalloc_array".
>
> * Replace the specification of a data structure by a pointer dereference
>   to make the corresponding size determination a bit safer according to
>   the Linux coding style convention.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  arch/powerpc/kvm/e500_mmu.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c
> index 2be2afc4..0a2eeb1 100644
> --- a/arch/powerpc/kvm/e500_mmu.c
> +++ b/arch/powerpc/kvm/e500_mmu.c
> @@ -905,8 +905,6 @@ static int vcpu_mmu_init(struct kvm_vcpu *vcpu,
>  int kvmppc_e500_tlb_init(struct kvmppc_vcpu_e500 *vcpu_e500)
>  {
>  	struct kvm_vcpu *vcpu = &vcpu_e500->vcpu;
> -	int entry_size = sizeof(struct kvm_book3e_206_tlb_entry);
> -	int entries = KVM_E500_TLB0_SIZE + KVM_E500_TLB1_SIZE;
>
>  	if (e500_mmu_host_init(vcpu_e500))
>  		goto err;
> @@ -921,7 +919,10 @@ int kvmppc_e500_tlb_init(struct kvmppc_vcpu_e500 *vcpu_e500)
>  	vcpu_e500->gtlb_params[1].ways = KVM_E500_TLB1_SIZE;
>  	vcpu_e500->gtlb_params[1].sets = 1;
>
> -	vcpu_e500->gtlb_arch = kmalloc(entries * entry_size, GFP_KERNEL);
> +	vcpu_e500->gtlb_arch = kmalloc_array(KVM_E500_TLB0_SIZE +
> +					     KVM_E500_TLB1_SIZE,
> +					     sizeof(*vcpu_e500->gtlb_arch),
> +					     GFP_KERNEL);

There are changes here that are not mentioned in the commit log.

julia

>  	if (!vcpu_e500->gtlb_arch)
>  		return -ENOMEM;
>
> --
> 2.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[toc] | [prev] | [next] | [standalone]

#1471406 — [PATCH 2/6] KVM: PPC: e500: Less function calls in kvm_vcpu_ioctl_config_tlb() after error detection

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 28 Aug 2016 17:34:46 +0200

The kfree() function was called in two cases by the
kvm_vcpu_ioctl_config_tlb() function during error handling
even if the passed data structure element contained a null pointer.

* Split a condition check for memory allocation failures.

* Adjust jump targets according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/powerpc/kvm/e500_mmu.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c
index 26f3737..b65a894 100644
--- a/arch/powerpc/kvm/e500_mmu.c
+++ b/arch/powerpc/kvm/e500_mmu.c
@@ -785,35 +785,39 @@ int kvm_vcpu_ioctl_config_tlb(struct kvm_vcpu *vcpu,
 
 	ret = get_user_pages_fast(cfg->array, num_pages, 1, pages);
 	if (ret < 0)
-		goto err_pages;
+		goto free_pages;
 
 	if (ret != num_pages) {
 		num_pages = ret;
 		ret = -EFAULT;
-		goto err_put_page;
+		goto put_pages;
 	}
 
 	virt = vmap(pages, num_pages, VM_MAP, PAGE_KERNEL);
 	if (!virt) {
 		ret = -ENOMEM;
-		goto err_put_page;
+		goto put_pages;
 	}
 
 	privs[0] = kzalloc(sizeof(struct tlbe_priv) * params.tlb_sizes[0],
 			   GFP_KERNEL);
+	if (!privs[0]) {
+		ret = -ENOMEM;
+		goto put_pages;
+	}
+
 	privs[1] = kzalloc(sizeof(struct tlbe_priv) * params.tlb_sizes[1],
 			   GFP_KERNEL);
-
-	if (!privs[0] || !privs[1]) {
+	if (!privs[1]) {
 		ret = -ENOMEM;
-		goto err_privs;
+		goto free_privs_first;
 	}
 
 	g2h_bitmap = kzalloc(sizeof(u64) * params.tlb_sizes[1],
 	                     GFP_KERNEL);
 	if (!g2h_bitmap) {
 		ret = -ENOMEM;
-		goto err_privs;
+		goto free_privs_second;
 	}
 
 	free_gtlb(vcpu_e500);
@@ -845,16 +849,14 @@ int kvm_vcpu_ioctl_config_tlb(struct kvm_vcpu *vcpu,
 
 	kvmppc_recalc_tlb1map_range(vcpu_e500);
 	return 0;
-
-err_privs:
-	kfree(privs[0]);
+ free_privs_second:
 	kfree(privs[1]);
-
-err_put_page:
+ free_privs_first:
+	kfree(privs[0]);
+ put_pages:
 	for (i = 0; i < num_pages; i++)
 		put_page(pages[i]);
-
-err_pages:
+ free_pages:
 	kfree(pages);
 	return ret;
 }
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1471407 — [PATCH 4/6] KVM: PPC: e500: Replace kzalloc() calls by kcalloc() in two functions

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 28 Aug 2016 18:30:38 +0200

* A multiplication for the size determination of a memory allocation
  indicated that an array data structure should be processed.
  Thus use the corresponding function "kcalloc".

  Suggested-by: Paolo Bonzini <pbonzini@redhat.com>

  This issue was detected also by using the Coccinelle software.

* Replace the specification of data structures by pointer dereferences
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/powerpc/kvm/e500_mmu.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c
index e9c19e9..2be2afc4 100644
--- a/arch/powerpc/kvm/e500_mmu.c
+++ b/arch/powerpc/kvm/e500_mmu.c
@@ -799,22 +799,21 @@ int kvm_vcpu_ioctl_config_tlb(struct kvm_vcpu *vcpu,
 		goto put_pages;
 	}
 
-	privs[0] = kzalloc(sizeof(struct tlbe_priv) * params.tlb_sizes[0],
-			   GFP_KERNEL);
+	privs[0] = kcalloc(params.tlb_sizes[0], sizeof(*privs[0]), GFP_KERNEL);
 	if (!privs[0]) {
 		ret = -ENOMEM;
 		goto put_pages;
 	}
 
-	privs[1] = kzalloc(sizeof(struct tlbe_priv) * params.tlb_sizes[1],
-			   GFP_KERNEL);
+	privs[1] = kcalloc(params.tlb_sizes[1], sizeof(*privs[1]), GFP_KERNEL);
 	if (!privs[1]) {
 		ret = -ENOMEM;
 		goto free_privs_first;
 	}
 
-	g2h_bitmap = kzalloc(sizeof(u64) * params.tlb_sizes[1],
-	                     GFP_KERNEL);
+	g2h_bitmap = kcalloc(params.tlb_sizes[1],
+			     sizeof(*g2h_bitmap),
+			     GFP_KERNEL);
 	if (!g2h_bitmap) {
 		ret = -ENOMEM;
 		goto free_privs_second;
@@ -929,20 +928,20 @@ int kvmppc_e500_tlb_init(struct kvmppc_vcpu_e500 *vcpu_e500)
 	vcpu_e500->gtlb_offset[0] = 0;
 	vcpu_e500->gtlb_offset[1] = KVM_E500_TLB0_SIZE;
 
-	vcpu_e500->gtlb_priv[0] = kzalloc(sizeof(struct tlbe_ref) *
-					  vcpu_e500->gtlb_params[0].entries,
+	vcpu_e500->gtlb_priv[0] = kcalloc(vcpu_e500->gtlb_params[0].entries,
+					  sizeof(struct tlbe_ref),
 					  GFP_KERNEL);
 	if (!vcpu_e500->gtlb_priv[0])
 		goto err;
 
-	vcpu_e500->gtlb_priv[1] = kzalloc(sizeof(struct tlbe_ref) *
-					  vcpu_e500->gtlb_params[1].entries,
+	vcpu_e500->gtlb_priv[1] = kcalloc(vcpu_e500->gtlb_params[1].entries,
+					  sizeof(struct tlbe_ref),
 					  GFP_KERNEL);
 	if (!vcpu_e500->gtlb_priv[1])
 		goto err;
 
-	vcpu_e500->g2h_tlb1_map = kzalloc(sizeof(u64) *
-					  vcpu_e500->gtlb_params[1].entries,
+	vcpu_e500->g2h_tlb1_map = kcalloc(vcpu_e500->gtlb_params[1].entries,
+					  sizeof(*vcpu_e500->g2h_tlb1_map),
 					  GFP_KERNEL);
 	if (!vcpu_e500->g2h_tlb1_map)
 		goto err;
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1471408 — [PATCH 1/6] KVM: PPC: e500: Use kmalloc_array() in kvm_vcpu_ioctl_config_tlb()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 28 Aug 2016 16:30:07 +0200

* A multiplication for the size determination of a memory allocation
  indicated that an array data structure should be processed.
  Thus use the corresponding function "kmalloc_array".

  This issue was detected by using the Coccinelle software.

* Replace the specification of a data type by a pointer dereference
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/powerpc/kvm/e500_mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c
index 29911a0..26f3737 100644
--- a/arch/powerpc/kvm/e500_mmu.c
+++ b/arch/powerpc/kvm/e500_mmu.c
@@ -779,7 +779,7 @@ int kvm_vcpu_ioctl_config_tlb(struct kvm_vcpu *vcpu,
 
 	num_pages = DIV_ROUND_UP(cfg->array + array_len - 1, PAGE_SIZE) -
 		    cfg->array / PAGE_SIZE;
-	pages = kmalloc(sizeof(struct page *) * num_pages, GFP_KERNEL);
+	pages = kmalloc_array(num_pages, sizeof(*pages), GFP_KERNEL);
 	if (!pages)
 		return -ENOMEM;
 
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1471409 — [PATCH 3/6] KVM: PPC: e500: Delete an unnecessary initialisation in kvm_vcpu_ioctl_config_tlb()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 28 Aug 2016 17:37:10 +0200

The local variable "g2h_bitmap" will be set to an appropriate value
a bit later. Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/powerpc/kvm/e500_mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c
index b65a894..e9c19e9 100644
--- a/arch/powerpc/kvm/e500_mmu.c
+++ b/arch/powerpc/kvm/e500_mmu.c
@@ -743,7 +743,7 @@ int kvm_vcpu_ioctl_config_tlb(struct kvm_vcpu *vcpu,
 	char *virt;
 	struct page **pages;
 	struct tlbe_priv *privs[2] = {};
-	u64 *g2h_bitmap = NULL;
+	u64 *g2h_bitmap;
 	size_t array_len;
 	u32 sets;
 	int num_pages, ret, i;
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1471410 — [PATCH 6/6] KVM: PPC: e500: Rename jump labels in kvmppc_e500_tlb_init()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 28 Aug 2016 18:45:26 +0200

Adjust jump labels according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/powerpc/kvm/e500_mmu.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c
index 0a2eeb1..da8f22b 100644
--- a/arch/powerpc/kvm/e500_mmu.c
+++ b/arch/powerpc/kvm/e500_mmu.c
@@ -933,26 +933,25 @@ int kvmppc_e500_tlb_init(struct kvmppc_vcpu_e500 *vcpu_e500)
 					  sizeof(struct tlbe_ref),
 					  GFP_KERNEL);
 	if (!vcpu_e500->gtlb_priv[0])
-		goto err;
+		goto free_vcpu;
 
 	vcpu_e500->gtlb_priv[1] = kcalloc(vcpu_e500->gtlb_params[1].entries,
 					  sizeof(struct tlbe_ref),
 					  GFP_KERNEL);
 	if (!vcpu_e500->gtlb_priv[1])
-		goto err;
+		goto free_vcpu;
 
 	vcpu_e500->g2h_tlb1_map = kcalloc(vcpu_e500->gtlb_params[1].entries,
 					  sizeof(*vcpu_e500->g2h_tlb1_map),
 					  GFP_KERNEL);
 	if (!vcpu_e500->g2h_tlb1_map)
-		goto err;
+		goto free_vcpu;
 
 	vcpu_mmu_init(vcpu, vcpu_e500->gtlb_params);
 
 	kvmppc_recalc_tlb1map_range(vcpu_e500);
 	return 0;
-
-err:
+ free_vcpu:
 	free_gtlb(vcpu_e500);
 	return -1;
 }
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1471414 — Re: [PATCH 6/6] KVM: PPC: e500: Rename jump labels in kvmppc_e500_tlb_init()

On Sun, 28 Aug 2016, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 28 Aug 2016 18:45:26 +0200
>
> Adjust jump labels according to the current Linux coding style convention.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  arch/powerpc/kvm/e500_mmu.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c
> index 0a2eeb1..da8f22b 100644
> --- a/arch/powerpc/kvm/e500_mmu.c
> +++ b/arch/powerpc/kvm/e500_mmu.c
> @@ -933,26 +933,25 @@ int kvmppc_e500_tlb_init(struct kvmppc_vcpu_e500 *vcpu_e500)
>  					  sizeof(struct tlbe_ref),
>  					  GFP_KERNEL);
>  	if (!vcpu_e500->gtlb_priv[0])
> -		goto err;
> +		goto free_vcpu;
>
>  	vcpu_e500->gtlb_priv[1] = kcalloc(vcpu_e500->gtlb_params[1].entries,
>  					  sizeof(struct tlbe_ref),
>  					  GFP_KERNEL);
>  	if (!vcpu_e500->gtlb_priv[1])
> -		goto err;
> +		goto free_vcpu;
>
>  	vcpu_e500->g2h_tlb1_map = kcalloc(vcpu_e500->gtlb_params[1].entries,
>  					  sizeof(*vcpu_e500->g2h_tlb1_map),
>  					  GFP_KERNEL);
>  	if (!vcpu_e500->g2h_tlb1_map)
> -		goto err;
> +		goto free_vcpu;
>
>  	vcpu_mmu_init(vcpu, vcpu_e500->gtlb_params);
>
>  	kvmppc_recalc_tlb1map_range(vcpu_e500);
>  	return 0;
> -
> -err:
> + free_vcpu:
>  	free_gtlb(vcpu_e500);
>  	return -1;

I doubt that -1 is the best return value.  One could guess that it should
be -ENOMEM.  But see what the call sites expect.

julia

>  }
> --
> 2.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web