On Mon, Jan 31, 2022 at 08:13:53PM +0100, Stefan Raspl wrote: > On 1/30/22 19:02, Tony Lu wrote: > > According to the man page of TCP_CORK [1], if set, don't send out > > partial frames. All queued partial frames are sent when option is > > cleared again. > > > > When applications call setsockopt to disable TCP_CORK, this call is > > protected by lock_sock(), and tries to mod_delayed_work() to 0, in order > > to send pending data right now. However, the delayed work smc_tx_work is > > also protected by lock_sock(). There introduces lock contention for > > sending data. > > > > To fix it, send pending data directly which acts like TCP, without > > lock_sock() protected in the context of setsockopt (already lock_sock()ed), > > and cancel unnecessary dealyed work, which is protected by lock. > > > > [1] https://linux.die.net/man/7/tcp > > > > Signed-off-by: Tony Lu <tonylu@xxxxxxxxxxxxxxxxx> > > --- > > net/smc/af_smc.c | 4 ++-- > > net/smc/smc_tx.c | 25 +++++++++++++++---------- > > net/smc/smc_tx.h | 1 + > > 3 files changed, 18 insertions(+), 12 deletions(-) > > > > diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c > > index ffab9cee747d..ef021ec6b361 100644 > > --- a/net/smc/af_smc.c > > +++ b/net/smc/af_smc.c > > @@ -2600,8 +2600,8 @@ static int smc_setsockopt(struct socket *sock, int level, int optname, > > sk->sk_state != SMC_CLOSED) { > > if (!val) { > > SMC_STAT_INC(smc, cork_cnt); > > - mod_delayed_work(smc->conn.lgr->tx_wq, > > - &smc->conn.tx_work, 0); > > + smc_tx_pending(&smc->conn); > > + cancel_delayed_work(&smc->conn.tx_work); > > } > > } > > break; > > diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c > > index be241d53020f..7b0b6e24582f 100644 > > --- a/net/smc/smc_tx.c > > +++ b/net/smc/smc_tx.c > > @@ -597,27 +597,32 @@ int smc_tx_sndbuf_nonempty(struct smc_connection *conn) > > return rc; > > } > > -/* Wakeup sndbuf consumers from process context > > - * since there is more data to transmit > > - */ > > -void smc_tx_work(struct work_struct *work) > > +void smc_tx_pending(struct smc_connection *conn) > > Could you add a comment that we're expecting lock_sock() to be held when > calling this function? I will add it in the next separated patch. Thank you, Tony Lu
