Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Jon Maloy <jon.maloy@ericsson.com>
To:     Ying Xue <ying.xue@windriver.com>,
        "dvyukov@google.com" <dvyukov@google.com>
CC:     "davem@davemloft.net" <davem@davemloft.net>,
        "parthasarathy.bhuvaragan@ericsson.com" 
        <parthasarathy.bhuvaragan@ericsson.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "tipc-discussion@lists.sourceforge.net" 
        <tipc-discussion@lists.sourceforge.net>
Subject: RE: [PATCH net] tipc: eliminate possible recursive locking detected
 by LOCKDEP
Thread-Topic: [PATCH net] tipc: eliminate possible recursive locking detected
 by LOCKDEP
Thread-Index: AQHUYVqpPhlpQDofhUe3cmdRqVx8uqUaBE5g
Date:   Thu, 11 Oct 2018 13:30:09 +0000
Message-ID: <BN6PR15MB1506DF62FF6856794B109C4A9AE10@BN6PR15MB1506.namprd15.prod.outlook.com>
References: <1539259076-8562-1-git-send-email-ying.xue@windriver.com>
In-Reply-To: <1539259076-8562-1-git-send-email-ying.xue@windriver.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: ericsson.com does not designate
 permitted sender hosts)
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 6b441e1f-15aa-482b-bd16-08d62f7daa56
X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Oct 2018 13:30:09.2230
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 92e84ceb-fbfd-47ab-be52-080c6b87953f
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR15MB1220
X-OriginatorOrg: ericsson.com
X-Brightmail-Tracker: H4sIAAAAAAAAA02Se0hTURzHOffe3V2txWn5+GEpMZRk5XwQMeiBEdoIShECX1gzb2rqlE1N
        k2KUlBnWhBQ1xSnDUoLh2kzcUrbIZzHN1IoMzUear8qoLJXcrkX/fb7n+z2/x+EwpLCB58Wk
        KrJZpUKeLqJdqcrox3kBisiOuKCa9wJptb2QkmrGr/Olg23VtLRT6yE12i9IJ0ZGyFBaZmx8
        Q8i0hhyZWbtMyJYNPjK90URE8mJdDyWx6am5rDLwyFnXlKnFGTrrrn/e7Cc9oUZWUTFyYQDv
        h4WVHqoYuTJC/AxBi9VOcOI7goHxOv4/cfNaBY8TOgJGu/qdgsIaEnQddxDnlBHwzTZNc+ID
        guZ3jbSjDY39Yba8CDnYDUdBy2on6QiR+AUBSzWthMPYgWNgwrrC40KxYG2e4HMcAvM1S6SD
        KewHhuZSysECHA/2G93Ou0IcBguL/c4GLjgcSueGnHUQ9oAfvQ+dGRJ7wtvJWoLbG4POYic5
        dofZifWNPLORj4G+qRjueDf0DqkRx97wsvbWJrfz4V5lMscB8LmsbLPMSVgcXHc+C+AuBGOD
        fZuGGNZ0JTyO06DiiZnmuAbBqEmiQSFV/43H8T7Qmr/SHO+Fhro5ssq58nboqZyktIhqQu4q
        VpWYkRwSImGVqedUqkyFRMFmG9DG77Eafwe1otmPR20IM0i0VUAe74gT8uS5qvwMGwKGFLkJ
        Aiva44SCJHn+JVaZeUaZk86qbGgnQ4k8BdJTj2KFOFmezaaxbBar/OsSjIuXGt03XQ0a5vvq
        b+MmzXiqpT7hfIFu1BS92HJxmy1+UHPg2IMrMYHDGQUH1eqxiMTQwrBfsY1mbwKSw3v7fCL0
        c6Gv2mQNrwvchm3j5hJfy9qu4pmnwcGmWgu5RXpY9NwSPyBejSrfIxGXnNB3m62Seubn6fnL
        XwyNDQm5wdN+RSJKlSIPFpNKlfwPtu4SwTkDAAA=
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Acked-by: Jon Maloy <jon.maloy@ericsson.com>

///jon


