From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751906AbdJBPWx (ORCPT ); Mon, 2 Oct 2017 11:22:53 -0400 Received: from shelob.surriel.com ([96.67.55.147]:51754 "EHLO shelob.surriel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751770AbdJBPWw (ORCPT ); Mon, 2 Oct 2017 11:22:52 -0400 Message-ID: <1506957764.21121.122.camel@surriel.com> Subject: Re: [PATCH v2 2/2] pid: Remove pidhash From: Rik van Riel To: Oleg Nesterov Cc: Gargi Sharma , linux-kernel@vger.kernel.org, julia.lawall@lip6.fr, akpm@linux-foundation.org, mingo@kernel.org, pasha.tatashin@oracle.com, ktkhai@virtuozzo.com Date: Mon, 02 Oct 2017 11:22:44 -0400 In-Reply-To: <20171002152135.GB9481@redhat.com> References: <20170927154543.GA3788@redhat.com> <1506951313.21121.119.camel@surriel.com> <20171002152135.GB9481@redhat.com> Content-Type: multipart/signed; micalg="pgp-sha256"; protocol="application/pgp-signature"; boundary="=-FcSz481LvWqTPAo5wsXN" X-Mailer: Evolution 3.22.6 (3.22.6-2.fc25) Mime-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-FcSz481LvWqTPAo5wsXN Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, 2017-10-02 at 17:21 +0200, Oleg Nesterov wrote: > Hi Rik, >=20 > On 10/02, Rik van Riel wrote: > >=20 > > Gargi and I are looking at that code, and trying to figure out > > exactly what needs to be done to make all of this correct. >=20 > see another email I sent to Gargi a minute ago, >=20 > > 2) With pid_ns_prepare_proc out of the way, we can put all the code > > =C2=A0=C2=A0=C2=A0from below where the call to pid_ns_prepare_proc is n= ow (except > > =C2=A0=C2=A0=C2=A0error handing) into the main loop of pid allocation, = so we can > > =C2=A0=C2=A0=C2=A0do all that stuff under the pidmap_lock: > >=20 > > =C2=A0=C2=A0=C2=A0for (i =3D ns->level; i >=3D 0; i--) { > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0... > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0idr_alloc_cyclic(...) > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0get_pid_ns(ns); > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0atomic_set(&pid->count, 1); > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0for (...) > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0INIT_HLIST_HEAD(...) > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0ns->nr_allocated++; > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0... > > =C2=A0 } >=20 > I do not see how this can fix the problem with not-fully-initialized > pid returned by find_pid_ns(). >=20 > As for PIDNS_ADDING/PIDNS_HASH_ADDING, _perhaps_ we can cleanup this > logic > a bit and do the check earlier, but imo this needs another/separate > change. >=20 > I'd suggest to keep the current logic and the order of initialization > and > just do >=20 > for (i =3D ns->level; i >=3D 0; i--) { > ... >=20 > // do not expose the new pid to find_pid_ns() until it > // is fully initialized > nr =3D idr_alloc_cyclic(&tmp->idr, /*pid*/ NULL, ...); > ... > } >=20 > ... >=20 > spin_lock_irq(&pidmap_lock); > if (!(ns->nr_hashed & PIDNS_HASH_ADDING)) > goto out_unlock; > for ( ; upid >=3D pid->numbers; --upid) { > - hlist_add_head_rcu(&upid->pid_chain, > - &pid_hash[pid_hashfn(upid->nr, upid- > >ns)]); > + // finally make it visible to find_pid_ns() > + idr_replace(upid->ns-idr, pid, upid->nr); > upid->ns->nr_hashed++; > } > spin_unlock_irq(&pidmap_lock); >=20 > Or I missed something? You are right, that would both fix the problem, and keep the error paths relatively simple. Gargi, what do you think? --=20 All Rights Reversed. --=-FcSz481LvWqTPAo5wsXN Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAABCAAGBQJZ0lnEAAoJEM553pKExN6Da9sH/jj7BKzia+i1ManiiynqN1JR h5hP2mIWDPhbGsC1r4z9MFH+dsEdojZnNgwK8dhg37UYUx9umSjOpKkwrJQ58feL uHMLLIz9l8/+btgdkKvWyiUfOhNMpkVXs7iFiwRf5x8z9xaBpBlYYqz99l+Ta+RS RXSdTikfRiXthwdmK1972W0rPfNEEkWkYWbU3qrb9c409aNULaRgel2HwPJNESvY LA9c1/8P09KNki1wcjRBvodAZWxlWuEvDR8uDrQkVKzBVhjKK+6eIiep550gi10R 2qHvMOcZdFRsjHxNHfF0n3StpEONHx6fTho1HGIFUOE3+OyK1OXyKEBYSE88OWs= =2L+B -----END PGP SIGNATURE----- --=-FcSz481LvWqTPAo5wsXN--