On 08/30, Andrii Nakryiko wrote: > Andrii, let me reply to your email "out of order". First of all: > Can we please let me land these patches first? It's been a while. I > don't think anything is really broken with the logic. OK, agreed. I'll probably write another email (too late for me today), but I agree that "avoid register_rwsem in handler_chain" is obviously a good goal, lets discuss the possible cleanups or even fixlets later, when this series is already applied. > On Fri, Aug 30, 2024 at 7:33 AM Oleg Nesterov <oleg@xxxxxxxxxx> wrote: > > > > No, I think you found a problem. UPROBE_HANDLER_REMOVE can be lost if > > uc->filter == NULL of if it returns true. See another reply I sent a > > minute ago. > > > > For better or worse, but I think there is (or has to be) and implicit > contract that if uprobe (or uretprobe for that matter as well, but > that's a separate issue) handler can return UPROBE_HANDLER_REMOVE, > then it *has to* also provide filter. IOW, uc->handler and uc->filter must be consistent. But the current API doesn't require this contract, so this patch adds a difference which I didn't notice when I reviewed this change. (In fact I noticed the difference, but I thought that it should be fine). > If it doesn't provide filter > callback, it doesn't care about PID filtering and thus can't and > shouldn't cause unregistration. At first glance I disagree, but see above. > > I think the fix is simple, plus we need to cleanup this logic anyway, > > I'll try to send some code on Monday. Damn I am stupid. Nothing new ;) The "simple" fix I had in mind can't work. But we can do other things which we can discuss later. Oleg.
