On Thu, 27 Jun 2024 09:47:10 -0700 Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > On Thu, Jun 27, 2024 at 6:04 AM Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote: > > > > On Mon, 24 Jun 2024 17:21:38 -0700 > > Andrii Nakryiko <andrii@xxxxxxxxxx> wrote: > > > > > -static int __uprobe_register(struct inode *inode, loff_t offset, > > > - loff_t ref_ctr_offset, struct uprobe_consumer *uc) > > > +int uprobe_register_batch(struct inode *inode, int cnt, > > > + uprobe_consumer_fn get_uprobe_consumer, void *ctx) > > > > Is this interface just for avoiding memory allocation? Can't we just > > allocate a temporary array of *uprobe_consumer instead? > > Yes, exactly, to avoid the need for allocating another array that > would just contain pointers to uprobe_consumer. Consumers would never > just have an array of `struct uprobe_consumer *`, because > uprobe_consumer struct is embedded in some other struct, so the array > interface isn't the most convenient. OK, I understand it. > > If you feel strongly, I can do an array, but this necessitates > allocating an extra array *and keeping it* for the entire duration of > BPF multi-uprobe link (attachment) existence, so it feels like a > waste. This is because we don't want to do anything that can fail in > the detachment logic (so no temporary array allocation there). No need to change it, that sounds reasonable. > > Anyways, let me know how you feel about keeping this callback. IMHO, maybe the interface function is better to change to `uprobe_consumer *next_uprobe_consumer(void **data)`. If caller side uses a linked list of structure, index access will need to follow the list every time. Thank you, > > > > > Thank you, > > > > -- > > Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx> -- Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
