Re: [PATCH v5 3/6] signal: Add calculate_sigpending()

[Eric W. Biederman]

<wen.yang99@xxxxxxxxxx> writes:

> EricW.Biederman <ebiederm@xxxxxxxxxxxx> wrote:
>> Add a function calculate_sigpending to test to see if any signals are
>> pending for a new task immediately following fork. Signals have to
>> happen either before or after fork. Today our practice is to push
>> all of the signals to before the fork, but that has the downside that
>> frequent or periodic signals can make fork take much much longer than
>> normal or prevent fork from completing entirely.
>> 
>
>> + calculate_sigpending();
>> }
>> /*
>> diff --git a/kernel/signal.c b/kernel/signal.c
>> index dddbea558455..1e06f1eba363 100644
>> --- a/kernel/signal.c
>> +++ b/kernel/signal.c
>> @@ -172,6 +172,17 @@ void recalc_sigpending(void)
>> }
>> +void calculate_sigpending(void)
>> +{
>> + /* Have any signals or users of TIF_SIGPENDING been delayed
>> + * until after fork?
>> + */
>> + spin_lock_irq(&current->sighand->siglock);
>> + set_tsk_thread_flag(current, TIF_SIGPENDING);
>> + recalc_sigpending();
>> + spin_unlock_irq(&current->sighand->siglock);
>> +}
>> +
>
> The new function calculate_sigpending is similar to  recalc_sigpending,
> but recalc_sigpending has no spin_lock_irq(&current->sighand->siglock) in it.
> This gives recalc_sigpending more flexibility, 
> we may use spin_lock_irq or spin_lock_irqsave before recalc_sigpending .
> eg: 
>
> static int autofs4_write(struct autofs_sb_info *sbi,
>              struct file *file, const void *addr, int bytes)
> {
>   ...
>         spin_lock_irqsave(&current->sighand->siglock, flags);
>         sigdelset(&current->pending.signal, SIGPIPE);
>         recalc_sigpending();
>         spin_unlock_irqrestore(&current->sighand->siglock, flags);
>   ... 
> }
>
> or:
> void kernel_sigaction(int sig, __sighandler_t action)
> {
>     spin_lock_irq(&current->sighand->siglock);
> ...
>         recalc_sigpending();
> ...   
>     spin_unlock_irq(&current->sighand->siglock);
> }
>
>
> But calculate_sigpending is currently hardwired to call spin_lock_irq.

calculate_sigpending really only exists to keep the code comprehensible.

It is only ever expected to be called in exactly one place so the lack
of flexibility should not be a problem.  Further the use of irqsave
is discouraged unless it is necessary.

The irqsave in autofs_write actually looks like a misfeature.  We take
a mutex a few lines earlier, so we know that irqs are enabled.  Saving
and restoring them is uncessary work.  Further unless I am missing
something that code path should be calling kernel_dequeue_signal, to
ensure that any siginfo associated with that SIGPIPE gets dequeued.

Eric