From mboxrd@z Thu Jan  1 00:00:00 1970
From: Dario Faggioli <dario.faggioli@citrix.com>
Subject: Re: [PATCH 03/16] xen: sched: make implementing
 .alloc_pdata optional
Date: Fri, 1 Apr 2016 19:01:18 +0200
Message-ID: <1459530078.5082.289.camel@citrix.com>
References: <20160318185524.8117.74837.stgit@Solace.station>
 <20160318190416.8117.57233.stgit@Solace.station>
 <56F009BC.4020807@suse.com>
 <56F01C3D02000078000DEE5B@prv-mh.provo.novell.com>
Mime-Version: 1.0
Content-Type: multipart/mixed; boundary="===============7378560661548879003=="
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta6.messagelabs.com ([85.158.143.247])
 by lists.xenproject.org with esmtp (Exim 4.84_2)
 (envelope-from <prvs=892b76386=dario.faggioli@citrix.com>)
 id 1am2So-0007OP-L6
 for xen-devel@lists.xenproject.org; Fri, 01 Apr 2016 17:02:26 +0000
In-Reply-To: <56F01C3D02000078000DEE5B@prv-mh.provo.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
 <mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
 <mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Errors-To: xen-devel-bounces@lists.xen.org
Sender: "Xen-devel" <xen-devel-bounces@lists.xen.org>
To: Jan Beulich <JBeulich@suse.com>, Juergen Gross <JGross@suse.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>, xen-devel@lists.xenproject.org, Josh Whitehead <josh.whitehead@dornerworks.com>, Meng Xu <mengxu@cis.upenn.edu>, Robert VanVossen <robert.vanvossen@dornerworks.com>
List-Id: xen-devel@lists.xenproject.org

--===============7378560661548879003==
Content-Type: multipart/signed; micalg=pgp-sha1;
	protocol="application/pgp-signature"; boundary="=-JtzddPOCzCFNmRkqG4uq"

--=-JtzddPOCzCFNmRkqG4uq
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, 2016-03-21 at 09:07 -0600, Jan Beulich wrote:
> > > > On 21.03.16 at 15:48, <JGross@suse.com> wrote:
> > On 18/03/16 20:04, Dario Faggioli wrote:
> > > @@ -1491,9 +1491,9 @@ static int cpu_schedule_up(unsigned int
> > > cpu)
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if ( idle_vcpu[cpu] =3D=3D NULL )
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return -ENOMEM;
> > > =C2=A0
> > > -=C2=A0=C2=A0=C2=A0=C2=A0if ( (ops.alloc_pdata !=3D NULL) &&
> > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0((sd->sched_pr=
iv =3D ops.alloc_pdata(&ops, cpu)) =3D=3D NULL)
> > > )
> > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return -ENOMEM;
> > > +=C2=A0=C2=A0=C2=A0=C2=A0sd->sched_priv =3D SCHED_OP(&ops, alloc_pdat=
a, cpu);
> > > +=C2=A0=C2=A0=C2=A0=C2=A0if ( IS_ERR(sd->sched_priv) )
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return PTR_ERR(sd->s=
ched_priv);
> > Calling xfree() with an IS_ERR() value might be a bad idea.
> > Either you need to set sd->sched_priv to NULL in error case or you
> > modify xfree() to return immediately not only in the NULL case, but
> > in the IS_ERR() case as well.
> The latter option is a no-go imo.
>=20
Ok, I'll do:

=C2=A0 =C2=A0 sd->sched_priv =3D SCHED_OP(&ops, alloc_pdata, cpu);
=C2=A0 =C2=A0 if ( IS_ERR(sd->sched_priv) )
=C2=A0 =C2=A0 {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 int err =3D PTR_ERR(sd->sched_priv);

=C2=A0 =C2=A0 =C2=A0 =C2=A0 sd->sched_priv =3D NULL;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 return err;
=C2=A0 =C2=A0 }

Is this ok?

And, just to be sure I got your point (Juergen), you're referring to
the situation where cpu_schedule_up() fails, we go to CPU_UP_CANCELLED,
which calls cpu_schedule_donw(), which calls free_pdata on
sd->sched_priv (inside which we may reach an xfree()), aren't you?

In fact, alloc_pdata is also called in schedule_cpu_switch(), but in
that case, I don't see anyone calling xfree() if alloc_pdata fails...
Am I missing it?

Thanks and Regards,
Dario
--=C2=A0
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


--=-JtzddPOCzCFNmRkqG4uq
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

iEYEABECAAYFAlb+qWAACgkQk4XaBE3IOsTIwQCfemN8YN3JUGNllWNI4jK4CIpR
kjsAniqnwyPOjQBp5bLAOyLXf2pEcM9i
=GhiV
-----END PGP SIGNATURE-----

--=-JtzddPOCzCFNmRkqG4uq--


--===============7378560661548879003==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: base64
Content-Disposition: inline

X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs
IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y
Zy94ZW4tZGV2ZWwK

--===============7378560661548879003==--