Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

Hi Tycho!

I'm glad you inherited this :).

Oh, I wasn't suggesting that it was about killable vs. unkillable.

Couple of things: 1. Should prepare_to_wait_event check if the task is in PF_EXITING, and if so, refuse to wait unless a specific flag is provided? I'd be curious if you just add a kprobe to prepare_to_wait_event that checks for PF_EXITING, how many cases are valid?

2. Following this:

  zap_pid_ns_processes ->
     __fatal_signal_pending(task)
     group_send_sig_info
       do_send_sig_info
         send_signal_locked
           __send_signal_locked -> (jump to out_set)
             sigaddset // It has the pending signal here
             ....
             complete_signal


Shouldn't it wake up, even if in its in PF_EXITING, that would trigger as reassessment of the condition, and then the `__fatal_signal_pending` check would make it return -ERESTARTSYS.

One note, in the post:

  # grep Pnd /proc/1544574/status
  SigPnd: 0000000000000000
  ShdPnd: 0000000000000100

> Viewing process status this way, you can see 0x100 (i.e. the 9th bit is set) under SigPnd, which is the signal number corresponding to SIGKILL.

Shouldn't it be "ShdPnd"?



> Couple of things: 1. Should prepare_to_wait_event check if the task is in PF_EXITING, and if so, refuse to wait unless a specific flag is provided? I'd be curious if you just add a kprobe to prepare_to_wait_event that checks for PF_EXITING, how many cases are valid?

I would argue they're all invalid if PF_EXITING is present. Maybe I should send a patch to WARN() and see how much I get yelled at.

> Shouldn't it wake up, even if in its in PF_EXITING, that would trigger as reassessment of the condition, and then the `__fatal_signal_pending` check would make it return -ERESTARTSYS.

No, because the signal doesn't get delivered by complete_signal(). wants_signal() returns false if PF_EXITING is set. (Another maybe-interesting thing would be to just delete that check.) Or am I misunderstanding you?

> Shouldn't it be "ShdPnd"

derp, fixed, thanks.


> Or am I misunderstanding you?

Oh, I see, you're suggesting exactly,

> (Another maybe-interesting thing would be to just delete that check.)

I agree.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: