| 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
| ||
|
Date: Wed, 14 Mar 2018 20:58:33 +0000
From: John Garry <john.garry@...wei.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
CC: <peterz@...radead.org>, <mingo@...hat.com>,
<alexander.shishkin@...ux.intel.com>, <jolsa@...hat.com>,
<namhyung@...nel.org>, <ak@...ux.intel.com>, <wcohen@...hat.com>,
<sukadev@...ux.vnet.ibm.com>, <linux-kernel@...r.kernel.org>,
<linuxarm@...wei.com>
Subject: Re: [PATCH] perf vendor events: fix processing for xfs
On 14/03/2018 20:26, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 14, 2018 at 07:39:25PM +0000, John Garry escreveu:
>> On 14/03/2018 18:53, Arnaldo Carvalho de Melo wrote:
>>> Em Thu, Mar 15, 2018 at 01:10:52AM +0800, John Garry escreveu:
>>>> In the recently introduced support for vendor subdirectory,
>>>> the checking for directory entries under xfs (or any other fs
>>>> which does not support dirent.d_type) is missing the check
>>>> for links for current and parent directory. This can result
>>>> in a broken pmu_events.c being generated.
>>>>
>>>> Fix this by adding the appropriate check in is_leaf_dir().
>>>
>>> So I'll lookup the patch that introduced the patch and squash this one
>>> with it, so that we don't break 'git bisect' on ppc.
>>
>> Right, so it's going to be "perf vendor events: add support for pmu events
>> vendor subdirectory".
>>
>> BTW, I don't think it's specifically ppc which was broken, but just when
>> building from xfs.
>>
>> Much appreciated,
>> John
>
> This is the ammended cset, and yes, I mentioned just XFS not ppc, added
> Sukadev to the CC list and the link to your patch that he tested. Also
> stamped his Tested-by there.
>
> This will all go again the container build test set before a new pull
> req to Ingo, where I'll avoid sending the previous 31 patches to the
> mailing list, just the ones that came after it, as perf/core keeps
> growing :-)
It looks ok,
Thanks a lot,
John
>
> - Arnaldo
>
> commit d24081387a00d2e10789eb012429144c549598be
> Author: John Garry <john.garry@...wei.com>
> Date: Thu Mar 8 18:58:29 2018 +0800
>
> perf vendor events: Add support for pmu events vendor subdirectory
>
> For some architectures (like arm), it is required to support a vendor
> subdirectory and not locate all the JSONs for a specific vendor in the
> same folder.
>
> This is because all the events for the same vendor will be placed in the
> same pmu events table, which may cause conflict. This conflict would be
> in the instance that a vendor's custom implemented events do have the
> same meaning on different platforms, so events in the pmu table would
> conflict. In addition, per list command may show events which are not
> even supported for a given platform.
>
> This patch adds support for a arch/vendor/platform directory hierarchy,
> while maintaining backwards-compatibility for existing arch/platform
> structure. In this, each platform would always have its own pmu events
> table.
>
> In generated file pmu_events.c, each platform table name is in the
> format pme{_vendor}_platform, like this:
>
> struct pmu_events_map pmu_events_map[] = {
> {
> .cpuid = "0x00000000420f5160",
> .version = "v1",
> .type = "core",
> .table = pme_cavium_thunderx2
> },
> {
> .cpuid = 0,
> .version = 0,
> .type = 0,
> .table = 0,
> },
> };
>
> Signed-off-by: John Garry <john.garry@...wei.com>
> Acked-by: Jiri Olsa <jolsa@...nel.org>
> Tested-by: Sukadev Bhattiprolu <sukadev@...ux.vnet.ibm.com>
> Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
> Cc: Andi Kleen <ak@...ux.intel.com>
> Cc: Ganapatrao Kulkarni <ganapatrao.kulkarni@...ium.com>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Shaokun Zhang <zhangshaokun@...ilicon.com>
> Cc: Sukadev Bhattiprolu <sukadev@...ux.vnet.ibm.com>
> Cc: Will Deacon <will.deacon@....com>
> Cc: William Cohen <wcohen@...hat.com>
> Cc: linux-arm-kernel@...ts.infradead.org
> Cc: linuxarm@...wei.com
> Link: http://lkml.kernel.org/r/1520506716-197429-5-git-send-email-john.garry@huawei.com
> Link: http://lkml.kernel.org/r/1521047452-28565-1-git-send-email-john.garry@huawei.com
> [ Add missing limits.h include, fixing the build on at least all Alpine Linux versions tested (3.4 to 3.7 + edge), ]
> [ Applied a patch to fix reading ./.. directories in XFS, see second Link tag ]
> Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
>
> diff --git a/tools/perf/pmu-events/README b/tools/perf/pmu-events/README
> index 2407abc1d441..655286ff8767 100644
> --- a/tools/perf/pmu-events/README
> +++ b/tools/perf/pmu-events/README
> @@ -28,6 +28,10 @@ sub directory. Thus for the Silvermont X86 CPU:
> Cache.json Memory.json Virtual-Memory.json
> Frontend.json Pipeline.json
>
> +The JSONs folder for a CPU model/family may be placed in the root arch
> +folder, or may be placed in a vendor sub-folder under the arch folder
> +for instances where the arch and vendor are not the same.
> +
> Using the JSON files and the mapfile, 'jevents' generates the C source file,
> 'pmu-events.c', which encodes the two sets of tables:
>
> diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
> index 1d02fafdc34d..0981d313064f 100644
> --- a/tools/perf/pmu-events/jevents.c
> +++ b/tools/perf/pmu-events/jevents.c
> @@ -39,6 +39,7 @@
> #include <unistd.h>
> #include <stdarg.h>
> #include <libgen.h>
> +#include <limits.h>
> #include <dirent.h>
> #include <sys/time.h> /* getrlimit */
> #include <sys/resource.h> /* getrlimit */
> @@ -572,7 +573,7 @@ static char *file_name_to_table_name(char *fname)
> * Derive rest of table name from basename of the JSON file,
> * replacing hyphens and stripping out .json suffix.
> */
> - n = asprintf(&tblname, "pme_%s", basename(fname));
> + n = asprintf(&tblname, "pme_%s", fname);
> if (n < 0) {
> pr_info("%s: asprintf() error %s for file %s\n", prog,
> strerror(errno), fname);
> @@ -582,7 +583,7 @@ static char *file_name_to_table_name(char *fname)
> for (i = 0; i < strlen(tblname); i++) {
> c = tblname[i];
>
> - if (c == '-')
> + if (c == '-' || c == '/')
> tblname[i] = '_';
> else if (c == '.') {
> tblname[i] = '\0';
> @@ -739,25 +740,80 @@ static int get_maxfds(void)
> static FILE *eventsfp;
> static char *mapfile;
>
> +static int is_leaf_dir(const char *fpath)
> +{
> + DIR *d;
> + struct dirent *dir;
> + int res = 1;
> +
> + d = opendir(fpath);
> + if (!d)
> + return 0;
> +
> + while ((dir = readdir(d)) != NULL) {
> + if (!strcmp(dir->d_name, ".") || !strcmp(dir->d_name, ".."))
> + continue;
> +
> + if (dir->d_type == DT_DIR) {
> + res = 0;
> + break;
> + } else if (dir->d_type == DT_UNKNOWN) {
> + char path[PATH_MAX];
> + struct stat st;
> +
> + sprintf(path, "%s/%s", fpath, dir->d_name);
> + if (stat(path, &st))
> + break;
> +
> + if (S_ISDIR(st.st_mode)) {
> + res = 0;
> + break;
> + }
> + }
> + }
> +
> + closedir(d);
> +
> + return res;
> +}
> +
> static int process_one_file(const char *fpath, const struct stat *sb,
> int typeflag, struct FTW *ftwbuf)
> {
> - char *tblname, *bname = (char *) fpath + ftwbuf->base;
> + char *tblname, *bname;
> int is_dir = typeflag == FTW_D;
> int is_file = typeflag == FTW_F;
> int level = ftwbuf->level;
> int err = 0;
>
> + if (level == 2 && is_dir) {
> + /*
> + * For level 2 directory, bname will include parent name,
> + * like vendor/platform. So search back from platform dir
> + * to find this.
> + */
> + bname = (char *) fpath + ftwbuf->base - 2;
> + for (;;) {
> + if (*bname == '/')
> + break;
> + bname--;
> + }
> + bname++;
> + } else
> + bname = (char *) fpath + ftwbuf->base;
> +
> pr_debug("%s %d %7jd %-20s %s\n",
> is_file ? "f" : is_dir ? "d" : "x",
> level, sb->st_size, bname, fpath);
>
> - /* base dir */
> - if (level == 0)
> + /* base dir or too deep */
> + if (level == 0 || level > 3)
> return 0;
>
> +
> /* model directory, reset topic */
> - if (level == 1 && is_dir) {
> + if ((level == 1 && is_dir && is_leaf_dir(fpath)) ||
> + (level == 2 && is_dir)) {
> if (close_table)
> print_events_table_suffix(eventsfp);
>
>
> .
>
Powered by blists - more mailing lists