2024-04-10 23:08:29

by Thomas Gleixner

Subject: [patch V2 23/50] signal: Remove task argument from dequeue_signal()

The task pointer which is handed to dequeue_signal() is always current. The
argument along with the first comment about signalfd in that function is
confusing at best. Remove it and use current internally.

Update the stale comment for dequeue_signal() while at it.

Signed-off-by: Thomas Gleixner <[email protected]>
Reviewed-by: Frederic Weisbecker <[email protected]>
---
fs/signalfd.c | 4 ++--
include/linux/sched/signal.h | 5 ++---
kernel/signal.c | 23 ++++++++++-------------
3 files changed, 14 insertions(+), 18 deletions(-)

--- a/fs/signalfd.c
+++ b/fs/signalfd.c
@@ -160,7 +160,7 @@ static ssize_t signalfd_dequeue(struct s
DECLARE_WAITQUEUE(wait, current);

spin_lock_irq(&current->sighand->siglock);
- ret = dequeue_signal(current, &ctx->sigmask, info, &type);
+ ret = dequeue_signal(&ctx->sigmask, info, &type);
switch (ret) {
case 0:
if (!nonblock)
@@ -175,7 +175,7 @@ static ssize_t signalfd_dequeue(struct s
add_wait_queue(&current->sighand->signalfd_wqh, &wait);
for (;;) {
set_current_state(TASK_INTERRUPTIBLE);
- ret = dequeue_signal(current, &ctx->sigmask, info, &type);
+ ret = dequeue_signal(&ctx->sigmask, info, &type);
if (ret != 0)
break;
if (signal_pending(current)) {
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -276,8 +276,7 @@ static inline void signal_set_stop_flags
extern void flush_signals(struct task_struct *);
extern void ignore_signals(struct task_struct *);
extern void flush_signal_handlers(struct task_struct *, int force_default);
-extern int dequeue_signal(struct task_struct *task, sigset_t *mask,
- kernel_siginfo_t *info, enum pid_type *type);
+extern int dequeue_signal(sigset_t *mask, kernel_siginfo_t *info, enum pid_type *type);

static inline int kernel_dequeue_signal(void)
{
@@ -287,7 +286,7 @@ static inline int kernel_dequeue_signal(
int ret;

spin_lock_irq(&task->sighand->siglock);
- ret = dequeue_signal(task, &task->blocked, &__info, &__type);
+ ret = dequeue_signal(&task->blocked, &__info, &__type);
spin_unlock_irq(&task->sighand->siglock);

return ret;
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -618,20 +618,18 @@ static int __dequeue_signal(struct sigpe
}

/*
- * Dequeue a signal and return the element to the caller, which is
- * expected to free it.
- *
- * All callers have to hold the siglock.
+ * Try to dequeue a signal. If a deliverable signal is found fill in the
+ * caller provided siginfo and return the signal number. Otherwise return
+ * 0.
*/
-int dequeue_signal(struct task_struct *tsk, sigset_t *mask,
- kernel_siginfo_t *info, enum pid_type *type)
+int dequeue_signal(sigset_t *mask, kernel_siginfo_t *info, enum pid_type *type)
{
+ struct task_struct *tsk = current;
bool resched_timer = false;
int signr;

- /* We only dequeue private signals from ourselves, we don't let
- * signalfd steal them
- */
+ lockdep_assert_held(&tsk->sighand->siglock);
+
*type = PIDTYPE_PID;
signr = __dequeue_signal(&tsk->pending, mask, info, &resched_timer);
if (!signr) {
@@ -2787,8 +2785,7 @@ bool get_signal(struct ksignal *ksig)
type = PIDTYPE_PID;
signr = dequeue_synchronous_signal(&ksig->info);
if (!signr)
- signr = dequeue_signal(current, &current->blocked,
- &ksig->info, &type);
+ signr = dequeue_signal(&current->blocked, &ksig->info, &type);

if (!signr)
break; /* will return 0 */
@@ -3642,7 +3639,7 @@ static int do_sigtimedwait(const sigset_
signotset(&mask);

spin_lock_irq(&tsk->sighand->siglock);
- sig = dequeue_signal(tsk, &mask, info, &type);
+ sig = dequeue_signal(&mask, info, &type);
if (!sig && timeout) {
/*
* None ready, temporarily unblock those we're interested
@@ -3661,7 +3658,7 @@ static int do_sigtimedwait(const sigset_
spin_lock_irq(&tsk->sighand->siglock);
__set_task_blocked(tsk, &tsk->real_blocked);
sigemptyset(&tsk->real_blocked);
- sig = dequeue_signal(tsk, &mask, info, &type);
+ sig = dequeue_signal(&mask, info, &type);
}
spin_unlock_irq(&tsk->sighand->siglock);

2024-04-18 14:43:57

by Oleg Nesterov

Subject: Re: [patch V2 23/50] signal: Remove task argument from dequeue_signal()

On 04/11, Thomas Gleixner wrote:
>
> The task pointer which is handed to dequeue_signal() is always current. The
> argument along with the first comment about signalfd in that function is
> confusing at best. Remove it and use current internally.
>
> Update the stale comment for dequeue_signal() while at it.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Reviewed-by: Frederic Weisbecker <[email protected]>
> ---
> fs/signalfd.c | 4 ++--
> include/linux/sched/signal.h | 5 ++---
> kernel/signal.c | 23 ++++++++++-------------

Reviewed-by: Oleg Nesterov <[email protected]>