Messages in this thread | | | Date | Thu, 28 Apr 2016 19:09:02 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v2 5/7] perf: Introduce address range filtering |
| |
On Wed, Apr 27, 2016 at 06:44:46PM +0300, Alexander Shishkin wrote: > +/* > + * In order to contain the amount of racy and tricky in the address filter > + * configuration management, it is a two part process: > + * > + * (p1) when userspace mappings change as a result of (1) or (2) or (3) below, > + * we update the addresses of corresponding vmas in > + * event::addr_filters_offs array and bump the event::addr_filters_gen; > + * (p2) when an event is scheduled in (pmu::add), it calls > + * perf_event_addr_filters_sync() which calls pmu::addr_filters_sync() > + * if the generation has changed since the previous call. > + * > + * If (p1) happens while the event is active, we restart it to force (p2).
Tiny nit; restart only does ::stop(),::start(), which doesn't go through ::add().
> + * > + * (1) perf_addr_filters_apply(): adjusting filters' offsets based on > + * pre-existing mappings, called once when new filters arrive via SET_FILTER > + * ioctl; > + * (2) perf_addr_filters_adjust(): adjusting filters' offsets based on newly > + * registered mapping, called for every new mmap(), with mm::mmap_sem down > + * for reading; > + * (3) perf_event_addr_filters_exec(): clearing filters' offsets in the process > + * of exec. > + */
> +static unsigned long perf_addr_filter_apply(struct perf_addr_filter *filter, > + struct mm_struct *mm) > +{ > + struct vm_area_struct *vma; > + > + for (vma = mm->mmap; vma->vm_next; vma = vma->vm_next) {
Doesn't this miss the very last vma? That is, I was expecting:
for (vma = mm->mmap; vma; vma = vma->vm_next) {
> + struct file *file = vma->vm_file; > + unsigned long off = vma->vm_pgoff << PAGE_SHIFT; > + unsigned long vma_size = vma->vm_end - vma->vm_start; > + > + if (!file) > + continue; > + > + if (!perf_addr_filter_match(filter, file, off, vma_size)) > + continue; > + > + return vma->vm_start; > + } > + > + return 0; > +}
| |