aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMengting Zhang <zhangmengting@huawei.com>2017-12-13 02:01:53 -0500
committerArnaldo Carvalho de Melo <acme@redhat.com>2017-12-27 10:15:58 -0500
commitca8000684ec4e66f965e1f9547a3c6cb834154ca (patch)
tree62a5e394e5569cbc5690d1412f2018980fb876c8
parenta9a3f1d18a6c9ccf89728e23474645aa91e2f4f1 (diff)
perf evsel: Enable ignore_missing_thread for pid option
While monitoring a multithread process with pid option, perf sometimes may return sys_perf_event_open failure with 3(No such process) if any of the process's threads die before we open the event. However, we want perf continue monitoring the remaining threads and do not exit with error. Here, the patch enables perf_evsel::ignore_missing_thread for -p option to ignore complete failure if any of threads die before we open the event. But it may still return sys_perf_event_open failure with 22(Invalid) if we monitors several event groups. sys_perf_event_open: pid 28960 cpu 40 group_fd 118202 flags 0x8 sys_perf_event_open: pid 28961 cpu 40 group_fd 118203 flags 0x8 WARNING: Ignored open failure for pid 28962 sys_perf_event_open: pid 28962 cpu 40 group_fd [118203] flags 0x8 sys_perf_event_open failed, error -22 That is because when we ignore a missing thread, we change the thread_idx without dealing with its fds, FD(evsel, cpu, thread). Then get_group_fd() may return a wrong group_fd for the next thread and sys_perf_event_open() return with 22. sys_perf_event_open(){ ... if (group_fd != -1) perf_fget_light()//to get corresponding group_leader by group_fd ... if (group_leader) if (group_leader->ctx->task != ctx->task)//should on the same task goto err_context ... } This patch also fixes this bug by introducing perf_evsel__remove_fd() and update_fds to allow removing fds for the missing thread. Changes since v1: - Change group_fd__remove() into a more genetic way without changing code logic - Remove redundant condition Changes since v2: - Use a proper function name and add some comment. - Multiline comment style fixes. Committer testing: Before this patch the recently added 'perf stat --per-thread' for system wide counting would race while enumerating all threads using /proc: [root@jouet ~]# perf stat --per-thread failed to parse CPUs map: No such file or directory Usage: perf stat [<options>] [<command>] -C, --cpu <cpu> list of cpus to monitor in system-wide -a, --all-cpus system-wide collection from all CPUs [root@jouet ~]# perf stat --per-thread failed to parse CPUs map: No such file or directory Usage: perf stat [<options>] [<command>] -C, --cpu <cpu> list of cpus to monitor in system-wide -a, --all-cpus system-wide collection from all CPUs [root@jouet ~]# When, say, the kernel was being built, so lots of shortlived threads, after this patch this doesn't happen. Signed-off-by: Mengting Zhang <zhangmengting@huawei.com> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com> Acked-by: Jiri Olsa <jolsa@redhat.com> Cc: Cheng Jian <cj.chengjian@huawei.com> Cc: Li Bin <huawei.libin@huawei.com> Cc: Wang Nan <wangnan0@huawei.com> Link: http://lkml.kernel.org/r/1513148513-6974-1-git-send-email-zhangmengting@huawei.com [ Remove one use 'evlist' alias variable ] Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
-rw-r--r--tools/perf/builtin-record.c4
-rw-r--r--tools/perf/util/evsel.c47
2 files changed, 47 insertions, 4 deletions
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 98da8cb8de93..50385d89c497 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1804,8 +1804,8 @@ int cmd_record(int argc, const char **argv)
1804 goto out; 1804 goto out;
1805 } 1805 }
1806 1806
1807 /* Enable ignoring missing threads when -u option is defined. */ 1807 /* Enable ignoring missing threads when -u/-p option is defined. */
1808 rec->opts.ignore_missing_thread = rec->opts.target.uid != UINT_MAX; 1808 rec->opts.ignore_missing_thread = rec->opts.target.uid != UINT_MAX || rec->opts.target.pid;
1809 1809
1810 err = -ENOMEM; 1810 err = -ENOMEM;
1811 if (perf_evlist__create_maps(rec->evlist, &rec->opts.target) < 0) 1811 if (perf_evlist__create_maps(rec->evlist, &rec->opts.target) < 0)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 1cf044cbae36..a4d256ea0dc4 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1599,10 +1599,46 @@ static int __open_attr__fprintf(FILE *fp, const char *name, const char *val,
1599 return fprintf(fp, " %-32s %s\n", name, val); 1599 return fprintf(fp, " %-32s %s\n", name, val);
1600} 1600}
1601 1601
1602static void perf_evsel__remove_fd(struct perf_evsel *pos,
1603 int nr_cpus, int nr_threads,
1604 int thread_idx)
1605{
1606 for (int cpu = 0; cpu < nr_cpus; cpu++)
1607 for (int thread = thread_idx; thread < nr_threads - 1; thread++)
1608 FD(pos, cpu, thread) = FD(pos, cpu, thread + 1);
1609}
1610
1611static int update_fds(struct perf_evsel *evsel,
1612 int nr_cpus, int cpu_idx,
1613 int nr_threads, int thread_idx)
1614{
1615 struct perf_evsel *pos;
1616
1617 if (cpu_idx >= nr_cpus || thread_idx >= nr_threads)
1618 return -EINVAL;
1619
1620 evlist__for_each_entry(evsel->evlist, pos) {
1621 nr_cpus = pos != evsel ? nr_cpus : cpu_idx;
1622
1623 perf_evsel__remove_fd(pos, nr_cpus, nr_threads, thread_idx);
1624
1625 /*
1626 * Since fds for next evsel has not been created,
1627 * there is no need to iterate whole event list.
1628 */
1629 if (pos == evsel)
1630 break;
1631 }
1632 return 0;
1633}
1634
1602static bool ignore_missing_thread(struct perf_evsel *evsel, 1635static bool ignore_missing_thread(struct perf_evsel *evsel,
1636 int nr_cpus, int cpu,
1603 struct thread_map *threads, 1637 struct thread_map *threads,
1604 int thread, int err) 1638 int thread, int err)
1605{ 1639{
1640 pid_t ignore_pid = thread_map__pid(threads, thread);
1641
1606 if (!evsel->ignore_missing_thread) 1642 if (!evsel->ignore_missing_thread)
1607 return false; 1643 return false;
1608 1644
@@ -1618,11 +1654,18 @@ static bool ignore_missing_thread(struct perf_evsel *evsel,
1618 if (threads->nr == 1) 1654 if (threads->nr == 1)
1619 return false; 1655 return false;
1620 1656
1657 /*
1658 * We should remove fd for missing_thread first
1659 * because thread_map__remove() will decrease threads->nr.
1660 */
1661 if (update_fds(evsel, nr_cpus, cpu, threads->nr, thread))
1662 return false;
1663
1621 if (thread_map__remove(threads, thread)) 1664 if (thread_map__remove(threads, thread))
1622 return false; 1665 return false;
1623 1666
1624 pr_warning("WARNING: Ignored open failure for pid %d\n", 1667 pr_warning("WARNING: Ignored open failure for pid %d\n",
1625 thread_map__pid(threads, thread)); 1668 ignore_pid);
1626 return true; 1669 return true;
1627} 1670}
1628 1671
@@ -1727,7 +1770,7 @@ retry_open:
1727 if (fd < 0) { 1770 if (fd < 0) {
1728 err = -errno; 1771 err = -errno;
1729 1772
1730 if (ignore_missing_thread(evsel, threads, thread, err)) { 1773 if (ignore_missing_thread(evsel, cpus->nr, cpu, threads, thread, err)) {
1731 /* 1774 /*
1732 * We just removed 1 thread, so take a step 1775 * We just removed 1 thread, so take a step
1733 * back on thread index and lower the upper 1776 * back on thread index and lower the upper