linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/2] tools/perf: Add loongarch cpuinfo and fix build warning
@ 2023-08-22  9:32 Yanteng Si
  2023-08-22  9:32 ` [PATCH v1 1/2] perf/trace: Fix mmap_flags for archs use generic mman.h Yanteng Si
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Yanteng Si @ 2023-08-22  9:32 UTC (permalink / raw)
  To: acme
  Cc: Yanteng Si, mingo, peterz, mark.rutland, alexander.shishkin,
	jolsa, namhyung, irogers, adrian.hunter, chenhuacai,
	linux-perf-users, loongson-kernel, loongarch

* LoongArch and riscv don't have tools/arch/xxxxx/include/uapi/asm/mman.h, 
  let's modify the detection rules.
* Allow to use cpuinfo on loongarch.

Yanteng Si (2):
  perf/trace: Fix mmap_flags for archs use generic mman.h
  perf/tools: Allow to use cpuinfo on loongarch

 tools/perf/trace/beauty/mmap_flags.sh | 2 +-
 tools/perf/util/header.c              | 2 ++
 tools/perf/util/svghelper.c           | 7 ++++++-
 3 files changed, 9 insertions(+), 2 deletions(-)

-- 
2.31.4


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

* [PATCH v1 1/2] perf/trace: Fix mmap_flags for archs use generic mman.h
  2023-08-22  9:32 [PATCH v1 0/2] tools/perf: Add loongarch cpuinfo and fix build warning Yanteng Si
@ 2023-08-22  9:32 ` Yanteng Si
  2023-08-22 18:02   ` Arnaldo Carvalho de Melo
  2023-08-22  9:32 ` [PATCH v1 2/2] perf/tools: Allow to use cpuinfo on loongarch Yanteng Si
  2023-08-22 15:10 ` [PATCH v1 0/2] tools/perf: Add loongarch cpuinfo and fix build warning Huacai Chen
  2 siblings, 1 reply; 11+ messages in thread
From: Yanteng Si @ 2023-08-22  9:32 UTC (permalink / raw)
  To: acme
  Cc: Yanteng Si, mingo, peterz, mark.rutland, alexander.shishkin,
	jolsa, namhyung, irogers, adrian.hunter, chenhuacai,
	linux-perf-users, loongson-kernel, loongarch

Silence this warning:

grep: /root/linux-next/tools/arch/xxxxx/include/uapi/asm//mman.h:
No such file or directory

Signed-off-by: Yanteng Si <siyanteng@loongson.cn>
---
 tools/perf/trace/beauty/mmap_flags.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/trace/beauty/mmap_flags.sh b/tools/perf/trace/beauty/mmap_flags.sh
index 3022597c8c17..d035b29fc5ec 100755
--- a/tools/perf/trace/beauty/mmap_flags.sh
+++ b/tools/perf/trace/beauty/mmap_flags.sh
@@ -19,7 +19,7 @@ arch_mman=${arch_header_dir}/mman.h
 
 printf "static const char *mmap_flags[] = {\n"
 regex='^[[:space:]]*#[[:space:]]*define[[:space:]]+MAP_([[:alnum:]_]+)[[:space:]]+(0x[[:xdigit:]]+)[[:space:]]*.*'
-grep -E -q $regex ${arch_mman} && \
+([ -f ${arch_mman} ] && grep -E -q $regex ${arch_mman}) && \
 (grep -E $regex ${arch_mman} | \
 	sed -r "s/$regex/\2 \1 \1 \1 \2/g"	| \
 	xargs printf "\t[ilog2(%s) + 1] = \"%s\",\n#ifndef MAP_%s\n#define MAP_%s %s\n#endif\n")
-- 
2.31.4


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

* [PATCH v1 2/2] perf/tools: Allow to use cpuinfo on loongarch
  2023-08-22  9:32 [PATCH v1 0/2] tools/perf: Add loongarch cpuinfo and fix build warning Yanteng Si
  2023-08-22  9:32 ` [PATCH v1 1/2] perf/trace: Fix mmap_flags for archs use generic mman.h Yanteng Si
