From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751556AbdISM6P (ORCPT ); Tue, 19 Sep 2017 08:58:15 -0400 Received: from mail-pf0-f176.google.com ([209.85.192.176]:56716 "EHLO mail-pf0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751040AbdISM6M (ORCPT ); Tue, 19 Sep 2017 08:58:12 -0400 X-Google-Smtp-Source: AOwi7QBreSL5o6l78cnU2CbYows6dX+Qm9r6flCU/pLEzaIWULXyX6PcUVwA2qeGA9UFnK1yRjratg== Date: Tue, 19 Sep 2017 20:58:59 +0800 From: Boqun Feng To: linux-kernel@vger.kernel.org Cc: "Paul E. McKenney" , Byungchul Park , Steven Rostedt , Peter Zijlstra , Ingo Molnar Subject: Re: [PATCH] lockdep: Print proper scenario if cross deadlock detected at acquisition time Message-ID: <20170919125859.GC10893@tardis> References: <20170919125218.17802-1-boqun.feng@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="TYecfFk8j8mZq+dy" Content-Disposition: inline In-Reply-To: <20170919125218.17802-1-boqun.feng@gmail.com> User-Agent: Mutt/1.9.0 (2017-09-02) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --TYecfFk8j8mZq+dy Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Sep 19, 2017 at 08:52:06PM +0800, Boqun Feng wrote: > For a potential deadlock about CROSSRELEASE as follow: >=20 > P1 P2 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > lock(A) > lock(X) > lock(A) > commit(X) >=20 > A: normal lock, X: cross lock >=20 > , we could detect it at two places: >=20 > 1. commit time: >=20 > We have run P1 first, and have dependency A --> X in graph, and > then we run P2, and find the deadlock. >=20 > 2. acquisition time: >=20 > We have run P2 first, and have dependency X --> A, in > graph(because another P3 may run previously and is acquiring for > lock X), and then we run P1 and find the deadlock. >=20 > In current print_circular_lock_scenario(), for 1) we could print the > right scenario and note that's a deadlock related to CROSSRELEASE, > however for 2) we print the scenario as a normal lockdep deadlock, > instead we print something like: Hmm... this sentence is redundant.. the paragraph should be: =2E.. for 2) we currenlty print the scenario as a normal lockdep deadlock: Apologies for this. Regards, Boqun >=20 > | [ 35.310179] =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 > | [ 35.310749] WARNING: possible circular locking dependency detected > | [ 35.310749] 4.13.0-rc4+ #1 Not tainted > | [ 35.310749] ------------------------------------------------------ > | [ 35.310749] torture_onoff/766 is trying to acquire lock: > | [ 35.313943] ((complete)&st->done){+.+.}, at: [] t= akedown_cpu+0x86/0xf0 > | [ 35.313943]=20 > | [ 35.313943] but task is already holding lock: > | [ 35.313943] (sparse_irq_lock){+.+.}, at: [] irq_l= ock_sparse+0x12/0x20 > | [ 35.313943]=20 > | [ 35.313943] which lock already depends on the new lock. > ... > | [ 35.313943] other info that might help us debug this: > | [ 35.313943]=20 > | [ 35.313943] Possible unsafe locking scenario: > | [ 35.313943]=20 > | [ 35.313943] CPU0 CPU1 > | [ 35.313943] ---- ---- > | [ 35.313943] lock(sparse_irq_lock); > | [ 35.313943] lock((complete)&st->done); > | [ 35.313943] lock(sparse_irq_lock); > | [ 35.313943] lock((complete)&st->done); > | [ 35.313943]=20 > | [ 35.313943] *** DEADLOCK *** >=20 > It's better to print a proper scenario related to CROSSRELEASE to help > users find their bugs more easily, so improve this. >=20 > Cc: "Paul E. McKenney" > Cc: Byungchul Park > Cc: Steven Rostedt > Signed-off-by: Boqun Feng > --- > The sample of print_circular_lock_scenario() is from Paul Mckenney. >=20 > kernel/locking/lockdep.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) >=20 > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c > index 44c8d0d17170..67a407bcc814 100644 > --- a/kernel/locking/lockdep.c > +++ b/kernel/locking/lockdep.c > @@ -1156,6 +1156,23 @@ print_circular_lock_scenario(struct held_lock *src, > __print_lock_name(target); > printk(KERN_CONT ");\n"); > printk("\n *** DEADLOCK ***\n\n"); > + } else if (cross_lock(src->instance)) { > + printk(" Possible unsafe locking scenario by crosslock:\n\n"); > + printk(" CPU0 CPU1\n"); > + printk(" ---- ----\n"); > + printk(" lock("); > + __print_lock_name(target); > + printk(KERN_CONT ");\n"); > + printk(" lock("); > + __print_lock_name(source); > + printk(KERN_CONT ");\n"); > + printk(" lock("); > + __print_lock_name(parent =3D=3D source ? target : parent); > + printk(KERN_CONT ");\n"); > + printk(" unlock("); > + __print_lock_name(source); > + printk(KERN_CONT ");\n"); > + printk("\n *** DEADLOCK ***\n\n"); > } else { > printk(" Possible unsafe locking scenario:\n\n"); > printk(" CPU0 CPU1\n"); > --=20 > 2.14.1 >=20 --TYecfFk8j8mZq+dy Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEEj5IosQTPz8XU1wRHSXnow7UH+rgFAlnBFJAACgkQSXnow7UH +ridgggAlzeyUHpUAqeW8N51BuIKs5o18BECPqr6JLMtwsAuN6zQim8RuBaZCu9m AyPXpplAbB7it6NuUUb3IlN0gNZq7Bohn5k8VYffOcrCmhM1fioGGBcQm/KHfqBe NZ9KXt6p2n7LN1VT05jwPMf0xIlYXX+BEnCqkJy43L24W08fh4wTF3534ExSwnaJ 7nBz9yA2gVpk1/MaX97bKFvHuKLt1NK95zjCcNfQ2FhobXn1vIWnMQg6x1RDOZU5 ZV/CGWgS18orXllZM9l2sb5KzoGgio/9iB32XRR0qOK/4w26rmj7ehIUk8qmtp74 fUSMgLMpTAzhOUYrkdxiRN1CkPNNXA== =NePm -----END PGP SIGNATURE----- --TYecfFk8j8mZq+dy--