Re: [RFC PATCH 0/2] Two RFC patches for the same SMC socket wait queue mismatch issue

From: Karsten Graul <kgraul@linux.ibm.com>
To: Wen Gu <guwen@linux.alibaba.com>, tonylu@linux.alibaba.com
Cc: davem@davemloft.net, kuba@kernel.org, linux-s390@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	dust.li@linux.alibaba.com, xuanzhuo@linux.alibaba.com
Subject: Re: [RFC PATCH 0/2] Two RFC patches for the same SMC socket wait queue mismatch issue
Date: Thu, 11 Nov 2021 15:21:15 +0100	[thread overview]
Message-ID: <369755c0-8b3e-cf69-d7f2-8993700efc4a@linux.ibm.com> (raw)
In-Reply-To: <1636548651-44649-1-git-send-email-guwen@linux.alibaba.com>

On 10/11/2021 13:50, Wen Gu wrote:
> Hi, Karsten
> 
> Thanks for your reply. The previous discussion about the issue of socket
> wait queue mismatch in SMC fallback can be referred from:
> https://lore.kernel.org/all/db9acf73-abef-209e-6ec2-8ada92e2cfbc@linux.ibm.com/
> 
> This set of patches includes two RFC patches, they are both aimed to fix
> the same issue, the mismatch of socket wait queue in SMC fallback.
> 
> In your last reply, I am suggested to add the complete description about
> the intention of initial patch in order that readers can understand the
> idea behind it. This has been done in "[RFC PATCH net v2 0/2] net/smc: Fix
> socket wait queue mismatch issue caused by fallback" of this mail.
> 
> Unfortunately, I found a defect later in the solution of the initial patch
> or the v2 patch mentioned above. The defect is about fasync_list and related
> to 67f562e3e14 ("net/smc: transfer fasync_list in case of fallback").
> 
> When user applications use sock_fasync() to insert entries into fasync_list,
> the wait queue they operate is smc socket->wq. But in initial patch or
> the v2 patch, I swapped sk->sk_wq of smc socket and clcsocket in smc_create(),
> thus the sk_data_ready / sk_write_space.. of smc will wake up clcsocket->wq
> finally. So the entries added into smc socket->wq.fasync_list won't be woken
> up at all before fallback.
> 
> So the solution in initial patch or the v2 patch of this mail by swapping
> sk->sk_wq of smc socket and clcsocket seems a bad way to fix this issue.
> 
> Therefore, I tried another solution by removing the wait queue entries from
> smc socket->wq to clcsocket->wq during the fallback, which is described in the
> "[RFC PATCH net 2/2] net/smc: Transfer remaining wait queue entries" of this
> mail. In our test environment, this patch can fix the fallback issue well.

Still running final tests but overall its working well here, too.
Until we maybe find a 'cleaner' solution if this I would like to go with your
current fixes. But I would like to improve the wording of the commit message and
the comments a little bit if you are okay with that.

If you send a new series with the 2 patches then I would take them and post them
to the list again with my changes.

What do you think?