linux-kernel - Re: [PATCH v2 2/2]: perf record: enable asynchronous trace writing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 27 Aug 2018 12:33:07 +0300
From:   Alexey Budankov <alexey.budankov@...ux.intel.com>
To:     Jiri Olsa <jolsa@...hat.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Andi Kleen <ak@...ux.intel.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v2 2/2]: perf record: enable asynchronous trace writing

Hi,

On 27.08.2018 11:38, Jiri Olsa wrote:
> On Thu, Aug 23, 2018 at 07:47:01PM +0300, Alexey Budankov wrote:
> 
> SNIP
> 
>>  static int record__mmap_read_evlist(struct record *rec, struct perf_evlist *evlist,
>>  				    bool overwrite)
>>  {
>>  	u64 bytes_written = rec->bytes_written;
>> -	int i;
>> -	int rc = 0;
>> +	int i, rc = 0;
>>  	struct perf_mmap *maps;
>> +	int trace_fd = rec->session->data->file.fd;
>> +	struct aiocb **mmap_aio = rec->evlist->mmap_aio;
>> +	int mmap_aio_size = 0;
>> +	off_t off;
>>  
>>  	if (!evlist)
>>  		return 0;
>> @@ -546,14 +620,17 @@ static int record__mmap_read_evlist(struct record *rec, struct perf_evlist *evli
>>  	if (overwrite && evlist->bkw_mmap_state != BKW_MMAP_DATA_PENDING)
>>  		return 0;
>>  
>> +	off = lseek(trace_fd, 0, SEEK_CUR);
>> +
> 
> with async write, do we need to query/set the offset like this
> all the time?

It looks like we need it this way. Internally glibc AIO implements writes 
using pwrite64 syscall in our case. The sycall requires offset as a parameter 
and doesn't update file position on the completion.

> 
> could we just keep/update the offset value in the 'struct perf_data_file'
> and skip both lseek calls?

Don't see how it is possible. offset is different for every enqeued write 
operation and write areas don't intersect for the whole writing loop. 
To know the final file position it is required to iterate thru 
the loop.

> 
> jirka
> 
> SNIP
> 
>>  
>> +	record__mmap_read_sync(trace_fd, mmap_aio, mmap_aio_size, rec);
>> +
>> +	lseek(trace_fd, off, SEEK_SET);
>> +
>