Re: [PATCH] locking/rwsem: fix down_write_killable for CONFIG_RWSEM_GENERIC_SPINLOCK

[Niklas Cassel]

Peter, any comment on this?

You fixed the rwsem implementation that is used by default for x86,
however the rwsem implementation used by default on mips, sparc32,
and many other archs is still affected, hence this patch.

Have a good morning.

/Niklas

On 02/25/2017 01:17 AM, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@xxxxxxxx>
>
> SIGKILL has been sent, but the task is stuck in down_read (after do_exit),
> even though no task is doing down_write on the rwsem in question.
>
> INFO: task libupnp:21868 blocked for more than 120 seconds.
>       Tainted: G           O    4.9.11-devel #1
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> libupnp         D    0 21868      1 0x08100008
> Stack : 00000001 8ecf44a0 8ef644a0 8e06fe40 00000000 808b0000 00000001 00000000
>         00000001 77aab068 00000001 00000001 00000000 8ecf44a0 00400000 800d4d88
>         8ecf44a0 00000002 80698ac0 00000001 8c3f0544 00000001 00000000 8ecf44a0
>         00400000 80694a58 8ecf44a0 00000000 8ecf44a0 8c3f0544 8ecf44a0 80698ac8
>         8c3f0500 00000000 77a053b4 800454bc 8958dd68 8ef83d68 8ecf44a0 00000001
>         ...
> Call Trace:
> [<806942b0>] __schedule+0x738/0xd48
> [<80694a58>] schedule+0x44/0xb8
> [<80698ac8>] __down_read+0xc8/0x124
> [<8004c034>] do_exit+0x18c/0xa78
> [<8004c9cc>] do_group_exit+0x5c/0xc4
> [<8004ca54>] __wake_up_parent+0x0/0x24
>
> This bug has already been fixed for CONFIG_RWSEM_XCHGADD_ALGORITHM in
> commit 04cafed7fc19 ("locking/rwsem: Fix down_write_killable()"), however,
> this bug also exists for CONFIG_RWSEM_GENERIC_SPINLOCK.
>
> Cc: stable <stable@xxxxxxxxxxxxxxx> # 4.7+
> Fixes: d47996082f52 ("locking/rwsem: Introduce basis for down_write_killable()")
> Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxxx>
> ---
>  kernel/locking/rwsem-spinlock.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/locking/rwsem-spinlock.c b/kernel/locking/rwsem-spinlock.c
> index 5eacab880f67..ab7271e16918 100644
> --- a/kernel/locking/rwsem-spinlock.c
> +++ b/kernel/locking/rwsem-spinlock.c
> @@ -212,10 +212,9 @@ int __sched __down_write_common(struct rw_semaphore *sem, int state)
>  		 */
>  		if (sem->count == 0)
>  			break;
> -		if (signal_pending_state(state, current)) {
> -			ret = -EINTR;
> -			goto out;
> -		}
> +		if (signal_pending_state(state, current))
> +			goto out_nolock;
> +
>  		set_current_state(state);
>  		raw_spin_unlock_irqrestore(&sem->wait_lock, flags);
>  		schedule();
> @@ -223,12 +222,19 @@ int __sched __down_write_common(struct rw_semaphore *sem, int state)
>  	}
>  	/* got the lock */
>  	sem->count = -1;
> -out:
>  	list_del(&waiter.list);
>  
>  	raw_spin_unlock_irqrestore(&sem->wait_lock, flags);
>  
>  	return ret;
> +
> +out_nolock:
> +	list_del(&waiter.list);
> +	if (!list_empty(&sem->wait_list))
> +		__rwsem_do_wake(sem, 1);
> +	raw_spin_unlock_irqrestore(&sem->wait_lock, flags);
> +
> +	return -EINTR;
>  }
>  
>  void __sched __down_write(struct rw_semaphore *sem)