@ 2023-08-22  9:32 ` Yanteng Si
  2023-08-22 17:50   ` Arnaldo Carvalho de Melo
  2023-08-22 15:10 ` [PATCH v1 0/2] tools/perf: Add loongarch cpuinfo and fix build warning Huacai Chen
  2 siblings, 1 reply; 11+ messages in thread
From: Yanteng Si @ 2023-08-22  9:32 UTC (permalink / raw)
  To: acme
  Cc: Yanteng Si, mingo, peterz, mark.rutland, alexander.shishkin,
	jolsa, namhyung, irogers, adrian.hunter, chenhuacai,
	linux-perf-users, loongson-kernel, loongarch

Define these macros so that the CPU name can be displayed
when running perf report and perf timechart.

Signed-off-by: Yanteng Si <siyanteng@loongson.cn>
---
 tools/perf/util/header.c    | 2 ++
 tools/perf/util/svghelper.c | 7 ++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 52fbf526fe74..c6e19192bd28 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -456,6 +456,8 @@ static int write_cpudesc(struct feat_fd *ff,
 #define CPUINFO_PROC	{ "Processor", }
 #elif defined(__xtensa__)
 #define CPUINFO_PROC	{ "core ID", }
+#elif defined(__loongarch__)
+#define CPUINFO_PROC	{ "Model Name", }
 #else
 #define CPUINFO_PROC	{ "model name", }
 #endif
diff --git a/tools/perf/util/svghelper.c b/tools/perf/util/svghelper.c
index 5c62d3118c41..0259715ad6a6 100644
--- a/tools/perf/util/svghelper.c
+++ b/tools/perf/util/svghelper.c
@@ -322,6 +322,11 @@ void svg_waiting(int Yslot, int cpu, u64 start, u64 end, const char *backtrace)
 
 static char *cpu_model(void)
 {
+#if defined(__loongarch__)
+#define CPU_TYPE	"Model Name"
+#else
+#define CPU_TYPE	"model name"
+#endif
 	static char cpu_m[255];
 	char buf[256];
 	FILE *file;
@@ -331,7 +336,7 @@ static char *cpu_model(void)
 	file = fopen("/proc/cpuinfo", "r");
 	if (file) {
 		while (fgets(buf, 255, file)) {
-			if (strstr(buf, "model name")) {
+			if (strstr(buf, CPU_TYPE)) {
 				strlcpy(cpu_m, &buf[13], 255);
 				break;
 			}
-- 
2.31.4


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

* Re: [PATCH v1 0/2] tools/perf: Add loongarch cpuinfo and fix build warning
  2023-08-22  9:32 [PATCH v1 0/2] tools/perf: Add loongarch cpuinfo and fix build warning Yanteng Si
  2023-08-22  9:32 ` [PATCH v1 1/2] perf/trace: Fix mmap_flags for archs use generic mman.h Yanteng Si
  2023-08-22  9:32 ` [PATCH v1 2/2] perf/tools: Allow to use cpuinfo on loongarch Yanteng Si
@ 2023-08-22 15:10 ` Huacai Chen
  2 siblings, 0 replies; 11+ messages in thread
From: Huacai Chen @ 2023-08-22 15:10 UTC (permalink / raw)
  To: Yanteng Si
  Cc: acme, mingo, peterz, mark.rutland, alexander.shishkin, jolsa,
	namhyung, irogers, adrian.hunter, linux-perf-users,
	loongson-kernel, loongarch

For this series:

Acked-by: Huacai Chen <chenhuacai@loongson.cn>

On Tue, Aug 22, 2023 at 5:32 PM Yanteng Si <siyanteng@loongson.cn> wrote:
>
> * LoongArch and riscv don't have tools/arch/xxxxx/include/uapi/asm/mman.h,
>   let's modify the detection rules.
> * Allow to use cpuinfo on loongarch.
>
> Yanteng Si (2):
>   perf/trace: Fix mmap_flags for archs use generic mman.h
>   perf/tools: Allow to use cpuinfo on loongarch
>
>  tools/perf/trace/beauty/mmap_flags.sh | 2 +-
>  tools/perf/util/header.c              | 2 ++
>  tools/perf/util/svghelper.c           | 7 ++++++-
>  3 files changed, 9 insertions(+), 2 deletions(-)
>
> --
> 2.31.4
>
>

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

