在 2024/8/8 1:31, Andrii Nakryiko 写道: > 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. OK, sorry about that. > >> >> IMO, we need a simple "bool restore_sigpending" in uprobe_task, it will make the >> necessary changes really simple. > [...] >> >> (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) I also face the same choice when Oleg suggested me to add new flag to track the denied flag, due to I haven't encountered scenarios outside of uprobe that would deny signal, so I'm not confident of introduce new TIF_ flag without a fully understanding of potential potential impacts. >> >> Oleg. >> -- BR Liao, Chang
