From owner-svn-src-all@FreeBSD.ORG Tue Jul 1 11:42:52 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 5126ED8D; Tue, 1 Jul 2014 11:42:52 +0000 (UTC) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id E446125E5; Tue, 1 Jul 2014 11:42:51 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.9/8.14.9) with ESMTP id s61BgjbE066338 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 1 Jul 2014 14:42:45 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.8.3 kib.kiev.ua s61BgjbE066338 Received: (from kostik@localhost) by tom.home (8.14.9/8.14.9/Submit) id s61BgjSD066337; Tue, 1 Jul 2014 14:42:45 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Tue, 1 Jul 2014 14:42:45 +0300 From: Konstantin Belousov To: Mateusz Guzik Subject: Re: svn commit: r268087 - head/sys/kern Message-ID: <20140701114245.GO93733@kib.kiev.ua> References: <201407010921.s619LXHL063077@svn.freebsd.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="li6R/r59jzh/oGCG" Content-Disposition: inline In-Reply-To: <201407010921.s619LXHL063077@svn.freebsd.org> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.0 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on tom.home Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 Jul 2014 11:42:52 -0000 --li6R/r59jzh/oGCG Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jul 01, 2014 at 09:21:33AM +0000, Mateusz Guzik wrote: > Author: mjg > Date: Tue Jul 1 09:21:32 2014 > New Revision: 268087 > URL: http://svnweb.freebsd.org/changeset/base/268087 >=20 > Log: > Don't call crcopysafe or uifind unnecessarily in execve. > =20 > MFC after: 1 week >=20 > Modified: > head/sys/kern/kern_exec.c >=20 > Modified: head/sys/kern/kern_exec.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_exec.c Tue Jul 1 08:36:56 2014 (r268086) > +++ head/sys/kern/kern_exec.c Tue Jul 1 09:21:32 2014 (r268087) > @@ -336,7 +336,7 @@ do_execve(td, args, mac_p) > struct proc *p =3D td->td_proc; > struct nameidata nd; > struct ucred *newcred =3D NULL, *oldcred; > - struct uidinfo *euip; > + struct uidinfo *euip =3D NULL; > register_t *stack_base; > int error, i; > struct image_params image_params, *imgp; > @@ -601,8 +601,6 @@ interpret: > /* > * Malloc things before we need locks. > */ > - newcred =3D crget(); > - euip =3D uifind(attr.va_uid); > i =3D imgp->args->begin_envv - imgp->args->begin_argv; > /* Cache arguments if they fit inside our allowance */ > if (ps_arg_cache_limit >=3D i + sizeof(struct pargs)) { > @@ -631,7 +629,7 @@ interpret: > PROC_LOCK(p); > if (oldsigacts) > p->p_sigacts =3D newsigacts; > - oldcred =3D crcopysafe(p, newcred); > + oldcred =3D p->p_ucred; > /* Stop profiling */ > stopprofclock(p); > =20 > @@ -721,6 +719,8 @@ interpret: > vn_lock(imgp->vp, LK_SHARED | LK_RETRY); > if (error !=3D 0) > goto done1; > + newcred =3D crdup(oldcred); > + euip =3D uifind(attr.va_uid); > PROC_LOCK(p); > /* > * Set the new credentials. > @@ -745,7 +745,6 @@ interpret: > change_svuid(newcred, newcred->cr_uid); > change_svgid(newcred, newcred->cr_gid); > p->p_ucred =3D newcred; > - newcred =3D NULL; > } else { > if (oldcred->cr_uid =3D=3D oldcred->cr_ruid && > oldcred->cr_gid =3D=3D oldcred->cr_rgid) > @@ -764,10 +763,12 @@ interpret: > */ > if (oldcred->cr_svuid !=3D oldcred->cr_uid || > oldcred->cr_svgid !=3D oldcred->cr_gid) { > + PROC_UNLOCK(p); > + newcred =3D crdup(oldcred); > + PROC_LOCK(p); > change_svuid(newcred, newcred->cr_uid); > change_svgid(newcred, newcred->cr_gid); > p->p_ucred =3D newcred; > - newcred =3D NULL; > } > } > =20 > @@ -844,11 +845,10 @@ done1: > /* > * Free any resources malloc'd earlier that we didn't use. > */ > - uifree(euip); > - if (newcred =3D=3D NULL) > + if (euip !=3D NULL) > + uifree(euip); > + if (newcred !=3D NULL) > crfree(oldcred); > - else > - crfree(newcred); > VOP_UNLOCK(imgp->vp, 0); > =20 > /* Old code did the malloc(M_WAITOK) call in crget() before the text vnode was locked. After your change, the crdup() is called with the vnode locked. Witness would not tell you that anything is wrong there, but the new code is worse than the previous structure, even if malloc() was sometimes done when not needed. To satisfy the memory request from malloc(), pagedaemon or laundry may need to lock the vnode, which creates a circular dependency. Pagedaemon locks vnodes with timeout, which just means that it would not be able to clean pages while execve() is stuck in malloc(M_WAITOK), while laundry takes the vnode lock without timeout, hanging until the malloc request is satisfied. The rule is, do not allocate memory while vnodes are locked. It is not always followed, but it makes no sense to change existing correct code to broke the pattern. --li6R/r59jzh/oGCG Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJTsp61AAoJEJDCuSvBvK1BVr0P/0ZrHyYse98/Vl3mFB8V5XFf +cLNbCobf8bd1bNCn0h8dStfCQ9rQayHcDbM+sVquQDAh8lyg6B8HUDZuW7TDQSJ 7NM36JZMQI34PGCrucpQCpVOTwnruNmtHEuudi5TVfjyceLOu5dim6QVSOZ80buk p6/zc448dc0gz74ec+01ZYQa9fzhjseNOJlwyZSYHfUr5YfXmDMrl0S4esxbsVNB CPBiBortiAvpsDhpHR+/w5KwHmjgTDoogwSIbOX1Jas5XRnbnvMGQMhFTUNzz/Eb x2o0xg3jrfBR214ESwHtp+Kz6g+Gi/tr4VrfZ58LJE4fIs7MKjF19x9wfL22MVBi W5USQwC7z73msSCUBkS7O3VIaciiz6lv6UAkCNYoCzwsfgTZIY5QniSp6KAK3SDV alJhXrYDv/2QeU7js2k0HUxOg/3aj2WCpvZDkeDWsbjtzuqQLZnAG3UiYI3ukfLm rLWhsP25zP4R6VZji/yNVbw9/gERipyS9ssv1YIlazGku2VJZA8JMHIhMTuF4W9A AKu/vxfzr9RM0FV17bHoD+5q94ciVQuhgbKA8gxDIu5yLy5UMBjkorX0yQVpa2da /ME/GaUwsGrca2cV0clEwvJd5z8+NqixL+mePeJ6Q/Apr/DgLQjKiMKAV6hrpJKD 0OxeZKDsZ0UeWLoki+iE =t4qw -----END PGP SIGNATURE----- --li6R/r59jzh/oGCG--