From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Gibson Date: Mon, 21 Sep 2015 01:37:08 +0000 Subject: Re: [PATCH] KVM: PPC: Take the kvm->srcu lock in kvmppc_h_logical_ci_load/store() Message-Id: <20150921013708.GF20331@voom.fritz.box> MIME-Version: 1 Content-Type: multipart/mixed; boundary="Zs/RYxT/hKAHzkfQ" List-Id: References: <1442559448-1947-1-git-send-email-thuth@redhat.com> In-Reply-To: <1442559448-1947-1-git-send-email-thuth@redhat.com> To: Thomas Huth Cc: kvm-ppc@vger.kernel.org, Paul Mackerras , Alexander Graf , kvm@vger.kernel.org --Zs/RYxT/hKAHzkfQ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 18, 2015 at 08:57:28AM +0200, Thomas Huth wrote: > Access to the kvm->buses (like with the kvm_io_bus_read() and -write() > functions) has to be protected via the kvm->srcu lock. > The kvmppc_h_logical_ci_load() and -store() functions are missing > this lock so far, so let's add it there, too. > This fixes the problem that the kernel reports "suspicious RCU usage" > when lock debugging is enabled. >=20 > Fixes: 99342cf8044420eebdf9297ca03a14cb6a7085a1 > Signed-off-by: Thomas Huth Nice catch. Looks like I missed this because the places kvm_io_bus_{read,write}() are called on x86 are buried about 5 layers below where the srcu lock is taken :/. Reviewed-by: David Gibson > --- > arch/powerpc/kvm/book3s.c | 6 ++++++ > 1 file changed, 6 insertions(+) >=20 > diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c > index d75bf32..096e5eb 100644 > --- a/arch/powerpc/kvm/book3s.c > +++ b/arch/powerpc/kvm/book3s.c > @@ -828,12 +828,15 @@ int kvmppc_h_logical_ci_load(struct kvm_vcpu *vcpu) > unsigned long size =3D kvmppc_get_gpr(vcpu, 4); > unsigned long addr =3D kvmppc_get_gpr(vcpu, 5); > u64 buf; > + int srcu_idx; > int ret; > =20 > if (!is_power_of_2(size) || (size > sizeof(buf))) > return H_TOO_HARD; > =20 > + srcu_idx =3D srcu_read_lock(&vcpu->kvm->srcu); > ret =3D kvm_io_bus_read(vcpu, KVM_MMIO_BUS, addr, size, &buf); > + srcu_read_unlock(&vcpu->kvm->srcu, srcu_idx); > if (ret !=3D 0) > return H_TOO_HARD; > =20 > @@ -868,6 +871,7 @@ int kvmppc_h_logical_ci_store(struct kvm_vcpu *vcpu) > unsigned long addr =3D kvmppc_get_gpr(vcpu, 5); > unsigned long val =3D kvmppc_get_gpr(vcpu, 6); > u64 buf; > + int srcu_idx; > int ret; > =20 > switch (size) { > @@ -891,7 +895,9 @@ int kvmppc_h_logical_ci_store(struct kvm_vcpu *vcpu) > return H_TOO_HARD; > } > =20 > + srcu_idx =3D srcu_read_lock(&vcpu->kvm->srcu); > ret =3D kvm_io_bus_write(vcpu, KVM_MMIO_BUS, addr, size, &buf); > + srcu_read_unlock(&vcpu->kvm->srcu, srcu_idx); > if (ret !=3D 0) > return H_TOO_HARD; > =20 --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --Zs/RYxT/hKAHzkfQ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJV/19EAAoJEGw4ysog2bOSqcsQAJdPaKtI/8lWH5KFghwQNQZs p6LKPU68Ag5S0Lg1W3y2VwCTdyKkvJdgHzSjXhFfUZboF/EvxK/7QfMT3XNaDV7h MAWnj61OUAVaXEIFOMuEqjDpH4Sb77rknRMe3qEbHmrNjABBQH8/5FpaIS0NvlYs qOrdrhTUdV2QCUIht3XQ09eVtyzpXaIftEZvqE38juZAHe5LY56wAm1rZP1CcNR0 Yo27SqW/XwMm4kUtxNaiMuNnm2hF8B5soQIRM1CM3wAziO/eqboO5qg7TvdXJuco 3zZ6Ewq7Z3jLcYYQggbZxhR0TdkfQleki2JiQ0xxQToZs1LNEcOq2x52lkk/pJ8X pjoTIUWfE+JKdiagf41JY3tq9TQY60m4AGYhzTb4rMbHNahZxEfSgzKBlVXgdBMg mUsx3Qx2rYaWj1R0IrehE/SaHbbc51Fvh1HEBJHoM89OBLc6XQeDtShyWbqdEqqE 7ij+nEp1xACtHZa/byW7lmRN8Zgu21dPGqEVCmXNVcMBhGcXw0eKHueSF26qgCAJ ICppQUVd0LA+Di67bv6NkXOK9QyfGDcyqPG4Xp5+mxS2dUMJaPxXR4lrIFGhVLYN 5wg99cxE5IOGgOY738G4YEJm7eAOO62TDVzwezVsIjkPjcgiFE6r8qpjUcmkG9rT UyIxxgEdN+dCi6CikXry =qWEw -----END PGP SIGNATURE----- --Zs/RYxT/hKAHzkfQ-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Gibson Subject: Re: [PATCH] KVM: PPC: Take the kvm->srcu lock in kvmppc_h_logical_ci_load/store() Date: Mon, 21 Sep 2015 11:37:08 +1000 Message-ID: <20150921013708.GF20331@voom.fritz.box> References: <1442559448-1947-1-git-send-email-thuth@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Zs/RYxT/hKAHzkfQ" Cc: kvm-ppc@vger.kernel.org, Paul Mackerras , Alexander Graf , kvm@vger.kernel.org To: Thomas Huth Return-path: Received: from ozlabs.org ([103.22.144.67]:52116 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755586AbbIUBgt (ORCPT ); Sun, 20 Sep 2015 21:36:49 -0400 Content-Disposition: inline In-Reply-To: <1442559448-1947-1-git-send-email-thuth@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: --Zs/RYxT/hKAHzkfQ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 18, 2015 at 08:57:28AM +0200, Thomas Huth wrote: > Access to the kvm->buses (like with the kvm_io_bus_read() and -write() > functions) has to be protected via the kvm->srcu lock. > The kvmppc_h_logical_ci_load() and -store() functions are missing > this lock so far, so let's add it there, too. > This fixes the problem that the kernel reports "suspicious RCU usage" > when lock debugging is enabled. >=20 > Fixes: 99342cf8044420eebdf9297ca03a14cb6a7085a1 > Signed-off-by: Thomas Huth Nice catch. Looks like I missed this because the places kvm_io_bus_{read,write}() are called on x86 are buried about 5 layers below where the srcu lock is taken :/. Reviewed-by: David Gibson > --- > arch/powerpc/kvm/book3s.c | 6 ++++++ > 1 file changed, 6 insertions(+) >=20 > diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c > index d75bf32..096e5eb 100644 > --- a/arch/powerpc/kvm/book3s.c > +++ b/arch/powerpc/kvm/book3s.c > @@ -828,12 +828,15 @@ int kvmppc_h_logical_ci_load(struct kvm_vcpu *vcpu) > unsigned long size =3D kvmppc_get_gpr(vcpu, 4); > unsigned long addr =3D kvmppc_get_gpr(vcpu, 5); > u64 buf; > + int srcu_idx; > int ret; > =20 > if (!is_power_of_2(size) || (size > sizeof(buf))) > return H_TOO_HARD; > =20 > + srcu_idx =3D srcu_read_lock(&vcpu->kvm->srcu); > ret =3D kvm_io_bus_read(vcpu, KVM_MMIO_BUS, addr, size, &buf); > + srcu_read_unlock(&vcpu->kvm->srcu, srcu_idx); > if (ret !=3D 0) > return H_TOO_HARD; > =20 > @@ -868,6 +871,7 @@ int kvmppc_h_logical_ci_store(struct kvm_vcpu *vcpu) > unsigned long addr =3D kvmppc_get_gpr(vcpu, 5); > unsigned long val =3D kvmppc_get_gpr(vcpu, 6); > u64 buf; > + int srcu_idx; > int ret; > =20 > switch (size) { > @@ -891,7 +895,9 @@ int kvmppc_h_logical_ci_store(struct kvm_vcpu *vcpu) > return H_TOO_HARD; > } > =20 > + srcu_idx =3D srcu_read_lock(&vcpu->kvm->srcu); > ret =3D kvm_io_bus_write(vcpu, KVM_MMIO_BUS, addr, size, &buf); > + srcu_read_unlock(&vcpu->kvm->srcu, srcu_idx); > if (ret !=3D 0) > return H_TOO_HARD; > =20 --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --Zs/RYxT/hKAHzkfQ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJV/19EAAoJEGw4ysog2bOSqcsQAJdPaKtI/8lWH5KFghwQNQZs p6LKPU68Ag5S0Lg1W3y2VwCTdyKkvJdgHzSjXhFfUZboF/EvxK/7QfMT3XNaDV7h MAWnj61OUAVaXEIFOMuEqjDpH4Sb77rknRMe3qEbHmrNjABBQH8/5FpaIS0NvlYs qOrdrhTUdV2QCUIht3XQ09eVtyzpXaIftEZvqE38juZAHe5LY56wAm1rZP1CcNR0 Yo27SqW/XwMm4kUtxNaiMuNnm2hF8B5soQIRM1CM3wAziO/eqboO5qg7TvdXJuco 3zZ6Ewq7Z3jLcYYQggbZxhR0TdkfQleki2JiQ0xxQToZs1LNEcOq2x52lkk/pJ8X pjoTIUWfE+JKdiagf41JY3tq9TQY60m4AGYhzTb4rMbHNahZxEfSgzKBlVXgdBMg mUsx3Qx2rYaWj1R0IrehE/SaHbbc51Fvh1HEBJHoM89OBLc6XQeDtShyWbqdEqqE 7ij+nEp1xACtHZa/byW7lmRN8Zgu21dPGqEVCmXNVcMBhGcXw0eKHueSF26qgCAJ ICppQUVd0LA+Di67bv6NkXOK9QyfGDcyqPG4Xp5+mxS2dUMJaPxXR4lrIFGhVLYN 5wg99cxE5IOGgOY738G4YEJm7eAOO62TDVzwezVsIjkPjcgiFE6r8qpjUcmkG9rT UyIxxgEdN+dCi6CikXry =qWEw -----END PGP SIGNATURE----- --Zs/RYxT/hKAHzkfQ--