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


Groups > linux.kernel > #1220230 > unrolled thread

[PATCH V2 03/14] perf tools: Remove redundant validation from perf_evlist__propagate_maps

Started byAdrian Hunter <adrian.hunter@intel.com>
First post2015-09-07 16:40 +0200
Last post2015-09-08 09:30 +0200
Articles 3 — 2 participants

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 V2 03/14] perf tools: Remove redundant validation from perf_evlist__propagate_maps Adrian Hunter <adrian.hunter@intel.com> - 2015-09-07 16:40 +0200
    Re: [PATCH V2 03/14] perf tools: Remove redundant validation from  perf_evlist__propagate_maps Jiri Olsa <jolsa@redhat.com> - 2015-09-08 09:00 +0200
      Re: [PATCH V2 03/14] perf tools: Remove redundant validation from  perf_evlist__propagate_maps Adrian Hunter <adrian.hunter@intel.com> - 2015-09-08 09:30 +0200

#1220230 — [PATCH V2 03/14] perf tools: Remove redundant validation from perf_evlist__propagate_maps

FromAdrian Hunter <adrian.hunter@intel.com>
Date2015-09-07 16:40 +0200
Subject[PATCH V2 03/14] perf tools: Remove redundant validation from perf_evlist__propagate_maps
Message-ID<q65Kq-8kq-37@gated-at.bofh.it>
The validation checks that the values that were just assigned, got
assigned i.e. the error can't ever happen.  Subsequent patches will
call this code in places where errors are not being returned.
Changing those code paths to return this non-existent error is
counter-productive, so just remove it.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evlist.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 9cb9296cc4f8..840d5210be72 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1103,8 +1103,8 @@ int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
 	return perf_evlist__mmap_ex(evlist, pages, overwrite, 0, false);
 }
 
-static int perf_evlist__propagate_maps(struct perf_evlist *evlist,
-				       bool has_user_cpus)
+static void perf_evlist__propagate_maps(struct perf_evlist *evlist,
+					bool has_user_cpus)
 {
 	struct perf_evsel *evsel;
 
@@ -1119,13 +1119,7 @@ static int perf_evlist__propagate_maps(struct perf_evlist *evlist,
 		}
 
 		evsel->threads = thread_map__get(evlist->threads);
-
-		if ((evlist->cpus && !evsel->cpus) ||
-		    (evlist->threads && !evsel->threads))
-			return -ENOMEM;
 	}
-
-	return 0;
 }
 
 int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
@@ -1144,7 +1138,9 @@ int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
 	if (evlist->cpus == NULL)
 		goto out_delete_threads;
 
-	return perf_evlist__propagate_maps(evlist, !!target->cpu_list);
+	perf_evlist__propagate_maps(evlist, !!target->cpu_list);
+
+	return 0;
 
 out_delete_threads:
 	thread_map__put(evlist->threads);
@@ -1162,7 +1158,9 @@ int perf_evlist__set_maps(struct perf_evlist *evlist,
 	thread_map__put(evlist->threads);
 	evlist->threads = threads;
 
-	return perf_evlist__propagate_maps(evlist, false);
+	perf_evlist__propagate_maps(evlist, false);
+
+	return 0;
 }
 
 int perf_evlist__apply_filters(struct perf_evlist *evlist, struct perf_evsel **err_evsel)
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1220524 — Re: [PATCH V2 03/14] perf tools: Remove redundant validation from perf_evlist__propagate_maps