> -----Original Message-----
> From: Ying Xue <ying.xue@windriver.com>
> Sent: October 11, 2018 7:58 AM
> To: Jon Maloy <jon.maloy@ericsson.com>; dvyukov@google.com
> Cc: davem@davemloft.net; parthasarathy.bhuvaragan@ericsson.com;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; tipc-
> discussion@lists.sourceforge.net
> Subject: [PATCH net] tipc: eliminate possible recursive locking detected =
by
> LOCKDEP
>=20
> When booting kernel with LOCKDEP option, below warning info was found:
>=20
> WARNING: possible recursive locking detected 4.19.0-rc7+ #14 Not tainted
> --------------------------------------------
> swapper/0/1 is trying to acquire lock:
> 00000000dcfc0fc8 (&(&list->lock)->rlock#4){+...}, at: spin_lock_bh
> include/linux/spinlock.h:334 [inline]
> 00000000dcfc0fc8 (&(&list->lock)->rlock#4){+...}, at:
> tipc_link_reset+0x125/0xdf0 net/tipc/link.c:850
>=20
> but task is already holding lock:
> 00000000cbb9b036 (&(&list->lock)->rlock#4){+...}, at: spin_lock_bh
> include/linux/spinlock.h:334 [inline]
> 00000000cbb9b036 (&(&list->lock)->rlock#4){+...}, at:
> tipc_link_reset+0xfa/0xdf0 net/tipc/link.c:849
>=20
> other info that might help us debug this:
>  Possible unsafe locking scenario:
>=20
>        CPU0
>        ----
>   lock(&(&list->lock)->rlock#4);
>   lock(&(&list->lock)->rlock#4);
>=20
>  *** DEADLOCK ***
>=20
>  May be due to missing lock nesting notation
>=20
> 2 locks held by swapper/0/1:
>  #0: 00000000f7539d34 (pernet_ops_rwsem){+.+.}, at:
> register_pernet_subsys+0x19/0x40 net/core/net_namespace.c:1051
>  #1: 00000000cbb9b036 (&(&list->lock)->rlock#4){+...}, at:
> spin_lock_bh include/linux/spinlock.h:334 [inline]
>  #1: 00000000cbb9b036 (&(&list->lock)->rlock#4){+...}, at:
> tipc_link_reset+0xfa/0xdf0 net/tipc/link.c:849
>=20
> stack backtrace:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.19.0-rc7+ #14 Hardware name:
> QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014 Call Tra=
ce:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x1af/0x295 lib/dump_stack.c:113  print_deadlock_bug
> kernel/locking/lockdep.c:1759 [inline]  check_deadlock
> kernel/locking/lockdep.c:1803 [inline]  validate_chain
> kernel/locking/lockdep.c:2399 [inline]
>  __lock_acquire+0xf1e/0x3c60 kernel/locking/lockdep.c:3411
>  lock_acquire+0x1db/0x520 kernel/locking/lockdep.c:3900
> __raw_spin_lock_bh include/linux/spinlock_api_smp.h:135 [inline]
>  _raw_spin_lock_bh+0x31/0x40 kernel/locking/spinlock.c:168  spin_lock_bh
> include/linux/spinlock.h:334 [inline]
>  tipc_link_reset+0x125/0xdf0 net/tipc/link.c:850
>  tipc_link_bc_create+0xb5/0x1f0 net/tipc/link.c:526
>  tipc_bcast_init+0x59b/0xab0 net/tipc/bcast.c:521
>  tipc_init_net+0x472/0x610 net/tipc/core.c:82
>  ops_init+0xf7/0x520 net/core/net_namespace.c:129
> __register_pernet_operations net/core/net_namespace.c:940 [inline]
>  register_pernet_operations+0x453/0xac0 net/core/net_namespace.c:1011
>  register_pernet_subsys+0x28/0x40 net/core/net_namespace.c:1052
>  tipc_init+0x83/0x104 net/tipc/core.c:140  do_one_initcall+0x109/0x70a
> init/main.c:885  do_initcall_level init/main.c:953 [inline]  do_initcalls
> init/main.c:961 [inline]  do_basic_setup init/main.c:979 [inline]
> kernel_init_freeable+0x4bd/0x57f init/main.c:1144
>  kernel_init+0x13/0x180 init/main.c:1063
>  ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:413
>=20
> The reason why the noise above was complained by LOCKDEP is because we
> nested to hold l->wakeupq.lock and l->inputq->lock in tipc_link_reset
> function. In fact it's unnecessary to move skb buffer from l->wakeupq que=
ue
> to l->inputq queue while holding the two locks at the same time.
> Instead, we can move skb buffers in l->wakeupq queue to a temporary list
> first and then move the buffers of the temporary list to l->inputq queue,
> which is also safe for us.
>=20
> Fixes: 3f32d0be6c16 ("tipc: lock wakeup & inputq at tipc_link_reset()")
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Ying Xue <ying.xue@windriver.com>
> ---
>  net/tipc/link.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>=20
> diff --git a/net/tipc/link.c b/net/tipc/link.c index fb886b5..1d21ae4 100=
644
> --- a/net/tipc/link.c
> +++ b/net/tipc/link.c
> @@ -843,14 +843,21 @@ static void link_prepare_wakeup(struct tipc_link *l=
)
>=20
>  void tipc_link_reset(struct tipc_link *l)  {
> +	struct sk_buff_head list;
> +
> +	__skb_queue_head_init(&list);
> +
>  	l->in_session =3D false;
>  	l->session++;
>  	l->mtu =3D l->advertised_mtu;
> +
>  	spin_lock_bh(&l->wakeupq.lock);
> +	skb_queue_splice_init(&l->wakeupq, &list);
> +	spin_unlock_bh(&l->wakeupq.lock);
> +
>  	spin_lock_bh(&l->inputq->lock);
> -	skb_queue_splice_init(&l->wakeupq, l->inputq);
> +	skb_queue_splice_init(&list, l->inputq);
>  	spin_unlock_bh(&l->inputq->lock);
> -	spin_unlock_bh(&l->wakeupq.lock);
>=20
>  	__skb_queue_purge(&l->transmq);
>  	__skb_queue_purge(&l->deferdq);
> --
> 2.7.4