On Fri, 29 Mar 2024 14:40:55 -0400
Steven Rostedt <rost...@goodmis.org> wrote:
> > +static vm_fault_t tracing_buffers_mmap_fault(struct vm_fault *vmf)
> > +{
> > + return VM_FAULT_SIGBUS;
> > +}
>
> If this is all it does, I don't believe it's needed.
>
> > +
> > +#ifdef CONFIG_TRACER_MAX_TRACE
> > +static int get_snapshot_map(struct trace_array *tr)
> > +{
> > + int err = 0;
> > +
> > + /*
> > + * Called with mmap_lock held. lockdep would be unhappy if we would now
> > + * take trace_types_lock. Instead use the specific
> > + * snapshot_trigger_lock.
> > + */
> > + spin_lock(&tr->snapshot_trigger_lock);
> > +
> > + if (tr->snapshot || tr->mapped == UINT_MAX)
> > + err = -EBUSY;
> > + else
> > + tr->mapped++;
> > +
> > + spin_unlock(&tr->snapshot_trigger_lock);
> > +
> > + /* Wait for update_max_tr() to observe iter->tr->mapped */
> > + if (tr->mapped == 1)
> > + synchronize_rcu();
> > +
> > + return err;
> > +
> > +}
> > +static void put_snapshot_map(struct trace_array *tr)
> > +{
> > + spin_lock(&tr->snapshot_trigger_lock);
> > + if (!WARN_ON(!tr->mapped))
> > + tr->mapped--;
> > + spin_unlock(&tr->snapshot_trigger_lock);
> > +}
> > +#else
> > +static inline int get_snapshot_map(struct trace_array *tr) { return 0; }
> > +static inline void put_snapshot_map(struct trace_array *tr) { }
> > +#endif
> > +
> > +static void tracing_buffers_mmap_close(struct vm_area_struct *vma)
> > +{
> > + struct ftrace_buffer_info *info = vma->vm_file->private_data;
> > + struct trace_iterator *iter = &info->iter;
> > +
> > + WARN_ON(ring_buffer_unmap(iter->array_buffer->buffer, iter->cpu_file));
> > + put_snapshot_map(iter->tr);
> > +}
> > +
> > +static void tracing_buffers_mmap_open(struct vm_area_struct *vma) { }
>
> Same for the open.
>
>
> > +
> > +static const struct vm_operations_struct tracing_buffers_vmops = {
> > + .open = tracing_buffers_mmap_open,
> > + .close = tracing_buffers_mmap_close,
> > + .fault = tracing_buffers_mmap_fault,
> > +};
>
> I replaced this with:
>
> static const struct vm_operations_struct tracing_buffers_vmops = {
> .close = tracing_buffers_mmap_close,
> };
>
> And it appears to work just fine. The mm code handles the NULL cases for
> .open and .fault.
>
> Is there any reason to do something different than the mm defaults?
Hi Vincent,
Do you plan on sending out a v20 series?
-- Steve