* Re: [PATCH v1 2/2] perf/tools: Allow to use cpuinfo on loongarch
  2023-08-22  9:32 ` [PATCH v1 2/2] perf/tools: Allow to use cpuinfo on loongarch Yanteng Si
@ 2023-08-22 17:50   ` Arnaldo Carvalho de Melo
  2023-08-23  7:11     ` Yanteng Si
  0 siblings, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-08-22 17:50 UTC (permalink / raw)
  To: Yanteng Si
  Cc: mingo, peterz, mark.rutland, alexander.shishkin, jolsa, namhyung,
	irogers, adrian.hunter, chenhuacai, linux-perf-users,
	loongson-kernel, loongarch

Em Tue, Aug 22, 2023 at 05:32:07PM +0800, Yanteng Si escreveu:
> Define these macros so that the CPU name can be displayed
> when running perf report and perf timechart.
> 
> Signed-off-by: Yanteng Si <siyanteng@loongson.cn>
> ---
>  tools/perf/util/header.c    | 2 ++
>  tools/perf/util/svghelper.c | 7 ++++++-
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 52fbf526fe74..c6e19192bd28 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -456,6 +456,8 @@ static int write_cpudesc(struct feat_fd *ff,
>  #define CPUINFO_PROC	{ "Processor", }
>  #elif defined(__xtensa__)
>  #define CPUINFO_PROC	{ "core ID", }
> +#elif defined(__loongarch__)
> +#define CPUINFO_PROC	{ "Model Name", }
>  #else
>  #define CPUINFO_PROC	{ "model name", }
>  #endif
> diff --git a/tools/perf/util/svghelper.c b/tools/perf/util/svghelper.c
> index 5c62d3118c41..0259715ad6a6 100644
> --- a/tools/perf/util/svghelper.c
> +++ b/tools/perf/util/svghelper.c
> @@ -322,6 +322,11 @@ void svg_waiting(int Yslot, int cpu, u64 start, u64 end, const char *backtrace)
>  
>  static char *cpu_model(void)
>  {
> +#if defined(__loongarch__)
> +#define CPU_TYPE	"Model Name"
> +#else
> +#define CPU_TYPE	"model name"
> +#endif
>  	static char cpu_m[255];
>  	char buf[256];
>  	FILE *file;
> @@ -331,7 +336,7 @@ static char *cpu_model(void)
>  	file = fopen("/proc/cpuinfo", "r");
>  	if (file) {
>  		while (fgets(buf, 255, file)) {
> -			if (strstr(buf, "model name")) {
> +			if (strstr(buf, CPU_TYPE)) {

Isn't it better to just switch to strcasestr()?

- Arnaldo

>  				strlcpy(cpu_m, &buf[13], 255);
>  				break;
>  			}
> -- 
> 2.31.4
> 

-- 

- Arnaldo

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

* Re: [PATCH v1 1/2] perf/trace: Fix mmap_flags for archs use generic mman.h
  2023-08-22  9:32 ` [PATCH v1 1/2] perf/trace: Fix mmap_flags for archs use generic mman.h Yanteng Si
@ 2023-08-22 18:02   ` Arnaldo Carvalho de Melo
  2023-08-23  7:11     ` Yanteng Si
  0 siblings, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-08-22 18:02 UTC (permalink / raw)
  To: Yanteng Si
  Cc: mingo, peterz, mark.rutland, alexander.shishkin, jolsa, namhyung,
	irogers, adrian.hunter, chenhuacai, linux-perf-users,
	loongson-kernel, loongarch

Em Tue, Aug 22, 2023 at 05:32:06PM +0800, Yanteng Si escreveu:
> Silence this warning:
> 
> grep: /root/linux-next/tools/arch/xxxxx/include/uapi/asm//mman.h:
> No such file or directory

 
> Signed-off-by: Yanteng Si <siyanteng@loongson.cn>
> ---
>  tools/perf/trace/beauty/mmap_flags.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/trace/beauty/mmap_flags.sh b/tools/perf/trace/beauty/mmap_flags.sh
> index 3022597c8c17..d035b29fc5ec 100755
> --- a/tools/perf/trace/beauty/mmap_flags.sh
> +++ b/tools/perf/trace/beauty/mmap_flags.sh
> @@ -19,7 +19,7 @@ arch_mman=${arch_header_dir}/mman.h
>  
>  printf "static const char *mmap_flags[] = {\n"
>  regex='^[[:space:]]*#[[:space:]]*define[[:space:]]+MAP_([[:alnum:]_]+)[[:space:]]+(0x[[:xdigit:]]+)[[:space:]]*.*'
> -grep -E -q $regex ${arch_mman} && \
> +([ -f ${arch_mman} ] && grep -E -q $regex ${arch_mman}) && \
>  (grep -E $regex ${arch_mman} | \
>  	sed -r "s/$regex/\2 \1 \1 \1 \2/g"	| \
>  	xargs printf "\t[ilog2(%s) + 1] = \"%s\",\n#ifndef MAP_%s\n#define MAP_%s %s\n#endif\n")

