Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 7 Aug 2014 16:46:14 +0300
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        "Alexander V. Chernikov" <melifaro@FreeBSD.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r267716 - in head/sys: kern sys
Message-ID:  <20140807134614.GU93733@kib.kiev.ua>
In-Reply-To: <201406221132.s5MBWNWj097084@svn.freebsd.org>
References:  <201406221132.s5MBWNWj097084@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--NUEmSKXm1rGj4SU8
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sun, Jun 22, 2014 at 11:32:23AM +0000, Alexander V. Chernikov wrote:
> Author: melifaro
> Date: Sun Jun 22 11:32:23 2014
> New Revision: 267716
> URL: http://svnweb.freebsd.org/changeset/base/267716
>=20
> Log:
>   Permit changing cpu mask for cpu set 1 in presence of drivers
>   binding their threads to particular CPU.
>  =20
>   Changing ithread cpu mask is now performed by special cpuset_setithread=
().
>   It creates additional cpuset root group on first bind invocation.
>  =20
>   No objection:	jhb
>   Tested by:	hiren
>   MFC after:	2 weeks
>   Sponsored by:	Yandex LLC
>=20
> Modified:
>   head/sys/kern/kern_cpuset.c
>   head/sys/kern/kern_intr.c
>   head/sys/sys/cpuset.h
>=20
> Modified: head/sys/kern/kern_cpuset.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sys/kern/kern_cpuset.c	Sun Jun 22 10:00:33 2014	(r267715)
> +++ head/sys/kern/kern_cpuset.c	Sun Jun 22 11:32:23 2014	(r267716)
> @@ -106,7 +106,7 @@ static uma_zone_t cpuset_zone;
>  static struct mtx cpuset_lock;
>  static struct setlist cpuset_ids;
>  static struct unrhdr *cpuset_unr;
> -static struct cpuset *cpuset_zero;
> +static struct cpuset *cpuset_zero, *cpuset_default;
> =20
>  /* Return the size of cpuset_t at the kernel level */
>  SYSCTL_INT(_kern_sched, OID_AUTO, cpusetsize, CTLFLAG_RD,
> @@ -716,6 +716,89 @@ out:
>  }
> =20
>  /*
> + * Apply new cpumask to the ithread.
> + */
> +int
> +cpuset_setithread(lwpid_t id, u_char cpu)
> +{
> +	struct cpuset *nset, *rset;
> +	struct cpuset *parent, *old_set;
> +	struct thread *td;
> +	struct proc *p;
> +	cpusetid_t cs_id;
> +	cpuset_t mask;
> +	int error;
> +
> +	nset =3D uma_zalloc(cpuset_zone, M_WAITOK);
> +	rset =3D uma_zalloc(cpuset_zone, M_WAITOK);
> +
> +	CPU_ZERO(&mask);
> +	if (cpu =3D=3D NOCPU)
> +		CPU_COPY(cpuset_root, &mask);
> +	else
> +		CPU_SET(cpu, &mask);
> +
> +	error =3D cpuset_which(CPU_WHICH_TID, id, &p, &td, &old_set);
> +	if (((cs_id =3D alloc_unr(cpuset_unr)) =3D=3D CPUSET_INVALID) || error =
!=3D 0)
Why calling alloc_unr() at all if error !=3D 0 ?

> +		goto out;
> +
> +	thread_lock(td);
> +	old_set =3D td->td_cpuset;
> +
> +	if (cpu =3D=3D NOCPU) {
> +		/*
> +		 * roll back to default set. We're not using cpuset_shadow()
> +		 * here because we can fail CPU_SUBSET() check. This can happen
> +		 * if default set does not contain all CPUs.
> +		 */
> +		error =3D _cpuset_create(nset, cpuset_default, &mask,
> +		    CPUSET_INVALID);
Why should _cpuset_create() be called under the thread_lock() taken ?

> +
> +		goto applyset;
> +	}
> +
> +	if (old_set->cs_id =3D=3D 1 || (old_set->cs_id =3D=3D CPUSET_INVALID &&
> +	    old_set->cs_parent->cs_id =3D=3D 1)) {
> +		/* Default mask, we need to use new root set */
> +		error =3D _cpuset_create(rset, cpuset_zero,
> +		    &cpuset_zero->cs_mask, cs_id);
And this one, does the call need thread_lock ?

> +		if (error !=3D 0) {
Wouldn't this leak thread_lock() ?

> +			PROC_UNLOCK(p);
> +			goto out;
> +		}
> +		rset->cs_flags |=3D CPU_SET_ROOT;
> +		parent =3D rset;
> +		rset =3D NULL;
> +		cs_id =3D CPUSET_INVALID;
> +	} else {
> +		/* Assume existing set was already allocated by previous call */
> +		parent =3D td->td_cpuset;
> +		old_set =3D NULL;
> +	}
> +
> +	error =3D cpuset_shadow(parent, nset, &mask);
> +applyset:
> +	if (error =3D=3D 0) {
> +		td->td_cpuset =3D nset;
> +		sched_affinity(td);
> +		nset =3D NULL;
> +	}
> +	thread_unlock(td);
> +	PROC_UNLOCK(p);
> +	if (old_set !=3D NULL)
> +		cpuset_rel(old_set);
> +out:
> +	if (nset !=3D NULL)
> +		uma_zfree(cpuset_zone, nset);
> +	if (rset !=3D NULL)
> +		uma_zfree(cpuset_zone, rset);
> +	if (cs_id !=3D CPUSET_INVALID)
> +		free_unr(cpuset_unr, cs_id);
> +	return (error);
> +}
> +
> +
> +/*
>   * Creates the cpuset for thread0.  We make two sets:
>   *=20
>   * 0 - The root set which should represent all valid processors in the
> @@ -735,6 +818,7 @@ cpuset_thread0(void)
>  	cpuset_zone =3D uma_zcreate("cpuset", sizeof(struct cpuset), NULL, NULL,
>  	    NULL, NULL, UMA_ALIGN_PTR, 0);
>  	mtx_init(&cpuset_lock, "cpuset", NULL, MTX_SPIN | MTX_RECURSE);
> +
>  	/*
>  	 * Create the root system set for the whole machine.  Doesn't use
>  	 * cpuset_create() due to NULL parent.
> @@ -747,12 +831,15 @@ cpuset_thread0(void)
>  	set->cs_flags =3D CPU_SET_ROOT;
>  	cpuset_zero =3D set;
>  	cpuset_root =3D &set->cs_mask;
> +
>  	/*
>  	 * Now derive a default, modifiable set from that to give out.
>  	 */
>  	set =3D uma_zalloc(cpuset_zone, M_WAITOK);
>  	error =3D _cpuset_create(set, cpuset_zero, &cpuset_zero->cs_mask, 1);
>  	KASSERT(error =3D=3D 0, ("Error creating default set: %d\n", error));
> +	cpuset_default =3D set;
> +
>  	/*
>  	 * Initialize the unit allocator. 0 and 1 are allocated above.
>  	 */
>=20
> Modified: head/sys/kern/kern_intr.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sys/kern/kern_intr.c	Sun Jun 22 10:00:33 2014	(r267715)
> +++ head/sys/kern/kern_intr.c	Sun Jun 22 11:32:23 2014	(r267716)
> @@ -295,7 +295,6 @@ intr_event_create(struct intr_event **ev
>  int
>  intr_event_bind(struct intr_event *ie, u_char cpu)
>  {
> -	cpuset_t mask;
>  	lwpid_t id;
>  	int error;
> =20
> @@ -316,14 +315,9 @@ intr_event_bind(struct intr_event *ie, u
>  	 */
>  	mtx_lock(&ie->ie_lock);
>  	if (ie->ie_thread !=3D NULL) {
> -		CPU_ZERO(&mask);
> -		if (cpu =3D=3D NOCPU)
> -			CPU_COPY(cpuset_root, &mask);
> -		else
> -			CPU_SET(cpu, &mask);
>  		id =3D ie->ie_thread->it_thread->td_tid;
>  		mtx_unlock(&ie->ie_lock);
> -		error =3D cpuset_setthread(id, &mask);
> +		error =3D cpuset_setithread(id, cpu);
>  		if (error)
>  			return (error);
>  	} else
> @@ -332,14 +326,10 @@ intr_event_bind(struct intr_event *ie, u
>  	if (error) {
>  		mtx_lock(&ie->ie_lock);
>  		if (ie->ie_thread !=3D NULL) {
> -			CPU_ZERO(&mask);
> -			if (ie->ie_cpu =3D=3D NOCPU)
> -				CPU_COPY(cpuset_root, &mask);
> -			else
> -				CPU_SET(ie->ie_cpu, &mask);
> +			cpu =3D ie->ie_cpu;
>  			id =3D ie->ie_thread->it_thread->td_tid;
>  			mtx_unlock(&ie->ie_lock);
> -			(void)cpuset_setthread(id, &mask);
> +			(void)cpuset_setithread(id, cpu);
>  		} else
>  			mtx_unlock(&ie->ie_lock);
>  		return (error);
>=20
> Modified: head/sys/sys/cpuset.h
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sys/sys/cpuset.h	Sun Jun 22 10:00:33 2014	(r267715)
> +++ head/sys/sys/cpuset.h	Sun Jun 22 11:32:23 2014	(r267716)
> @@ -118,6 +118,7 @@ struct cpuset *cpuset_thread0(void);
>  struct cpuset *cpuset_ref(struct cpuset *);
>  void	cpuset_rel(struct cpuset *);
>  int	cpuset_setthread(lwpid_t id, cpuset_t *);
> +int	cpuset_setithread(lwpid_t id, u_char cpu);
>  int	cpuset_create_root(struct prison *, struct cpuset **);
>  int	cpuset_setproc_update_set(struct proc *, struct cpuset *);
>  char	*cpusetobj_strprint(char *, const cpuset_t *);

--NUEmSKXm1rGj4SU8
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJT44MlAAoJEJDCuSvBvK1B6OMP/0BIF0NkCM4ElxwcjG/w2vXG
WIa+d8DuuVugZ+ej1zazjw6gs3uvuZ0ImrO5sF5m6+3Z21oFTwuv7b+quSW/r63O
Rl843Zjn9UwVD4WGX97seNWPnnr6xxCMk+Uc8dmlLmyAUC9GVetMh5C/Aw+roXb7
pZKK7dEu/MGv37yXMDxCINO1LC5gq7a+05OONdoTQYa8C736UwLr94MZi7garKeL
Cu5kcYJFfVD9gipiurC/1EK7Xjt58g0r2d4D+pdZvUe2G+wW3tV+yPHzqhTn9+fQ
0VlXHRFE92FFcrnecX3ejSpCMokqSpjWwNoglS8fe1Y5MhZD/7fOwA6D5wzmx5YA
IdL0EJ3/3+QUmdlszl00wmqD+s6ivQb/4Ttdxk+WzWH7amQZCv+KzPzRmCKvpwjC
zAghlyxZXOaUZ4ipOMnu/F5MCOPoEVu8pGeZuP812wJFjWXQmb00JVGyvm0qFf21
2mWzImB0OccNiC6dD/c8X2G/o6Q2J11/m+68mHMpX7JxfZCx8XulRB/kSukTzMMx
LKE4ZdU0RPwsD1vouAY9fxwZePqb2toDM48H3M/1HtERx/Kniek9caY0FgQRsgEO
MBLIGHiAkPaNk6mAaIoIT4+zcNra0MWa8q9lXo1RBk87qT9qN9q4DnTJEcg6nKcR
8mm9H5+vjC82IbommJm/
=KWGR
-----END PGP SIGNATURE-----

--NUEmSKXm1rGj4SU8--



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