From patchwork Sun Sep 26 00:14:48 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Gustavo F. Padovan" X-Patchwork-Id: 65769 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 8FA53B70B8 for ; Sun, 26 Sep 2010 10:28:27 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754940Ab0IZA1m (ORCPT ); Sat, 25 Sep 2010 20:27:42 -0400 Received: from mail-gw0-f46.google.com ([74.125.83.46]:61238 "EHLO mail-gw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752459Ab0IZA1l (ORCPT ); Sat, 25 Sep 2010 20:27:41 -0400 Received: by gwj17 with SMTP id 17so1286293gwj.19 for ; Sat, 25 Sep 2010 17:27:40 -0700 (PDT) Received: by 10.101.8.8 with SMTP id l8mr6033271ani.84.1285460860116; Sat, 25 Sep 2010 17:27:40 -0700 (PDT) Received: from vigoh ([201.82.129.65]) by mx.google.com with ESMTPS id c7sm6384376ana.18.2010.09.25.17.27.36 (version=TLSv1/SSLv3 cipher=RC4-MD5); Sat, 25 Sep 2010 17:27:37 -0700 (PDT) Date: Sat, 25 Sep 2010 21:14:48 -0300 From: "Gustavo F. Padovan" To: David Miller Cc: netdev@vger.kernel.org, linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org, marcel@holtmann.org, mathewm@codeaurora.org Subject: Re: Deadlock in Bluetooth code in 2.6.36 Message-ID: <20100926001448.GA2979@vigoh> References: <1285104013-9946-1-git-send-email-padovan@profusion.mobi> <20100924.211824.242130957.davem@davemloft.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20100924.211824.242130957.davem@davemloft.net> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org * David Miller [2010-09-24 21:18:24 -0700]: > From: "Gustavo F. Padovan" > Date: Tue, 21 Sep 2010 18:20:12 -0300 > > > My questions here is on how to fix this properly. Maybe > > sock_alloc_send_skb() should not be used with SOCK_STREAM and reliable > > protocols and I'm not aware of that? And should I use something like > > sk_stream_alloc_skb() instead? > > > > Any comments are welcome. With lucky we can fix that for 2.6.36 and > > together with others fixes we have queued deliver a fully functional > > L2CAP layer on 2.6.36. > > Use sock_alloc_send_skb() as you do now, but if it fails wait for socket > space to become available just like TCP does, then loop back and try to > allocate again if the space-wait doesn't return an error. sock_alloc_send_skb() doesn't fail when there is no space available it sleeps and try again later. That is the problem. So if sock_alloc_send_skb() is called with the socket lock held potentially we can have a starvation in the backlog queue and then the deadlock due to be unable to read the acknowledgments packets residing in the backlog queue. Our current solution is release the lock before call sock_alloc_send_skb() and acquire it again when sock_alloc_send_skb() returns. Patch follows: ------ Bluetooth: Fix deadlock in the ERTM logic The Enhanced Retransmission Mode(ERTM) is a reliable mode of operation of the Bluetooth L2CAP layer. Think on it like a simplified version of TCP. The problem we were facing here was a deadlock. ERTM uses a backlog queue to queue incomimg packets while the user is helding the lock. At some moment the sk_sndbuf can be exceeded and we can't alloc new skbs then the code sleep with the lock to wait for memory, that stalls the ERTM connection once we can't read the acknowledgements packets in the backlog queue to free memory and make the allocation of outcoming skb successful. This patch actually affect all users of bt_skb_send_alloc(), i.e., all L2CAP modes and SCO. Signed-off-by: Gustavo F. Padovan Signed-off-by: Ulisses Furquim --- include/net/bluetooth/bluetooth.h | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h index 27a902d..118b69b 100644 --- a/include/net/bluetooth/bluetooth.h +++ b/include/net/bluetooth/bluetooth.h @@ -161,10 +161,21 @@ static inline struct sk_buff *bt_skb_send_alloc(struct sock *sk, unsigned long l { struct sk_buff *skb; + release_sock(sk); if ((skb = sock_alloc_send_skb(sk, len + BT_SKB_RESERVE, nb, err))) { skb_reserve(skb, BT_SKB_RESERVE); bt_cb(skb)->incoming = 0; } + lock_sock(sk); + + if (*err) + return NULL; + + *err = sock_error(sk); + if (*err) { + kfree_skb(skb); + return NULL; + } return skb; }