On Wed, Aug 7, 2024 at 10:11 AM Oleg Nesterov <oleg@xxxxxxxxxx> wrote: > > On 08/07, Andrii Nakryiko wrote: > > > > Yes, I was waiting for more of Peter's comments, but I guess I'll just > > send a v2 today. > > OK, > > > I'll probably include the SRCU+timeout logic for > > return_instances, and maybe lockless VMA parts as well. > > Well, feel free to do what you think right, but perhaps it would be > better to push this series first? at least 1-4. Ok, I can send those first 4 patches first and hopefully we can land them soon and move to the next part. I just also wrote up details about that crash in rb_find_rcu(). > > As for lockless VMA. To me this needs more discussions. I didn't read We are still discussing, feel free to join the conversation. > your conversation with Peter and Suren carefully, but I too have some > concerns. Most probably I am wrong, and until I saw this thread I didn't > even know that vm_area_free() uses call_rcu() if CONFIG_PER_VMA_LOCK, > but still. > > > > As for 8/8 - I leave it to you and Peter. I'd prefer SRCU though ;) > > > > Honestly curious, why the preference? > > Well, you can safely ignore me, but since you have asked ;) > > I understand what SRCU does, and years ago I even understood (I hope) > the implementation. More or less the same for rcu_tasks. But as for > the _trace flavour, I simply fail to understand its semantics. Ok, I won't try to repeat Paul's explanations. If you are curious you can find them in comments to my previous batch register/unregister API patches. > > > BTW, while you are here :) What can you say about > > current->sighand->siglock use in handle_singlestep()? > > It should die, and this looks simple. I disagree with the patches > from Liao, see the > https://lore.kernel.org/all/20240801082407.1618451-1-liaochang1@xxxxxxxxxx/ > thread, but I agree with the intent. I wasn't aware of this patch, thanks for mentioning it. Strange that me or at least bpf@xxxxxxxxxxxxxxx wasn't CC'ed. Liao, please cc bpf@ mailing list for future patches like that. > > IMO, we need a simple "bool restore_sigpending" in uprobe_task, it will make the > necessary changes really simple. The simpler the better, I can't comment on correctness as I don't understand the logic well enough. Are you going to send a patch with your bool flag proposal? > > (To clarify. In fact I think that a new TIF_ or even PF_ flag makes more sense, > afaics it can have more users. But I don't think that uprobes can provide enough > justification for that right now) > > Oleg. >
