Received: by 2002:a25:c593:0:0:0:0:0 with SMTP id v141csp979457ybe; Thu, 19 Sep 2019 06:55:55 -0700 (PDT) X-Google-Smtp-Source: APXvYqxGBOLLXbIbLdZLzsQHWmTVENHh/DMFiI079sp5MUlWcOjwVpTeYkGbm7ALETNgYwsZ0eiO X-Received: by 2002:a50:9402:: with SMTP id p2mr16138053eda.111.1568901355575; Thu, 19 Sep 2019 06:55:55 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1568901355; cv=none; d=google.com; s=arc-20160816; b=cX1jxoJmBEwNKjCCMkhORYkl41ugK40U0fLAxCf+cwEfvEsmD0NBH002lAmbabrVwm EqQBqoQzShlG32jXIQLUok77B8pNhHEBq9MH9m2W/LAMdo8LhsEOHsAqsxYXkO4cTmlk +NPd/xInYfw6jF7yiNzCIfERctmMg4hoEQgzd6cM4bVnx9iaZtQnvIZZoQeO3uWx1Dj8 +nm4QnNQtHiQLKgDkzbXFXkDcz5+/wIrusW/e6WJt3ZGyAMLsyFQyAR40F27l0+Nm0v/ nzzCc9HKOqaCIBudKGo9/06h9T1s5ecHaWv2jJucE2uGRu/JSJDDUyWh1jQOFweVGSs2 Q7GQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject:dkim-signature; bh=+3NvXqtBLDWzRCyshcHQ6a+vBUFhGZMLauBGugKUo3w=; b=A3I+9G7GwC5XIP2aVEnJzjzufxM8iZEsJcWcNgNAdawic6vLeUVOOlEmz0h+K5ZBE+ t0PNb0QvC9ETQ75w//imju/b0InGhlgwgWRl0ZZkNY3jv8LuZZZ2fzI9rotIeva+F7U2 WyfLMHHD59EnAUtTh/6hKKaAOSiCY8ng8TKDgyvQksK7MoRXPgDgP0pYkBqcU6jzKDlm JlG2jkvc8V5vT4bthk3qCmYPXY1NJIOWe+PzHkwLXGtoX1/hBxsdQpApXf7BkC+UGrKV OD3wzESertWPPwbPOxLDxNyt94O1gqZSzCg1WXbIYgKiCe2ZcxtN7JFO0TV0sEGJz50Z 21EA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@geanix.com header.s=first header.b=VMTVNpct; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=geanix.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id g15si5518693edm.349.2019.09.19.06.55.31; Thu, 19 Sep 2019 06:55:55 -0700 (PDT) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@geanix.com header.s=first header.b=VMTVNpct; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=geanix.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390446AbfISNBt (ORCPT + 99 others); Thu, 19 Sep 2019 09:01:49 -0400 Received: from first.geanix.com ([116.203.34.67]:38038 "EHLO first.geanix.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2388846AbfISNBs (ORCPT ); Thu, 19 Sep 2019 09:01:48 -0400 Received: from [192.168.8.20] (unknown [85.184.140.241]) by first.geanix.com (Postfix) with ESMTPSA id 9D2CA681A9; Thu, 19 Sep 2019 13:01:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=geanix.com; s=first; t=1568898101; bh=OrlqwK2SAAOFsSpde8lIVUxGZTfhMzH/o/6acmAaHjI=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=VMTVNpct4R2DY9WcONYPhvK8BXJr00zsIoaxhp+QQWifLueiubH76/o0AlhZWUuxo tZme9uFVopJPka55N1+YWsyOZSKTsRmLuqgiCQjB+NrsAdiTdLc96XE35cTLcigvvd B02Fd+zWCrsSEGRrHccJaNYaQXTevCxxpgRFp0dNTbzKdjytQXc7xDt2yoQ7bYl47V /Z0ua95vYF47BdBcqAfugjrRcx4hxscWSkeQUwPfekmwF7UUIexKa5ym6DZlSOSDJF k39+oPCcjz6Ixz/7/yPkcvSmaoS0lPtQ3ws52MXlC4r+yXGNG+lnyUSLSuZKxAOXNJ 3mEfLOjp8nPVA== Subject: Re: [PATCH] tty: n_gsm: avoid recursive locking with async port hangup To: Jiri Slaby , Greg Kroah-Hartman , Alan Cox , linux-kernel@vger.kernel.org Cc: Esben Haabendal , =?UTF-8?Q?Sean_Nyekj=c3=a6r?= References: <20190822215601.9028-1-martin@geanix.com> <4fd2d737-14a8-6fe6-16a1-c5e6d924f9e6@geanix.com> From: =?UTF-8?Q?Martin_Hundeb=c3=b8ll?= Message-ID: <4e206e1c-4f6d-bdc1-a435-9f4e8bb004ee@geanix.com> Date: Thu, 19 Sep 2019 15:01:16 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US-large Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3.1 required=4.0 tests=ALL_TRUSTED,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,URIBL_BLOCKED autolearn=disabled version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on b8b5098bc1bc Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 04/09/2019 10.18, Jiri Slaby wrote: > On 29. 08. 19, 21:42, Martin Hundebøll wrote: >> On 22/08/2019 23.56, Martin Hundebøll wrote: >>> When tearing down the n_gsm ldisc while one or more of its child ports >>> are open, a lock dep warning occurs: >>> >>> [   56.254258] ====================================================== >>> [   56.260447] WARNING: possible circular locking dependency detected >>> [   56.266641] 5.2.0-00118-g1fd58e20e5b0 #30 Not tainted >>> [   56.271701] ------------------------------------------------------ >>> [   56.277890] cmux/271 is trying to acquire lock: >>> [   56.282436] 8215283a (&tty->legacy_mutex){+.+.}, at: >>> __tty_hangup.part.0+0x58/0x27c >>> [   56.290128] >>> [   56.290128] but task is already holding lock: >>> [   56.295970] e9e2b842 (&gsm->mutex){+.+.}, at: >>> gsm_cleanup_mux+0x9c/0x15c >>> [   56.302699] >>> [   56.302699] which lock already depends on the new lock. >>> [   56.302699] >>> [   56.310884] >>> [   56.310884] the existing dependency chain (in reverse order) is: >>> [   56.318372] >>> [   56.318372] -> #2 (&gsm->mutex){+.+.}: >>> [   56.323624]        mutex_lock_nested+0x1c/0x24 >>> [   56.328079]        gsm_cleanup_mux+0x9c/0x15c >>> [   56.332448]        gsmld_ioctl+0x418/0x4e8 >>> [   56.336554]        tty_ioctl+0x96c/0xcb0 >>> [   56.340492]        do_vfs_ioctl+0x41c/0xa5c >>> [   56.344685]        ksys_ioctl+0x34/0x60 >>> [   56.348535]        ret_fast_syscall+0x0/0x28 >>> [   56.352815]        0xbe97cc04 >>> [   56.355791] >>> [   56.355791] -> #1 (&tty->ldisc_sem){++++}: >>> [   56.361388]        tty_ldisc_lock+0x50/0x74 >>> [   56.365581]        tty_init_dev+0x88/0x1c4 >>> [   56.369687]        tty_open+0x1c8/0x430 >>> [   56.373536]        chrdev_open+0xa8/0x19c >>> [   56.377560]        do_dentry_open+0x118/0x3c4 >>> [   56.381928]        path_openat+0x2fc/0x1190 >>> [   56.386123]        do_filp_open+0x68/0xd4 >>> [   56.390146]        do_sys_open+0x164/0x220 >>> [   56.394257]        kernel_init_freeable+0x328/0x3e4 >>> [   56.399146]        kernel_init+0x8/0x110 >>> [   56.403078]        ret_from_fork+0x14/0x20 >>> [   56.407183]        0x0 >>> [   56.409548] >>> [   56.409548] -> #0 (&tty->legacy_mutex){+.+.}: >>> [   56.415402]        __mutex_lock+0x64/0x90c >>> [   56.419508]        mutex_lock_nested+0x1c/0x24 >>> [   56.423961]        __tty_hangup.part.0+0x58/0x27c >>> [   56.428676]        gsm_cleanup_mux+0xe8/0x15c >>> [   56.433043]        gsmld_close+0x48/0x90 >>> [   56.436979]        tty_ldisc_kill+0x2c/0x6c >>> [   56.441173]        tty_ldisc_release+0x88/0x194 >>> [   56.445715]        tty_release_struct+0x14/0x44 >>> [   56.450254]        tty_release+0x36c/0x43c >>> [   56.454365]        __fput+0x94/0x1e8 >>> >>> Avoid the warning by doing the port hangup asynchronously. >> >> Any comments? > > I did not manage to reply before vacation, and after having "work = > NULL" in my head, I forgot, sorry. > > At the same time, I am a bit lost in the lockdep chain above. It mixes > close (#0), open (#1) and ioctl (#2), so how is this a "chain" in the > first place? The close is from my cmux program, that configured the line discpline. The open could be from my other process reading the virtual tty (gsmtty1)? Or is it a lock taken during the opening of the physical tty? Can the ioctl call used to configure the line discipline take a lock that is not released until the line discipline is changed back? > BTW, do you see an actual deadlock? And what tty driver do you use for > backend devices? I.e. what ttys do you set this ldisc to? After the first deadlock, I can use the UART, and configure GSM0710 multiplexing again, but the virtual tty's don't work: root@iwg26:~# cat /dev/gsmtty1 cat: can't open '/dev/gsmtty1': Protocol driver not attached This is on an i.MX6 ULL using its imx uart driver (drivers/tty/serial/imx.c). > See also the comment below. > >>> Signed-off-by: Martin Hundebøll >>> --- >>>   drivers/tty/n_gsm.c | 2 +- >>>   1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c >>> index d30525946892..36a3eb4ad4c5 100644 >>> --- a/drivers/tty/n_gsm.c >>> +++ b/drivers/tty/n_gsm.c >>> @@ -1716,7 +1716,7 @@ static void gsm_dlci_release(struct gsm_dlci *dlci) >>>           gsm_destroy_network(dlci); >>>           mutex_unlock(&dlci->mutex); >>>   -        tty_vhangup(tty); >>> +        tty_hangup(tty); >>>             tty_port_tty_set(&dlci->port, NULL); > > I am afraid there is changed semantics now: the scheduled hangup will > likely happen when the tty in tty_port is set to NULL already, so some > operations done in tty->ops->hangup might be a nop now. For example the > commonly used tty_port_hangup won't set TTY_IO_ERROR on the tty and > won't lower DTR and RTS on the line either. Is the hangup for the physical uart (ttymxc0), or the virtual ttys (gsmtty1)? In the latter case there wouldn't be any control lines to reset. Thanks, Martin