Messages in this thread | | | Subject | Re: [PATCH RFC 07/19] perf tools: Workaround missing maps for x86_64 KPTI entry trampolines | From | Adrian Hunter <> | Date | Mon, 14 May 2018 16:02:38 +0300 |
| |
On 11/05/18 17:45, Arnaldo Carvalho de Melo wrote: > Em Fri, May 11, 2018 at 02:18:01PM +0300, Adrian Hunter escreveu: >> On 10/05/18 23:47, Arnaldo Carvalho de Melo wrote: >>> Em Thu, May 10, 2018 at 05:19:22PM -0300, Arnaldo Carvalho de Melo escreveu: >>>> Em Thu, May 10, 2018 at 05:15:42PM -0300, Arnaldo Carvalho de Melo escreveu: >>>>> Em Thu, May 10, 2018 at 07:08:37PM +0000, Hunter, Adrian escreveu: >>>>>> Let me know if you want me to post the workaround patches separately, >>>>>> otherwise I will wait a bit before sending the patches again. >>> >>>>> I'll see if I went thru all of the patches already... >>> >>>> So I looked at the patches posted and one comment is about the terse >>>> commit logs for some of the kcore_copy patches, for instance: >>> >>>> -------------------- >>>> In preparation to add more program headers, get rid of kernel_map and >>>> modules_map. >>>> -------------------- >>> >>>> Can't this be made a bit more verbose? Lemme re-read the patch... >>> >>> So you had just one pointers to the kernel map and a module_maps, and >>> then this is replaced by kcore_copy__map() that instead of populating >>> those fields that are being removed: >>> >>> - struct phdr_data kernel_map; >>> - struct phdr_data modules_map; >>> >>> Will allocate and add "struct phdr_data" instances to the >>> kcore_copy_info->phdrs list, so I propose, to follow convention used >>> elsewhere in tools/perf/ that you rename kcore_copy__map() to >>> >>> kcore_copy_info__addnew(kci, fields) >>> >>> I would do it as: >>> >>> struct phdr_data *phdr_data__new(fields) >>> { >>> return zalloc() + init fields; >>> } >>> >>> struct phdr_data *kcore_copy_info__addnew(kci, fields) >>> { >>> struct phdr_data *pd = phdr_data__new(fields); >>> >>> if (pd) >>> list_add(&pd->list, &kci->phdrs) >>> } >>> >>> Also please rename pd->list to pd->node, to clarify that it is a node in >>> some list, not a list. >>> >>> The commit log list then could reflect that somehow, with something >>> around: >>> >>> ---------------------- >>> >>> Move ->kernel_map and ->modules_map to newly allocated entries in the >>> ->phdrs list. >>> >>> ---------------------- >>> >>> wdyt? >> >> I have done the changes but still have kcore_copy__map() calling >> kcore_copy_info__addnew(). The changes have been pushed to the same branch. > > Thanks! I'll process a perf/urgent round and then go over your updated > tree to get it tested on a skylake machine and merged to perf/core,
I changed some terminology 'x86_64 KPTI' -> 'x86 PTI' as requested by Ingo, and pushed to a new branch perf-tools-kpti-v0.
http://git.infradead.org/users/ahunter/linux-perf.git/shortlog/refs/heads/perf-tools-kpti-v0
| |