Re: [PATCH 3/4] perf/x86/cqm: One check and another variable less in intel_mbm_init()

2016-09-08 Thread Peter Zijlstra 
On Sun, Sep 04, 2016 at 07:58:32PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Sun, 4 Sep 2016 19:06:33 +0200
> 
> 1. Adjust a jump target to eliminate an extra check at the end.
> 
> 2. Move a jump label according to the current Linux coding style convention.

WTF does that even mean?

> 
> 3. Return only constant values for the success or failure indication.
> 
> 4. Delete the local variable "ret" which became unnecessary with
>this refactoring.

Also, 4 things should be 4 patches if anything.

> 
> Signed-off-by: Markus Elfring 
> ---
>  arch/x86/events/intel/cqm.c | 25 ++---
>  1 file changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/events/intel/cqm.c b/arch/x86/events/intel/cqm.c
> index 29f756e..304caf53 100644
> --- a/arch/x86/events/intel/cqm.c
> +++ b/arch/x86/events/intel/cqm.c
> @@ -1637,7 +1637,7 @@ static const struct x86_cpu_id intel_mbm_total_match[] 
> = {
>  
>  static int intel_mbm_init(void)
>  {
> - int ret = 0, maxid = cqm_max_rmid + 1;
> + int const maxid = cqm_max_rmid + 1;
>  
>   mbm_socket_max = topology_max_packages();
>   mbm_local = kmalloc_array(maxid * mbm_socket_max,
> @@ -1649,25 +1649,20 @@ static int intel_mbm_init(void)
>   mbm_total = kmalloc_array(maxid * mbm_socket_max,
> sizeof(*mbm_total),
> GFP_KERNEL);
> - if (!mbm_total) {
> - ret = -ENOMEM;
> - goto out;
> - }
> + if (!mbm_total)
> + goto free_mbm;
>  
>   mbm_timers = kmalloc_array(mbm_socket_max,
>  sizeof(*mbm_timers),
>  GFP_KERNEL);
> - if (!mbm_timers) {
> - ret = -ENOMEM;
> - goto out;
> - }
> - mbm_hrtimer_init();
> + if (!mbm_timers)
> + goto free_mbm;
>  
> -out:
> - if (ret)
> - mbm_cleanup();
> -
> - return ret;
> + mbm_hrtimer_init();
> + return 0;
> + free_mbm:

No!! no stupid indented labels.

> + mbm_cleanup();
> + return -ENOMEM;
>  }

Re: [PATCH 3/4] perf/x86/cqm: One check and another variable less in intel_mbm_init()

2016-09-08 Thread Peter Zijlstra 
On Sun, Sep 04, 2016 at 07:58:32PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Sun, 4 Sep 2016 19:06:33 +0200
> 
> 1. Adjust a jump target to eliminate an extra check at the end.
> 
> 2. Move a jump label according to the current Linux coding style convention.

WTF does that even mean?

> 
> 3. Return only constant values for the success or failure indication.
> 
> 4. Delete the local variable "ret" which became unnecessary with
>this refactoring.

Also, 4 things should be 4 patches if anything.

> 
> Signed-off-by: Markus Elfring 
> ---
>  arch/x86/events/intel/cqm.c | 25 ++---
>  1 file changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/events/intel/cqm.c b/arch/x86/events/intel/cqm.c
> index 29f756e..304caf53 100644
> --- a/arch/x86/events/intel/cqm.c
> +++ b/arch/x86/events/intel/cqm.c
> @@ -1637,7 +1637,7 @@ static const struct x86_cpu_id intel_mbm_total_match[] 
> = {
>  
>  static int intel_mbm_init(void)
>  {
> - int ret = 0, maxid = cqm_max_rmid + 1;
> + int const maxid = cqm_max_rmid + 1;
>  
>   mbm_socket_max = topology_max_packages();
>   mbm_local = kmalloc_array(maxid * mbm_socket_max,
> @@ -1649,25 +1649,20 @@ static int intel_mbm_init(void)
>   mbm_total = kmalloc_array(maxid * mbm_socket_max,
> sizeof(*mbm_total),
> GFP_KERNEL);
> - if (!mbm_total) {
> - ret = -ENOMEM;
> - goto out;
> - }
> + if (!mbm_total)
> + goto free_mbm;
>  
>   mbm_timers = kmalloc_array(mbm_socket_max,
>  sizeof(*mbm_timers),
>  GFP_KERNEL);
> - if (!mbm_timers) {
> - ret = -ENOMEM;
> - goto out;
> - }
> - mbm_hrtimer_init();
> + if (!mbm_timers)
> + goto free_mbm;
>  
> -out:
> - if (ret)
> - mbm_cleanup();
> -
> - return ret;
> + mbm_hrtimer_init();
> + return 0;
> + free_mbm:

No!! no stupid indented labels.

> + mbm_cleanup();
> + return -ENOMEM;
>  }

Re: [PATCH 3/4] perf/x86/cqm: One check and another variable less in intel_mbm_init()

2016-09-05 Thread Peter Zijlstra 
On Sun, Sep 04, 2016 at 07:58:32PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Sun, 4 Sep 2016 19:06:33 +0200
> 
> 1. Adjust a jump target to eliminate an extra check at the end.
> 
> 2. Move a jump label according to the current Linux coding style convention.

WTF does that even mean?

> 
> 3. Return only constant values for the success or failure indication.
> 
> 4. Delete the local variable "ret" which became unnecessary with
>this refactoring.

Also, 4 things should be 4 patches if anything.

> 
> Signed-off-by: Markus Elfring 
> ---
>  arch/x86/events/intel/cqm.c | 25 ++---
>  1 file changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/events/intel/cqm.c b/arch/x86/events/intel/cqm.c
> index 29f756e..304caf53 100644
> --- a/arch/x86/events/intel/cqm.c
> +++ b/arch/x86/events/intel/cqm.c
> @@ -1637,7 +1637,7 @@ static const struct x86_cpu_id intel_mbm_total_match[] 
> = {
>  
>  static int intel_mbm_init(void)
>  {
> - int ret = 0, maxid = cqm_max_rmid + 1;
> + int const maxid = cqm_max_rmid + 1;
>  
>   mbm_socket_max = topology_max_packages();
>   mbm_local = kmalloc_array(maxid * mbm_socket_max,
> @@ -1649,25 +1649,20 @@ static int intel_mbm_init(void)
>   mbm_total = kmalloc_array(maxid * mbm_socket_max,
> sizeof(*mbm_total),
> GFP_KERNEL);
> - if (!mbm_total) {
> - ret = -ENOMEM;
> - goto out;
> - }
> + if (!mbm_total)
> + goto free_mbm;
>  
>   mbm_timers = kmalloc_array(mbm_socket_max,
>  sizeof(*mbm_timers),
>  GFP_KERNEL);
> - if (!mbm_timers) {
> - ret = -ENOMEM;
> - goto out;
> - }
> - mbm_hrtimer_init();
> + if (!mbm_timers)
> + goto free_mbm;
>  
> -out:
> - if (ret)
> - mbm_cleanup();
> -
> - return ret;
> + mbm_hrtimer_init();
> + return 0;
> + free_mbm:

No!! no stupid indented labels.

> + mbm_cleanup();
> + return -ENOMEM;
>  }

Re: [PATCH 3/4] perf/x86/cqm: One check and another variable less in intel_mbm_init()

2016-09-05 Thread Peter Zijlstra 
On Sun, Sep 04, 2016 at 07:58:32PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Sun, 4 Sep 2016 19:06:33 +0200
> 
> 1. Adjust a jump target to eliminate an extra check at the end.
> 
> 2. Move a jump label according to the current Linux coding style convention.

WTF does that even mean?

> 
> 3. Return only constant values for the success or failure indication.
> 
> 4. Delete the local variable "ret" which became unnecessary with
>this refactoring.

Also, 4 things should be 4 patches if anything.

> 
> Signed-off-by: Markus Elfring 
> ---
>  arch/x86/events/intel/cqm.c | 25 ++---
>  1 file changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/events/intel/cqm.c b/arch/x86/events/intel/cqm.c
> index 29f756e..304caf53 100644
> --- a/arch/x86/events/intel/cqm.c
> +++ b/arch/x86/events/intel/cqm.c
> @@ -1637,7 +1637,7 @@ static const struct x86_cpu_id intel_mbm_total_match[] 
> = {
>  
>  static int intel_mbm_init(void)
>  {
> - int ret = 0, maxid = cqm_max_rmid + 1;
> + int const maxid = cqm_max_rmid + 1;
>  
>   mbm_socket_max = topology_max_packages();
>   mbm_local = kmalloc_array(maxid * mbm_socket_max,
> @@ -1649,25 +1649,20 @@ static int intel_mbm_init(void)
>   mbm_total = kmalloc_array(maxid * mbm_socket_max,
> sizeof(*mbm_total),
> GFP_KERNEL);
> - if (!mbm_total) {
> - ret = -ENOMEM;
> - goto out;
> - }
> + if (!mbm_total)
> + goto free_mbm;
>  
>   mbm_timers = kmalloc_array(mbm_socket_max,
>  sizeof(*mbm_timers),
>  GFP_KERNEL);
> - if (!mbm_timers) {
> - ret = -ENOMEM;
> - goto out;
> - }
> - mbm_hrtimer_init();
> + if (!mbm_timers)
> + goto free_mbm;
>  
> -out:
> - if (ret)
> - mbm_cleanup();
> -
> - return ret;
> + mbm_hrtimer_init();
> + return 0;
> + free_mbm:

No!! no stupid indented labels.

> + mbm_cleanup();
> + return -ENOMEM;
>  }

[PATCH 3/4] perf/x86/cqm: One check and another variable less in intel_mbm_init()

2016-09-04 Thread SF Markus Elfring 
From: Markus Elfring 
Date: Sun, 4 Sep 2016 19:06:33 +0200

1. Adjust a jump target to eliminate an extra check at the end.

2. Move a jump label according to the current Linux coding style convention.

3. Return only constant values for the success or failure indication.

4. Delete the local variable "ret" which became unnecessary with
   this refactoring.

Signed-off-by: Markus Elfring 
---
 arch/x86/events/intel/cqm.c | 25 ++---
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/arch/x86/events/intel/cqm.c b/arch/x86/events/intel/cqm.c
index 29f756e..304caf53 100644
--- a/arch/x86/events/intel/cqm.c
+++ b/arch/x86/events/intel/cqm.c
@@ -1637,7 +1637,7 @@ static const struct x86_cpu_id intel_mbm_total_match[] = {
 
 static int intel_mbm_init(void)
 {
-   int ret = 0, maxid = cqm_max_rmid + 1;
+   int const maxid = cqm_max_rmid + 1;
 
mbm_socket_max = topology_max_packages();
mbm_local = kmalloc_array(maxid * mbm_socket_max,
@@ -1649,25 +1649,20 @@ static int intel_mbm_init(void)
mbm_total = kmalloc_array(maxid * mbm_socket_max,
  sizeof(*mbm_total),
  GFP_KERNEL);
-   if (!mbm_total) {
-   ret = -ENOMEM;
-   goto out;
-   }
+   if (!mbm_total)
+   goto free_mbm;
 
mbm_timers = kmalloc_array(mbm_socket_max,
   sizeof(*mbm_timers),
   GFP_KERNEL);
-   if (!mbm_timers) {
-   ret = -ENOMEM;
-   goto out;
-   }
-   mbm_hrtimer_init();
+   if (!mbm_timers)
+   goto free_mbm;
 
-out:
-   if (ret)
-   mbm_cleanup();
-
-   return ret;
+   mbm_hrtimer_init();
+   return 0;
+ free_mbm:
+   mbm_cleanup();
+   return -ENOMEM;
 }
 
 static int __init intel_cqm_init(void)
-- 
2.9.3

[PATCH 3/4] perf/x86/cqm: One check and another variable less in intel_mbm_init()

2016-09-04 Thread SF Markus Elfring 
From: Markus Elfring 
Date: Sun, 4 Sep 2016 19:06:33 +0200

1. Adjust a jump target to eliminate an extra check at the end.

2. Move a jump label according to the current Linux coding style convention.

3. Return only constant values for the success or failure indication.

4. Delete the local variable "ret" which became unnecessary with
   this refactoring.

Signed-off-by: Markus Elfring 
---
 arch/x86/events/intel/cqm.c | 25 ++---
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/arch/x86/events/intel/cqm.c b/arch/x86/events/intel/cqm.c
index 29f756e..304caf53 100644
--- a/arch/x86/events/intel/cqm.c
+++ b/arch/x86/events/intel/cqm.c
@@ -1637,7 +1637,7 @@ static const struct x86_cpu_id intel_mbm_total_match[] = {
 
 static int intel_mbm_init(void)
 {
-   int ret = 0, maxid = cqm_max_rmid + 1;
+   int const maxid = cqm_max_rmid + 1;
 
mbm_socket_max = topology_max_packages();
mbm_local = kmalloc_array(maxid * mbm_socket_max,
@@ -1649,25 +1649,20 @@ static int intel_mbm_init(void)
mbm_total = kmalloc_array(maxid * mbm_socket_max,
  sizeof(*mbm_total),
  GFP_KERNEL);
-   if (!mbm_total) {
-   ret = -ENOMEM;
-   goto out;
-   }
+   if (!mbm_total)
+   goto free_mbm;
 
mbm_timers = kmalloc_array(mbm_socket_max,
   sizeof(*mbm_timers),
   GFP_KERNEL);
-   if (!mbm_timers) {
-   ret = -ENOMEM;
-   goto out;
-   }
-   mbm_hrtimer_init();
+   if (!mbm_timers)
+   goto free_mbm;
 
-out:
-   if (ret)
-   mbm_cleanup();
-
-   return ret;
+   mbm_hrtimer_init();
+   return 0;
+ free_mbm:
+   mbm_cleanup();
+   return -ENOMEM;
 }
 
 static int __init intel_cqm_init(void)
-- 
2.9.3