Re: Permit init(8) use its own cpuset group.

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

On Mon, Jun 02, 2014 at 06:52:10PM +0400, Alexander V. Chernikov wrote:
> Hello list!
>=20
> Currently init(8) uses group 1 which is root group.
> Modifications of this group affects both kernel and userland threads.
> Additionally, such modifications are impossible, for example, in presence
> of multi-queue NIC drivers (like igb or ixgbe) which binds their threads =
to
> particular cpus.
>=20
> Proposed change ("init_cpuset" loader tunable) permits changing cpu=20
> masks for
> userland more easily. Restricting user processes to migrate to/from CPU=
=20
> cores
> used for network traffic processing is one of the cases.
>=20
> Phabricator: https://phabric.freebsd.org/D141 (the same version attached=
=20
> inline)
>=20
> If there are no objections, I'll commit this next week.
Why is the tunable needed ? IMO it is more reasonable to create a new
cpuset always, at least I cannot explain why the existing behaviour
is useful.  If creating new cpuset unconditionally, you could consider
doing it in kernel in start_init().  You would also need to update the
cpuset(2) man page, which states that cpuset 1 is set for all processes.

Still, see comments below for the patch.

> Index: sbin/init/init.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
> --- sbin/init/init.c	(revision 266306)
> +++ sbin/init/init.c	(working copy)
> @@ -47,6 +47,8 @@ static const char rcsid[] =3D
>  #include <sys/param.h>
>  #include <sys/ioctl.h>
>  #include <sys/mount.h>
> +#include <sys/param.h>
> +#include <sys/cpuset.h>
>  #include <sys/sysctl.h>
>  #include <sys/wait.h>
>  #include <sys/stat.h>
> @@ -320,6 +322,19 @@ invalid:
>  			warning("Can't chroot to %s: %m", kenv_value);
>  	}
> =20
> +	if (kenv(KENV_GET, "init_cpuset", kenv_value, sizeof(kenv_value)) > 0) {
> +		if (getpid() =3D=3D 1) {
If you use the && operator for conditionals instead of two if()s, the nesti=
ng
level of the block can be reduced.

> +			cpusetid_t setid;
The variable must be declared at the function start.

> +
> +			setid =3D -1;
Why do you need to initialize the variable ?

> +			if (cpuset(&setid) !=3D 0) {
> +				warning("cpu set alloc failed: %m");
> +			} else {
> +				if (cpuset_setid(CPU_WHICH_PID, 1, setid) !=3D 0)
This could be else if () again to reduce indentation.

> +					warning("cpuset_setsid failed: %m");
> +			}
> +		}
> +	}
>  	/*
>  	 * Additional check if devfs needs to be mounted:
>  	 * If "/" and "/dev" have the same device number,

> _______________________________________________
> freebsd-hackers@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org"

--6k8oSBQUGGHRSAt9
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (FreeBSD)

iQIcBAEBAgAGBQJTjKryAAoJEJDCuSvBvK1BbUkP/0yDiTkAjyiMWLdm19y8Rkbz
2K5TcZCnbHbHvrd+Z3zGuH+Op7vSWvJ1sHuE4Qfe9VSPDQyk8T3vhSEp9s/KWlhe
dwOhBDxL6Nk/lhn9eGjLWjAe/UmVS0Q8M+v0QX4X7ZbCgVaHQApLu+z9ejeGmCt6
bGqZmXmYmshtPEm8k9NlV9sHwaAB8ySVGf9WYWUDPBg+MS9IFACavRF1tQeXYaH1
8duuzSbhUvARaM5qEz9fwep28AiyX7EFgsukfytS9O7JStDc8Qgjdc2iCtzCUqXI
duYwhMNfCEhoYbCCjdycd4FtyKhLgBbXEWMj5n7PipN9+qUemSwH7Ascjo0ybw/e
sXt35km9tqKm+xpsHzkKrW73/ibewCf5bN4xndaOwlgNRrmKNYvwCYa2xi4nRUqB
u6BCBBtIaDN+Vwa/993AKegbwPMF27HOdPGEpSigcq5AUYAh631rRNnNh0hipMn/
TiWR8BvBqNqkigKr/FYcUv2OrsengywUYSE81zVoWM3r/MdyKCqcmo6p9yHDuEXp
nSEFZjg/JYjIgGfjTvTvqoFjijw7k59zPFXhOGpyr0WSztstIq6p0mEheuMN6n0v
zOSXLChAeT/ijCIgTpp52jz0/BOmtzehWx329U2ueLN4GfNWKLvUck9VWFwZ0gBl
l4Fda7DWVipIP7Pg2pCe
=EELr
-----END PGP SIGNATURE-----

--6k8oSBQUGGHRSAt9--