From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ursula Braun Subject: Re: [PATCH net-next] net/smc: init conn.tx_work & conn.send_lock sooner Date: Thu, 17 May 2018 18:30:38 +0200 Message-ID: References: <20180517105421.182397-1-edumazet@google.com> <63bf1ebb-4a6f-43b8-1ee3-ced650e1dfdf@linux.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: David Miller , netdev , Eric Dumazet , linux-s390@vger.kernel.org To: Eric Dumazet Return-path: Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:58588 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751644AbeEQQbU (ORCPT ); Thu, 17 May 2018 12:31:20 -0400 Received: from pps.filterd (m0098410.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id w4HGUvPt046259 for ; Thu, 17 May 2018 12:31:20 -0400 Received: from e06smtp15.uk.ibm.com (e06smtp15.uk.ibm.com [195.75.94.111]) by mx0a-001b2d01.pphosted.com with ESMTP id 2j19uwuq67-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Thu, 17 May 2018 12:31:15 -0400 Received: from localhost by e06smtp15.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 17 May 2018 17:30:43 +0100 In-Reply-To: Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 05/17/2018 05:28 PM, Eric Dumazet wrote: > On Thu, May 17, 2018 at 6:58 AM Ursula Braun wrote: > > > >> On 05/17/2018 02:20 PM, Eric Dumazet wrote: >>> On Thu, May 17, 2018 at 5:13 AM Ursula Braun > wrote: >>> >>>> This problem should no longer show up with yesterday's net-next commit >>>> 569bc6436568 ("net/smc: no tx work trigger for fallback sockets"). >>> >>> It definitely triggers on latest net-next, which includes 569bc6436568 >>> >>> Thanks. >>> > >> Sorry, my fault. > >> Your proposed patch solves the problem. On the other hand the purpose of >> smc_tx_init() has been to cover tx-related socket initializations needed > for >> connection sockets only. tx_work is something that should be scheduled > only >> for active connection sockets in non-fallback mode. >> Thus I prefer this alternate patch to solve the problem: > >> --- >> net/smc/af_smc.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) > >> --- a/net/smc/af_smc.c >> +++ b/net/smc/af_smc.c >> @@ -1362,14 +1362,18 @@ static int smc_setsockopt(struct socket >> } >> break; >> case TCP_NODELAY: >> - if (sk->sk_state != SMC_INIT && sk->sk_state != > SMC_LISTEN) { >> + if (sk->sk_state != SMC_INIT && >> + sk->sk_state != SMC_LISTEN && >> + sk->sk_state != SMC_CLOSED) { >> if (val && !smc->use_fallback) >> mod_delayed_work(system_wq, > &smc->conn.tx_work, >> 0); >> } >> break; >> case TCP_CORK: >> - if (sk->sk_state != SMC_INIT && sk->sk_state != > SMC_LISTEN) { >> + if (sk->sk_state != SMC_INIT && >> + sk->sk_state != SMC_LISTEN && >> + sk->sk_state != SMC_CLOSED) { >> if (!val && !smc->use_fallback) >> mod_delayed_work(system_wq, > &smc->conn.tx_work, >> 0); > >> What do you think? > > I think my patch is cleaner. > > Deferring spinlock and workqueues setup is a recipe for disaster. > If your solution is preferred, I agree. In this case my today's net/smc patch net/smc: initialize tx_work before llc initial handshake for the net-tree is obsolete.