Re: [PATCH] perf synthetic-events: save 4kb from 2 stack frames

[Jiri Olsa <jolsa@redhat.com>]

On Thu, Mar 26, 2020 at 11:36:51PM -0700, Ian Rogers wrote:
> Reduce the scope of PATH_MAX sized char buffers so that the compiler can
> overlap their usage.
> 
> perf_event__synthesize_mmap_events before 'sub $0x45b8,%rsp' after
> 'sub $0x35b8,%rsp'.
> 
> perf_event__get_comm_ids before 'sub $0x2028,%rsp' after 'sub $0x1028,%rsp'.

nice catch.. is this actualy problem somewhere? I thought
we don't need to care that much about this in user space

jirka

> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/util/synthetic-events.c | 46 ++++++++++++++++++------------
>  1 file changed, 27 insertions(+), 19 deletions(-)
> 
> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> index 3f28af39f9c6..9ff54707bb30 100644
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
> @@ -70,7 +70,6 @@ int perf_tool__process_synth_event(struct perf_tool *tool,
>  static int perf_event__get_comm_ids(pid_t pid, char *comm, size_t len,
>  				    pid_t *tgid, pid_t *ppid)
>  {
> -	char filename[PATH_MAX];
>  	char bf[4096];
>  	int fd;
>  	size_t size = 0;
> @@ -80,12 +79,16 @@ static int perf_event__get_comm_ids(pid_t pid, char *comm, size_t len,
>  	*tgid = -1;
>  	*ppid = -1;
>  
> -	snprintf(filename, sizeof(filename), "/proc/%d/status", pid);
> +	{
> +		char filename[PATH_MAX];
>  
> -	fd = open(filename, O_RDONLY);
> -	if (fd < 0) {
> -		pr_debug("couldn't open %s\n", filename);
> -		return -1;
> +		snprintf(filename, sizeof(filename), "/proc/%d/status", pid);
> +
> +		fd = open(filename, O_RDONLY);
> +		if (fd < 0) {
> +			pr_debug("couldn't open %s\n", filename);
> +			return -1;
> +		}
>  	}
>  
>  	n = read(fd, bf, sizeof(bf) - 1);
> @@ -280,7 +283,6 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
>  				       struct machine *machine,
>  				       bool mmap_data)
>  {
> -	char filename[PATH_MAX];
>  	FILE *fp;
>  	unsigned long long t;
>  	bool truncation = false;
> @@ -292,18 +294,22 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
>  	if (machine__is_default_guest(machine))
>  		return 0;
>  
> -	snprintf(filename, sizeof(filename), "%s/proc/%d/task/%d/maps",
> -		 machine->root_dir, pid, pid);
> +#define FILENAME_FMT_STRING "%s/proc/%d/task/%d/maps"
> +	{
> +		char filename[PATH_MAX];
>  
> -	fp = fopen(filename, "r");
> -	if (fp == NULL) {
> -		/*
> -		 * We raced with a task exiting - just return:
> -		 */
> -		pr_debug("couldn't open %s\n", filename);
> -		return -1;
> -	}
> +		snprintf(filename, sizeof(filename), FILENAME_FMT_STRING,
> +			machine->root_dir, pid, pid);
>  
> +		fp = fopen(filename, "r");
> +		if (fp == NULL) {
> +			/*
> +			 * We raced with a task exiting - just return:
> +			 */
> +			pr_debug("couldn't open %s\n", filename);
> +			return -1;
> +		}
> +	}
>  	event->header.type = PERF_RECORD_MMAP2;
>  	t = rdclock();
>  
> @@ -320,10 +326,10 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
>  			break;
>  
>  		if ((rdclock() - t) > timeout) {
> -			pr_warning("Reading %s time out. "
> +			pr_warning("Reading " FILENAME_FMT_STRING " time out. "
>  				   "You may want to increase "
>  				   "the time limit by --proc-map-timeout\n",
> -				   filename);
> +				   machine->root_dir, pid, pid);
>  			truncation = true;
>  			goto out;
>  		}
> @@ -412,6 +418,8 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
>  
>  	fclose(fp);
>  	return rc;
> +
> +#undef FILENAME_FMT_STRING
>  }
>  
>  int perf_event__synthesize_modules(struct perf_tool *tool, perf_event__handler_t process,
> -- 
> 2.25.1.696.g5e7596f4ac-goog
>