Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 1 Jul 2014 09:40:57 -0400
From:      John Baldwin <jhb@freebsd.org>
To:        Mateusz Guzik <mjg@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r268074 - head/sys/kern
Message-ID:  <201407010940.57602.jhb@freebsd.org>
In-Reply-To: <201407010629.s616TFul082441@svn.freebsd.org>
References:  <201407010629.s616TFul082441@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tuesday, July 01, 2014 2:29:15 am Mateusz Guzik wrote:
> Author: mjg
> Date: Tue Jul  1 06:29:15 2014
> New Revision: 268074
> URL: http://svnweb.freebsd.org/changeset/base/268074
> 
> Log:
>   Perform a lockless check in sigacts_shared.
>   
>   It is used only during execve (i.e. singlethreaded), so there is no fear
>   of returning 'not shared' which soon becomes 'shared'.
>   
>   While here reorganize the code a little to avoid proc lock/unlock in
>   shared case.
>   
>   MFC after:	1 week
> 
> Modified:
>   head/sys/kern/kern_exec.c
>   head/sys/kern/kern_sig.c
> 
> Modified: head/sys/kern/kern_exec.c
> 
==============================================================================
> --- head/sys/kern/kern_exec.c	Tue Jul  1 06:23:48 2014	(r268073)
> +++ head/sys/kern/kern_exec.c	Tue Jul  1 06:29:15 2014	(r268074)
> @@ -621,18 +621,17 @@ interpret:
>  	 * handlers. In execsigs(), the new process will have its signals
>  	 * reset.
>  	 */
> -	PROC_LOCK(p);
> -	oldcred = crcopysafe(p, newcred);
>  	if (sigacts_shared(p->p_sigacts)) {
>  		oldsigacts = p->p_sigacts;
> -		PROC_UNLOCK(p);
>  		newsigacts = sigacts_alloc();
>  		sigacts_copy(newsigacts, oldsigacts);
> -		PROC_LOCK(p);
> -		p->p_sigacts = newsigacts;
>  	} else
>  		oldsigacts = NULL;
>  
> +	PROC_LOCK(p);
> +	if (oldsigacts)
> +		p->p_sigacts = newsigacts;
> +	oldcred = crcopysafe(p, newcred);
>  	/* Stop profiling */
>  	stopprofclock(p);
>  
> 
> Modified: head/sys/kern/kern_sig.c
> 
==============================================================================
> --- head/sys/kern/kern_sig.c	Tue Jul  1 06:23:48 2014	(r268073)
> +++ head/sys/kern/kern_sig.c	Tue Jul  1 06:29:15 2014	(r268074)
> @@ -3453,10 +3453,6 @@ sigacts_copy(struct sigacts *dest, struc
>  int
>  sigacts_shared(struct sigacts *ps)
>  {
> -	int shared;
>  
> -	mtx_lock(&ps->ps_mtx);
> -	shared = ps->ps_refcnt > 1;
> -	mtx_unlock(&ps->ps_mtx);
> -	return (shared);
> +	return (ps->ps_refcnt > 1);
>  }

You should KASSERT() in sigacts_shared that P_HADTHREADS is not set so that 
new code does not call this function unsafely in the future.

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201407010940.57602.jhb>