On 17.10.23 04:06, D. Wythe wrote:
On 10/13/23 3:04 AM, Wenjia Zhang wrote:
On 11.10.23 09:33, D. Wythe wrote:
From: "D. Wythe" <alibuda@xxxxxxxxxxxxxxxxx>
Note that we always hold a reference to sock when attempting
to submit close_work.
yes
Therefore, if we have successfully
canceled close_work from pending, we MUST release that reference
to avoid potential leaks.
Isn't the corresponding reference already released inside the
smc_close_passive_work()?
Hi Wenjia,
If we successfully cancel the close work from the pending state,
it means that smc_close_passive_work() has never been executed.
You can find more details here.
/**
* cancel_work_sync - cancel a work and wait for it to finish
* @work:the work to cancel
*
* Cancel @work and wait for its execution to finish. This function
* can be used even if the work re-queues itself or migrates to
* another workqueue. On return from this function, @work is
* guaranteed to be not pending or executing on any CPU.
*
* cancel_work_sync(&delayed_work->work) must not be used for
* delayed_work's. Use cancel_delayed_work_sync() instead.
*
* The caller must ensure that the workqueue on which @work was last
* queued can't be destroyed before this function returns.
*
* Return:
* %true if @work was pending, %false otherwise.
*/
boolcancel_work_sync(structwork_struct *work)
{
return__cancel_work_timer(work, false);
}
Best wishes,
D. Wythe
As I understand, queue_work() would wake up the work if the work is not
already on the queue. And the sock_hold() is just prio to the
queue_work(). That means, cancel_work_sync() would cancel the work
either before its execution or after. If your fix refers to the former
case, at this moment, I don't think the reference can be hold, thus it
is unnecessary to put it.
Fixes: 42bfba9eaa33 ("net/smc: immediate termination for SMCD link
groups")
Signed-off-by: D. Wythe <alibuda@xxxxxxxxxxxxxxxxx>
---
net/smc/smc_close.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c
index 449ef45..10219f5 100644
--- a/net/smc/smc_close.c
+++ b/net/smc/smc_close.c
@@ -116,7 +116,8 @@ static void smc_close_cancel_work(struct smc_sock
*smc)
struct sock *sk = &smc->sk;
release_sock(sk);
- cancel_work_sync(&smc->conn.close_work);
+ if (cancel_work_sync(&smc->conn.close_work))
+ sock_put(sk);
cancel_delayed_work_sync(&smc->conn.tx_work);
lock_sock(sk);
}