Humm, wouldn't this be simpler without that extra subshell? Like this:

diff --git a/tools/perf/trace/beauty/mmap_flags.sh b/tools/perf/trace/beauty/mmap_flags.sh
index 3022597c8c178f21..0ac1886a83b5aceb 100755
--- a/tools/perf/trace/beauty/mmap_flags.sh
+++ b/tools/perf/trace/beauty/mmap_flags.sh
@@ -19,6 +19,7 @@ arch_mman=${arch_header_dir}/mman.h
 
 printf "static const char *mmap_flags[] = {\n"
 regex='^[[:space:]]*#[[:space:]]*define[[:space:]]+MAP_([[:alnum:]_]+)[[:space:]]+(0x[[:xdigit:]]+)[[:space:]]*.*'
+test -f ${arch_mman} && \
 grep -E -q $regex ${arch_mman} && \
 (grep -E $regex ${arch_mman} | \
 	sed -r "s/$regex/\2 \1 \1 \1 \2/g"	| \

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

* Re: [PATCH v1 1/2] perf/trace: Fix mmap_flags for archs use generic mman.h
  2023-08-22 18:02   ` Arnaldo Carvalho de Melo
@ 2023-08-23  7:11     ` Yanteng Si
  2023-08-23 11:36       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 11+ messages in thread
From: Yanteng Si @ 2023-08-23  7:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: mingo, peterz, mark.rutland, alexander.shishkin, jolsa, namhyung,
	irogers, adrian.hunter, chenhuacai, linux-perf-users,
	loongson-kernel, loongarch


在 2023/8/23 02:02, Arnaldo Carvalho de Melo 写道:
> Em Tue, Aug 22, 2023 at 05:32:06PM +0800, Yanteng Si escreveu:
>> Silence this warning:
>>
>> grep: /root/linux-next/tools/arch/xxxxx/include/uapi/asm//mman.h:
>> No such file or directory
>   
>> Signed-off-by: Yanteng Si <siyanteng@loongson.cn>
>> ---
>>   tools/perf/trace/beauty/mmap_flags.sh | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/trace/beauty/mmap_flags.sh b/tools/perf/trace/beauty/mmap_flags.sh
>> index 3022597c8c17..d035b29fc5ec 100755
>> --- a/tools/perf/trace/beauty/mmap_flags.sh
>> +++ b/tools/perf/trace/beauty/mmap_flags.sh
>> @@ -19,7 +19,7 @@ arch_mman=${arch_header_dir}/mman.h
>>   
>>   printf "static const char *mmap_flags[] = {\n"
>>   regex='^[[:space:]]*#[[:space:]]*define[[:space:]]+MAP_([[:alnum:]_]+)[[:space:]]+(0x[[:xdigit:]]+)[[:space:]]*.*'
>> -grep -E -q $regex ${arch_mman} && \
>> +([ -f ${arch_mman} ] && grep -E -q $regex ${arch_mman}) && \
>>   (grep -E $regex ${arch_mman} | \
>>   	sed -r "s/$regex/\2 \1 \1 \1 \2/g"	| \
>>   	xargs printf "\t[ilog2(%s) + 1] = \"%s\",\n#ifndef MAP_%s\n#define MAP_%s %s\n#endif\n")
> Humm, wouldn't this be simpler without that extra subshell? Like this:
>
> diff --git a/tools/perf/trace/beauty/mmap_flags.sh b/tools/perf/trace/beauty/mmap_flags.sh
> index 3022597c8c178f21..0ac1886a83b5aceb 100755
> --- a/tools/perf/trace/beauty/mmap_flags.sh
> +++ b/tools/perf/trace/beauty/mmap_flags.sh
> @@ -19,6 +19,7 @@ arch_mman=${arch_header_dir}/mman.h
>   
>   printf "static const char *mmap_flags[] = {\n"
>   regex='^[[:space:]]*#[[:space:]]*define[[:space:]]+MAP_([[:alnum:]_]+)[[:space:]]+(0x[[:xdigit:]]+)[[:space:]]*.*'
> +test -f ${arch_mman} && \

The following lines have already used the [ ! -f ${arch_mman}, so let's 
stay consistent?


Thanks,

Yanteng

>   grep -E -q $regex ${arch_mman} && \
>   (grep -E $regex ${arch_mman} | \
>   	sed -r "s/$regex/\2 \1 \1 \1 \2/g"	| \


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

* Re: [PATCH v1 2/2] perf/tools: Allow to use cpuinfo on loongarch
  2023-08-22 17:50   ` Arnaldo Carvalho de Melo
@ 2023-08-23  7:11     ` Yanteng Si
  0 siblings, 0 replies; 11+ messages in thread
From: Yanteng Si @ 2023-08-23  7:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: mingo, peterz, mark.rutland, alexander.shishkin, jolsa, namhyung,
	irogers, adrian.hunter, chenhuacai, linux-perf-users,
	loongson-kernel, loongarch


在 2023/8/23 01:50, Arnaldo Carvalho de Melo 写道:
> Em Tue, Aug 22, 2023 at 05:32:07PM +0800, Yanteng Si escreveu:
>> Define these macros so that the CPU name can be displayed
>> when running perf report and perf timechart.
>>
>> Signed-off-by: Yanteng Si <siyanteng@loongson.cn>
>> ---
>>   tools/perf/util/header.c    | 2 ++
>>   tools/perf/util/svghelper.c | 7 ++++++-
>>   2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
>> index 52fbf526fe74..c6e19192bd28 100644
>> --- a/tools/perf/util/header.c
>> +++ b/tools/perf/util/header.c
>> @@ -456,6 +456,8 @@ static int write_cpudesc(struct feat_fd *ff,
>>   #define CPUINFO_PROC	{ "Processor", }
>>   #elif defined(__xtensa__)
>>   #define CPUINFO_PROC	{ "core ID", }
>> +#elif defined(__loongarch__)
>> +#define CPUINFO_PROC	{ "Model Name", }
>>   #else
>>   #define CPUINFO_PROC	{ "model name", }
>>   #endif
>> diff --git a/tools/perf/util/svghelper.c b/tools/perf/util/svghelper.c
>> index 5c62d3118c41..0259715ad6a6 100644
>> --- a/tools/perf/util/svghelper.c
>> +++ b/tools/perf/util/svghelper.c
>> @@ -322,6 +322,11 @@ void svg_waiting(int Yslot, int cpu, u64 start, u64 end, const char *backtrace)
>>   
>>   static char *cpu_model(void)
>>   {
>> +#if defined(__loongarch__)
>> +#define CPU_TYPE	"Model Name"
>> +#else
>> +#define CPU_TYPE	"model name"
>> +#endif
>>   	static char cpu_m[255];
>>   	char buf[256];
>>   	FILE *file;
>> @@ -331,7 +336,7 @@ static char *cpu_model(void)
>>   	file = fopen("/proc/cpuinfo", "r");
>>   	if (file) {
>>   		while (fgets(buf, 255, file)) {
>> -			if (strstr(buf, "model name")) {
>> +			if (strstr(buf, CPU_TYPE)) {
> Isn't it better to just switch to strcasestr()?

OK!


Thanks,

Yanteng

>
> - Arnaldo
>
>>   				strlcpy(cpu_m, &buf[13], 255);
>>   				break;
>>   			}
>> -- 
>> 2.31.4
>>


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

* Re: [PATCH v1 1/2] perf/trace: Fix mmap_flags for archs use generic mman.h
  2023-08-23  7:11     ` Yanteng Si
@ 2023-08-23 11:36       ` Arnaldo Carvalho de Melo
  2023-08-24  2:22         ` Yanteng Si
  0 siblings, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-08-23 11:36 UTC (permalink / raw)
  To: Yanteng Si
  Cc: mingo, peterz, mark.rutland, alexander.shishkin, jolsa, namhyung,
	irogers, adrian.hunter, chenhuacai, linux-perf-users,
	loongson-kernel, loongarch

Em Wed, Aug 23, 2023 at 03:11:01PM +0800, Yanteng Si escreveu:
> 
> 在 2023/8/23 02:02, Arnaldo Carvalho de Melo 写道:
> > Em Tue, Aug 22, 2023 at 05:32:06PM +0800, Yanteng Si escreveu:
> > > Silence this warning:
> > > 
> > > grep: /root/linux-next/tools/arch/xxxxx/include/uapi/asm//mman.h:
> > > No such file or directory
> > > Signed-off-by: Yanteng Si <siyanteng@loongson.cn>
> > > ---
> > >   tools/perf/trace/beauty/mmap_flags.sh | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/tools/perf/trace/beauty/mmap_flags.sh b/tools/perf/trace/beauty/mmap_flags.sh
> > > index 3022597c8c17..d035b29fc5ec 100755
> > > --- a/tools/perf/trace/beauty/mmap_flags.sh
> > > +++ b/tools/perf/trace/beauty/mmap_flags.sh
> > > @@ -19,7 +19,7 @@ arch_mman=${arch_header_dir}/mman.h
> > >   printf "static const char *mmap_flags[] = {\n"
> > >   regex='^[[:space:]]*#[[:space:]]*define[[:space:]]+MAP_([[:alnum:]_]+)[[:space:]]+(0x[[:xdigit:]]+)[[:space:]]*.*'
> > > -grep -E -q $regex ${arch_mman} && \
> > > +([ -f ${arch_mman} ] && grep -E -q $regex ${arch_mman}) && \
> > >   (grep -E $regex ${arch_mman} | \
> > >   	sed -r "s/$regex/\2 \1 \1 \1 \2/g"	| \
> > >   	xargs printf "\t[ilog2(%s) + 1] = \"%s\",\n#ifndef MAP_%s\n#define MAP_%s %s\n#endif\n")
> > Humm, wouldn't this be simpler without that extra subshell? Like this:
> > 
> > diff --git a/tools/perf/trace/beauty/mmap_flags.sh b/tools/perf/trace/beauty/mmap_flags.sh
> > index 3022597c8c178f21..0ac1886a83b5aceb 100755
> > --- a/tools/perf/trace/beauty/mmap_flags.sh
> > +++ b/tools/perf/trace/beauty/mmap_flags.sh
> > @@ -19,6 +19,7 @@ arch_mman=${arch_header_dir}/mman.h
> >   printf "static const char *mmap_flags[] = {\n"
> >   regex='^[[:space:]]*#[[:space:]]*define[[:space:]]+MAP_([[:alnum:]_]+)[[:space:]]+(0x[[:xdigit:]]+)[[:space:]]*.*'
> > +test -f ${arch_mman} && \
> 
> The following lines have already used the [ ! -f ${arch_mman}, so let's stay
> consistent?

Well, we can make it consistent by simplifying the other places where
the long, equivalent test is being done, which I think is the way to go,
agreed?

- Arnaldo
 
> 
> Thanks,
> 
> Yanteng
> 
> >   grep -E -q $regex ${arch_mman} && \
> >   (grep -E $regex ${arch_mman} | \
> >   	sed -r "s/$regex/\2 \1 \1 \1 \2/g"	| \
> 

-- 

- Arnaldo

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

* Re: [PATCH v1 1/2] perf/trace: Fix mmap_flags for archs use generic mman.h
  2023-08-23 11:36       ` Arnaldo Carvalho de Melo
@ 2023-08-24  2:22         ` Yanteng Si
  2023-08-24 13:33           ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 11+ messages in thread
From: Yanteng Si @ 2023-08-24  2:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: mingo, peterz, mark.rutland, alexander.shishkin, jolsa, namhyung,
	irogers, adrian.hunter, chenhuacai, linux-perf-users,
	loongson-kernel, loongarch


在 2023/8/23 19:36, Arnaldo Carvalho de Melo 写道:
> Em Wed, Aug 23, 2023 at 03:11:01PM +0800, Yanteng Si escreveu:
>> 在 2023/8/23 02:02, Arnaldo Carvalho de Melo 写道:
>>> Em Tue, Aug 22, 2023 at 05:32:06PM +0800, Yanteng Si escreveu:
>>>> Silence this warning:
>>>>
>>>> grep: /root/linux-next/tools/arch/xxxxx/include/uapi/asm//mman.h:
>>>> No such file or directory
>>>> Signed-off-by: Yanteng Si <siyanteng@loongson.cn>
>>>> ---
>>>>    tools/perf/trace/beauty/mmap_flags.sh | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/tools/perf/trace/beauty/mmap_flags.sh b/tools/perf/trace/beauty/mmap_flags.sh
>>>> index 3022597c8c17..d035b29fc5ec 100755
>>>> --- a/tools/perf/trace/beauty/mmap_flags.sh
>>>> +++ b/tools/perf/trace/beauty/mmap_flags.sh
>>>> @@ -19,7 +19,7 @@ arch_mman=${arch_header_dir}/mman.h
>>>>    printf "static const char *mmap_flags[] = {\n"
>>>>    regex='^[[:space:]]*#[[:space:]]*define[[:space:]]+MAP_([[:alnum:]_]+)[[:space:]]+(0x[[:xdigit:]]+)[[:space:]]*.*'
>>>> -grep -E -q $regex ${arch_mman} && \
>>>> +([ -f ${arch_mman} ] && grep -E -q $regex ${arch_mman}) && \
>>>>    (grep -E $regex ${arch_mman} | \
>>>>    	sed -r "s/$regex/\2 \1 \1 \1 \2/g"	| \
>>>>    	xargs printf "\t[ilog2(%s) + 1] = \"%s\",\n#ifndef MAP_%s\n#define MAP_%s %s\n#endif\n")
>>> Humm, wouldn't this be simpler without that extra subshell? Like this:
>>>
>>> diff --git a/tools/perf/trace/beauty/mmap_flags.sh b/tools/perf/trace/beauty/mmap_flags.sh
>>> index 3022597c8c178f21..0ac1886a83b5aceb 100755
>>> --- a/tools/perf/trace/beauty/mmap_flags.sh
>>> +++ b/tools/perf/trace/beauty/mmap_flags.sh
>>> @@ -19,6 +19,7 @@ arch_mman=${arch_header_dir}/mman.h
>>>    printf "static const char *mmap_flags[] = {\n"
>>>    regex='^[[:space:]]*#[[:space:]]*define[[:space:]]+MAP_([[:alnum:]_]+)[[:space:]]+(0x[[:xdigit:]]+)[[:space:]]*.*'
>>> +test -f ${arch_mman} && \
>> The following lines have already used the [ ! -f ${arch_mman}, so let's stay
>> consistent?
> Well, we can make it consistent by simplifying the other places where
> the long, equivalent test is being done, which I think is the way to go,
> agreed?

I agree, but this is not suitable to be done in the fix patch, I will 
split it out and make a new patch.


Thanks

Yanteng

>
> - Arnaldo
>   
>> Thanks,
>>
>> Yanteng
>>
>>>    grep -E -q $regex ${arch_mman} && \
>>>    (grep -E $regex ${arch_mman} | \
>>>    	sed -r "s/$regex/\2 \1 \1 \1 \2/g"	| \


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

* Re: [PATCH v1 1/2] perf/trace: Fix mmap_flags for archs use generic mman.h
  2023-08-24  2:22         ` Yanteng Si
@ 2023-08-24 13:33           ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-08-24 13:33 UTC (permalink / raw)
  To: Yanteng Si
  Cc: mingo, peterz, mark.rutland, alexander.shishkin, jolsa, namhyung,
	irogers, adrian.hunter, chenhuacai, linux-perf-users,
	loongson-kernel, loongarch

Em Thu, Aug 24, 2023 at 10:22:57AM +0800, Yanteng Si escreveu:
> 
> 在 2023/8/23 19:36, Arnaldo Carvalho de Melo 写道:
> > Em Wed, Aug 23, 2023 at 03:11:01PM +0800, Yanteng Si escreveu:
> > > 在 2023/8/23 02:02, Arnaldo Carvalho de Melo 写道:
> > > > Em Tue, Aug 22, 2023 at 05:32:06PM +0800, Yanteng Si escreveu:
> > > > > Silence this warning:
> > > > > 
> > > > > grep: /root/linux-next/tools/arch/xxxxx/include/uapi/asm//mman.h:
> > > > > No such file or directory
> > > > > Signed-off-by: Yanteng Si <siyanteng@loongson.cn>
> > > > > ---
> > > > >    tools/perf/trace/beauty/mmap_flags.sh | 2 +-
> > > > >    1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/tools/perf/trace/beauty/mmap_flags.sh b/tools/perf/trace/beauty/mmap_flags.sh
> > > > > index 3022597c8c17..d035b29fc5ec 100755
> > > > > --- a/tools/perf/trace/beauty/mmap_flags.sh
> > > > > +++ b/tools/perf/trace/beauty/mmap_flags.sh
> > > > > @@ -19,7 +19,7 @@ arch_mman=${arch_header_dir}/mman.h
> > > > >    printf "static const char *mmap_flags[] = {\n"
> > > > >    regex='^[[:space:]]*#[[:space:]]*define[[:space:]]+MAP_([[:alnum:]_]+)[[:space:]]+(0x[[:xdigit:]]+)[[:space:]]*.*'
> > > > > -grep -E -q $regex ${arch_mman} && \
> > > > > +([ -f ${arch_mman} ] && grep -E -q $regex ${arch_mman}) && \
> > > > >    (grep -E $regex ${arch_mman} | \
> > > > >    	sed -r "s/$regex/\2 \1 \1 \1 \2/g"	| \
> > > > >    	xargs printf "\t[ilog2(%s) + 1] = \"%s\",\n#ifndef MAP_%s\n#define MAP_%s %s\n#endif\n")
> > > > Humm, wouldn't this be simpler without that extra subshell? Like this:
> > > > 
> > > > diff --git a/tools/perf/trace/beauty/mmap_flags.sh b/tools/perf/trace/beauty/mmap_flags.sh
> > > > index 3022597c8c178f21..0ac1886a83b5aceb 100755
> > > > --- a/tools/perf/trace/beauty/mmap_flags.sh
> > > > +++ b/tools/perf/trace/beauty/mmap_flags.sh
> > > > @@ -19,6 +19,7 @@ arch_mman=${arch_header_dir}/mman.h
> > > >    printf "static const char *mmap_flags[] = {\n"
> > > >    regex='^[[:space:]]*#[[:space:]]*define[[:space:]]+MAP_([[:alnum:]_]+)[[:space:]]+(0x[[:xdigit:]]+)[[:space:]]*.*'
> > > > +test -f ${arch_mman} && \
> > > The following lines have already used the [ ! -f ${arch_mman}, so let's stay
> > > consistent?
> > Well, we can make it consistent by simplifying the other places where
> > the long, equivalent test is being done, which I think is the way to go,
> > agreed?
> 
> I agree, but this is not suitable to be done in the fix patch, I will split
> it out and make a new patch.

Sure, send the one with the fix, already using the simple form, then
convert the rest to the simpler form.

Thanks,

- Arnaldo

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

end of thread, other threads:[~2023-08-24 13:34 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-22  9:32 [PATCH v1 0/2] tools/perf: Add loongarch cpuinfo and fix build warning Yanteng Si
2023-08-22  9:32 ` [PATCH v1 1/2] perf/trace: Fix mmap_flags for archs use generic mman.h Yanteng Si
2023-08-22 18:02   ` Arnaldo Carvalho de Melo
2023-08-23  7:11     ` Yanteng Si
2023-08-23 11:36       ` Arnaldo Carvalho de Melo
2023-08-24  2:22         ` Yanteng Si
2023-08-24 13:33           ` Arnaldo Carvalho de Melo
2023-08-22  9:32 ` [PATCH v1 2/2] perf/tools: Allow to use cpuinfo on loongarch Yanteng Si
2023-08-22 17:50   ` Arnaldo Carvalho de Melo
2023-08-23  7:11     ` Yanteng Si
2023-08-22 15:10 ` [PATCH v1 0/2] tools/perf: Add loongarch cpuinfo and fix build warning Huacai Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).