Messages in this thread | | | From | Dmitry Vyukov <> | Date | Sun, 31 Jan 2021 11:28:32 +0100 | Subject | Re: Process-wide watchpoints |
| |
On Sun, Jan 31, 2021 at 11:04 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > On Thu, Nov 12, 2020 at 11:43 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > > > for sampling race detection), > > > > number of threads in the process can be up to, say, ~~10K and the > > > > watchpoint is intended to be set for a very brief period of time > > > > (~~few ms). > > > > > > Performance is a consideration here, doing lots of IPIs in such a short > > > window, on potentially large machines is a DoS risk. > > > > > > > This can be done today with both perf_event_open and ptrace. > > > > However, the problem is that both APIs work on a single thread level > > > > (? perf_event_open can be inherited by children, but not for existing > > > > siblings). So doing this would require iterating over, say, 10K > > > > > > One way would be to create the event before the process starts spawning > > > threads and keeping it disabled. Then every thread will inherit it, but > > > it'll be inactive. > > > > > > > I see at least one potential problem: what do we do if some sibling > > > > thread already has all 4 watchpoints consumed? > > > > > > That would be immediately avoided by this, since it will have the > > > watchpoint reserved per inheriting the event. > > > > > > Then you can do ioctl(PERF_EVENT_IOC_{MODIFY_ATTRIBUTES,ENABLE,DISABLE}) > > > to update the watch location and enable/disable it. This _will_ indeed > > > result in a shitload of IPIs if the threads are active, but it should > > > work. > > > > Aha! That's the possibility I missed. > > We will try to prototype this and get back with more questions if/when > > we have them. > > Thanks! > > Hi Peter, > > I've tested this approach and it works, but only in half. > PERF_EVENT_IOC_{ENABLE,DISABLE} work as advertised. > However, PERF_EVENT_IOC_MODIFY_ATTRIBUTES does not work for inherited > child events. > Does something like this make any sense to you? Are you willing to > accept such change? > > diff --git a/kernel/events/core.c b/kernel/events/core.c > index 55d18791a72d..f6974807a32c 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -3174,7 +3174,7 @@ int perf_event_refresh(struct perf_event *event, > int refresh) > } > EXPORT_SYMBOL_GPL(perf_event_refresh); > > -static int perf_event_modify_breakpoint(struct perf_event *bp, > +static int _perf_event_modify_breakpoint(struct perf_event *bp, > struct perf_event_attr *attr) > { > int err; > @@ -3189,6 +3189,28 @@ static int perf_event_modify_breakpoint(struct > perf_event *bp, > return err; > } > > +static int perf_event_modify_breakpoint(struct perf_event *bp, > + struct perf_event_attr *attr) > +{ > + struct perf_event *child; > + int err; > + > + WARN_ON_ONCE(bp->ctx->parent_ctx); > + > + mutex_lock(&bp->child_mutex); > + err = _perf_event_modify_breakpoint(bp, attr); > + if (err) > + goto unlock; > + list_for_each_entry(child, &bp->child_list, child_list) { > + err = _perf_event_modify_breakpoint(child, attr); > + if (err) > + goto unlock; > + } > +unlock: > + mutex_unlock(&bp->child_mutex); > + return err; > +} > + > static int perf_event_modify_attr(struct perf_event *event, > struct perf_event_attr *attr)
Not directly related to the above question, but related to my use case. Could we extend bpf_perf_event_data with some more data re breakpoint events?
struct bpf_perf_event_data { bpf_user_pt_regs_t regs; __u64 sample_period; __u64 addr; };
Ideally, I would like to have an actual access address, size and read/write type (may not match bp addr/size). Is that info easily available at the point of bpf hook call? Or, if that's not available at least breakpoint bp_type/bp_size.
Is it correct that we can materialize in bpf_perf_event_data anything that's available in bpf_perf_event_data_kern (if it makes sense in the public interface of course)?
struct bpf_perf_event_data_kern { bpf_user_pt_regs_t *regs; struct perf_sample_data *data; struct perf_event *event; };
Unfortunately I don't see perf_event_attr.bp_type/bp_size stored/accessible anywhere in bpf_perf_event_data_kern. What would be the right way to expose them in bpf_perf_event_data?
Thanks
| |