FromJiri Olsa <jolsa@redhat.com>
Date2015-09-08 09:00 +0200
SubjectRe: [PATCH V2 03/14] perf tools: Remove redundant validation from perf_evlist__propagate_maps
Message-ID<q6l2O-55D-7@gated-at.bofh.it>
In reply to#1220230
On Mon, Sep 07, 2015 at 05:27:45PM +0300, Adrian Hunter wrote:
> The validation checks that the values that were just assigned, got
> assigned i.e. the error can't ever happen.  Subsequent patches will
> call this code in places where errors are not being returned.
> Changing those code paths to return this non-existent error is
> counter-productive, so just remove it.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  tools/perf/util/evlist.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 9cb9296cc4f8..840d5210be72 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -1103,8 +1103,8 @@ int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
>  	return perf_evlist__mmap_ex(evlist, pages, overwrite, 0, false);
>  }
>  
> -static int perf_evlist__propagate_maps(struct perf_evlist *evlist,
> -				       bool has_user_cpus)
> +static void perf_evlist__propagate_maps(struct perf_evlist *evlist,
> +					bool has_user_cpus)
>  {
>  	struct perf_evsel *evsel;
>  
> @@ -1119,13 +1119,7 @@ static int perf_evlist__propagate_maps(struct perf_evlist *evlist,
>  		}
>  
>  		evsel->threads = thread_map__get(evlist->threads);
> -
> -		if ((evlist->cpus && !evsel->cpus) ||
> -		    (evlist->threads && !evsel->threads))
> -			return -ENOMEM;
>  	}
> -
> -	return 0;
>  }
>  
>  int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
> @@ -1144,7 +1138,9 @@ int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
>  	if (evlist->cpus == NULL)
>  		goto out_delete_threads;
>  
> -	return perf_evlist__propagate_maps(evlist, !!target->cpu_list);
> +	perf_evlist__propagate_maps(evlist, !!target->cpu_list);
> +
> +	return 0;
>  
>  out_delete_threads:
>  	thread_map__put(evlist->threads);
> @@ -1162,7 +1158,9 @@ int perf_evlist__set_maps(struct perf_evlist *evlist,
>  	thread_map__put(evlist->threads);
>  	evlist->threads = threads;
>  
> -	return perf_evlist__propagate_maps(evlist, false);
> +	perf_evlist__propagate_maps(evlist, false);
> +
> +	return 0;

perf_evlist__set_maps could now also return void

jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1220543 — Re: [PATCH V2 03/14] perf tools: Remove redundant validation from perf_evlist__propagate_maps

FromAdrian Hunter <adrian.hunter@intel.com>
Date2015-09-08 09:30 +0200
SubjectRe: [PATCH V2 03/14] perf tools: Remove redundant validation from perf_evlist__propagate_maps
Message-ID<q6lvP-5SG-13@gated-at.bofh.it>
In reply to#1220524
On 08/09/15 09:53, Jiri Olsa wrote:
> On Mon, Sep 07, 2015 at 05:27:45PM +0300, Adrian Hunter wrote:
>> The validation checks that the values that were just assigned, got
>> assigned i.e. the error can't ever happen.  Subsequent patches will
>> call this code in places where errors are not being returned.
>> Changing those code paths to return this non-existent error is
>> counter-productive, so just remove it.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>  tools/perf/util/evlist.c | 18 ++++++++----------
>>  1 file changed, 8 insertions(+), 10 deletions(-)
>>
>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>> index 9cb9296cc4f8..840d5210be72 100644
>> --- a/tools/perf/util/evlist.c
>> +++ b/tools/perf/util/evlist.c
>> @@ -1103,8 +1103,8 @@ int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
>>  	return perf_evlist__mmap_ex(evlist, pages, overwrite, 0, false);
>>  }
>>  
>> -static int perf_evlist__propagate_maps(struct perf_evlist *evlist,
>> -				       bool has_user_cpus)
>> +static void perf_evlist__propagate_maps(struct perf_evlist *evlist,
>> +					bool has_user_cpus)
>>  {
>>  	struct perf_evsel *evsel;
>>  
>> @@ -1119,13 +1119,7 @@ static int perf_evlist__propagate_maps(struct perf_evlist *evlist,
>>  		}
>>  
>>  		evsel->threads = thread_map__get(evlist->threads);
>> -
>> -		if ((evlist->cpus && !evsel->cpus) ||
>> -		    (evlist->threads && !evsel->threads))
>> -			return -ENOMEM;
>>  	}
>> -
>> -	return 0;
>>  }
>>  
>>  int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
>> @@ -1144,7 +1138,9 @@ int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
>>  	if (evlist->cpus == NULL)
>>  		goto out_delete_threads;
>>  
>> -	return perf_evlist__propagate_maps(evlist, !!target->cpu_list);
>> +	perf_evlist__propagate_maps(evlist, !!target->cpu_list);
>> +
>> +	return 0;
>>  
>>  out_delete_threads:
>>  	thread_map__put(evlist->threads);
>> @@ -1162,7 +1158,9 @@ int perf_evlist__set_maps(struct perf_evlist *evlist,
>>  	thread_map__put(evlist->threads);
>>  	evlist->threads = threads;
>>  
>> -	return perf_evlist__propagate_maps(evlist, false);
>> +	perf_evlist__propagate_maps(evlist, false);
>> +
>> +	return 0;
> 
> perf_evlist__set_maps could now also return void

And most of the callers aren't checking the return value either, so yes that
needs to be tidied up.  I will make a V3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web