Re: [PATCH bpf-next 1/2] bpf: add bpf_send_signal_thread() helper

[Yonghong Song <yhs@fb.com>]

On 1/13/20 4:58 PM, Andrii Nakryiko wrote:
> On Thu, Jan 9, 2020 at 5:16 PM Yonghong Song <yhs@fb.com> wrote:
>>
>> Commit 8b401f9ed244 ("bpf: implement bpf_send_signal() helper")
>> added helper bpf_send_signal() which permits bpf program to
>> send a signal to the current process.
>>
>> We found a use case where sending the signal to the current
>> thread is more preferable.
>>    - A bpf program will collect the stack trace and then
>>      send signal to the user application.
>>    - The user application will add some thread specific
>>      information to the just collected stack trace for
>>      later analysis.
>>
>> If bpf_send_signal() is used, user application will need
>> to check whether the thread receiving the signal matches
>> the thread collecting the stack by checking thread id.
>> If not, it will need to send signal to another thread
>> through pthread_kill().
>>
>> This patch proposed a new helper bpf_send_signal_thread(),
>> which sends the signal to the current thread. This way,
>> user space is guaranteed that bpf_program execution context
>> and user space signal handling context are the same thread.
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>>   include/uapi/linux/bpf.h | 18 ++++++++++++++++--
>>   kernel/trace/bpf_trace.c | 27 ++++++++++++++++++++++++---
>>   2 files changed, 40 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 52966e758fe5..3320f8bdfe7e 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -2714,7 +2714,7 @@ union bpf_attr {
>>    *
>>    * int bpf_send_signal(u32 sig)
>>    *     Description
>> - *             Send signal *sig* to the current task.
>> + *             Send signal *sig* to the process of the current task.
>>    *     Return
>>    *             0 on success or successfully queued.
>>    *
>> @@ -2850,6 +2850,19 @@ union bpf_attr {
>>    *     Return
>>    *             0 on success, or a negative error in case of failure.
>>    *
>> + * int bpf_send_signal_thread(u32 sig)
>> + *     Description
>> + *             Send signal *sig* to the current task.
> 
> 
> This all makes sense and looks good, but I think it's very unclear why
> the distinction between sending signal to process vs thread. Could you
> extend bpf_send_signal and bpf_send_signal_thread descriptions
> explaining the difference (e.g., that, according to POSIX, when
> sending signal to a process, any thread within that process can get
> signal delivered, while sending to a specific thread will ensure that
> that specific thread will receive desired signal).

Sounds good. Will send v2 with better descriptions later.

> 
> [...]
>