Re: [PATCH] spin_lock_irqsave() in autofs_write() is bogus

Dear Al,


Am 17.08.25 um 18:36 schrieb Al Viro:
> 	That function should never be (and never is) called with irqs
> disabled - we have an explicit mutex_lock() in there, if nothing else.
> Which makes spin_lock_irqsave() use in there pointless - we do need to
> disable irqs for ->siglock, but that should be spin_lock_irq().
> 
> 	The history is interesting - it goes all way back to 2.1.68pre1,
> and that obviously was a tree-wide work.  Might be interesting to look
> for other places with just-in-case spin_lock_irqsave()...
> 
> Signed-off-by: Al Viro <[email protected]>
> ---
> diff --git a/fs/autofs/waitq.c b/fs/autofs/waitq.c
> index 33dd4660d82f..4dc226e86360 100644
> --- a/fs/autofs/waitq.c
> +++ b/fs/autofs/waitq.c
> @@ -46,7 +46,7 @@ void autofs_catatonic_mode(struct autofs_sb_info *sbi)
>   static int autofs_write(struct autofs_sb_info *sbi,
>   			struct file *file, const void *addr, int bytes)
>   {
> -	unsigned long sigpipe, flags;
> +	unsigned long sigpipe;
>   	const char *data = (const char *)addr;
>   	ssize_t wr = 0;
>   
> @@ -66,10 +66,10 @@ static int autofs_write(struct autofs_sb_info *sbi,
>   	 * SIGPIPE unless it was already supposed to get one
>   	 */
>   	if (wr == -EPIPE && !sigpipe) {
> -		spin_lock_irqsave(&current->sighand->siglock, flags);
> +		spin_lock_irq(&current->sighand->siglock);
>   		sigdelset(&current->pending.signal, SIGPIPE);
>   		recalc_sigpending();
> -		spin_unlock_irqrestore(&current->sighand->siglock, flags);
> +		spin_unlock_irq(&current->sighand->siglock);
>   	}
>   
>   	/* if 'wr' returned 0 (impossible) we assume -EIO (safe) */
> 

Reviewed-by: Paul Menzel <[email protected]>


Kind regards,

Paul