On Thu, May 15, 2014 at 06:08:30PM +0300, Alexander Shishkin wrote:
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -252,6 +252,19 @@ struct pmu {
> * flush branch stack on context-switches (needed in cpu-wide mode)
> */
> void (*flush_branch_stack) (void);
> +
> + /*
> + * Allocate AUX space buffer: return an array of @nr_pages pages to be
> + * mapped to userspace that will also be passed to ->free_aux.
> + */
> + void *(*alloc_aux) (int cpu, int nr_pages, bool overwrite,
> + struct perf_event_mmap_page
> *user_page);
> + /* optional */
> +
> + /*
> + * Free AUX buffer
> + */
> + void (*free_aux) (void *aux); /* optional */
> };
I'm not entirely thrilled to expose it to the PMU like this.. I realize
you want this in order to get physically contiguous pages.
Are you aware of allocation constraints for other architectures?
> #define PERF_RECORD_MISC_CPUMODE_MASK (7 << 0)
> @@ -710,6 +726,18 @@ enum perf_event_type {
> */
> PERF_RECORD_MMAP2 = 10,
>
> + /*
> + * Records that new data landed in the AUX buffer part.
> + *
> + * struct {
> + * struct perf_event_header header;
> + *
> + * u64 aux_offset;
> + * u64 aux_size;
> + * };
> + */
> + PERF_RECORD_AUX = 11,
> +
> PERF_RECORD_MAX, /* non-ABI */
> };
Ideally the patch introducing this would also introduce code to generate
these records.
> @@ -4076,7 +4090,63 @@ static int perf_mmap(struct file *file, struct
> vm_area_struct *vma)
> return -EINVAL;
>
> vma_size = vma->vm_end - vma->vm_start;
> - nr_pages = (vma_size / PAGE_SIZE) - 1;
> +
> + if (vma->vm_pgoff == 0) {
> + nr_pages = (vma_size / PAGE_SIZE) - 1;
> + } else {
> + /*
> + * AUX area mapping: if rb->aux_nr_pages != 0, it's already
> + * mapped, all subsequent mappings should have the same size
> + * and offset. Must be above the normal perf buffer.
> + */
> + u64 aux_offset, aux_size;
> +
> + if (!event->rb)
> + return -EINVAL;
> +
> + nr_pages = vma_size / PAGE_SIZE;
> +
> + mutex_lock(&event->mmap_mutex);
> + ret = -EINVAL;
> +
> + rb = event->rb;
> + if (!rb)
> + goto aux_unlock;
> +
> + aux_offset = ACCESS_ONCE(rb->user_page->aux_offset);
> + aux_size = ACCESS_ONCE(rb->user_page->aux_size);
> +
> + if (aux_offset < perf_data_size(rb) + PAGE_SIZE)
> + goto aux_unlock;
> +
> + if (aux_offset != vma->vm_pgoff << PAGE_SHIFT)
> + goto aux_unlock;
> +
> + /* already mapped with a different offset */
> + if (rb_has_aux(rb) && rb->aux_pgoff != vma->vm_pgoff)
> + goto aux_unlock;
> +
> + if (aux_size != vma_size || aux_size != nr_pages * PAGE_SIZE)
> + goto aux_unlock;
> +
> + /* already mapped with a different size */
> + if (rb_has_aux(rb) && rb->aux_nr_pages != nr_pages)
> + goto aux_unlock;
> +
> + if (!atomic_inc_not_zero(&rb->mmap_count))
> + goto aux_unlock;
> +
> + if (rb_has_aux(rb)) {
> + atomic_inc(&rb->aux_mmap_count);
> + ret = 0;
> + goto unlock;
> + }
> +
> + atomic_set(&rb->aux_mmap_count, 1);
> + user_extra = nr_pages;
> +
> + goto accounting;
> + }
That appears to be missing a is_power_of_2(aux_size) check.
The problem with not having that is that since
perf_event_mmap_page::aux_{head,tail} are of Z mod 2^64 but your actual
{head,tail} are of Z mod aux_size, you need aux_size to be a full
divider of 2^64 or otherwise you get wrapping issues at the overflow.
Having it them all 2^n makes the divider trivial.
pgpbhmADBGKGu.pgp
Description: PGP signature
