From: Dust Li <dust.li@linux.alibaba.com> Date: 2022-03-01 09:44:12
Hi:
This series tries to improve the performance of SMC in datapath.
- patch #1, add sysctl interface to support tuning the behaviour of
SMC in container environment.
- patch #2/#3, add autocorking support which is very efficient for small
messages without trade-off for latency.
- patch #4, send directly on setting TCP_NODELAY, without wake up the
TX worker, this make it consistent with clearing TCP_CORK.
- patch #5, this correct the setting of RMB window update limit, so
we don't send CDC messages to update peer's RMB window too frequently
in some cases.
- patch #6, implemented something like NAPI in SMC, decrease the number
of hardirq when busy.
- patch #7, this moves TX work doing in the BH to the user context when
sock_lock is hold by user.
With this patchset applied, we can get a good performance gain:
- qperf tcp_bw test has shown a great improvement. Other benchmarks like
'netperf TCP_STREAM' or 'sockperf throughput' has similar result.
- In my testing environment, running qperf tcp_bw and tcp_lat, SMC behaves
better then TCP in most all message size.
Here are some test results with the following testing command:
client: smc_run taskset -c 1 qperf smc-server -oo msg_size:1:64K:*2 \
-t 30 -vu tcp_{bw|lat}
server: smc_run taskset -c 1 qperf
==== Bandwidth ====
MsgSize Origin SMC TCP SMC with patches
1 0.578 MB/s 2.392 MB/s(313.57%) 2.561 MB/s(342.83%)
2 1.159 MB/s 4.780 MB/s(312.53%) 5.162 MB/s(345.46%)
4 2.283 MB/s 10.266 MB/s(349.77%) 10.122 MB/s(343.46%)
8 4.668 MB/s 19.040 MB/s(307.86%) 20.521 MB/s(339.59%)
16 9.147 MB/s 38.904 MB/s(325.31%) 40.823 MB/s(346.29%)
32 18.369 MB/s 79.587 MB/s(333.25%) 80.535 MB/s(338.42%)
64 36.562 MB/s 148.668 MB/s(306.61%) 158.170 MB/s(332.60%)
128 72.961 MB/s 274.913 MB/s(276.80%) 316.217 MB/s(333.41%)
256 144.705 MB/s 512.059 MB/s(253.86%) 626.019 MB/s(332.62%)
512 288.873 MB/s 884.977 MB/s(206.35%) 1221.596 MB/s(322.88%)
1024 574.180 MB/s 1337.736 MB/s(132.98%) 2203.156 MB/s(283.70%)
2048 1095.192 MB/s 1865.952 MB/s( 70.38%) 3036.448 MB/s(177.25%)
4096 2066.157 MB/s 2380.337 MB/s( 15.21%) 3834.271 MB/s( 85.58%)
8192 3717.198 MB/s 2733.073 MB/s(-26.47%) 4904.910 MB/s( 31.95%)
16384 4742.221 MB/s 2958.693 MB/s(-37.61%) 5220.272 MB/s( 10.08%)
32768 5349.550 MB/s 3061.285 MB/s(-42.77%) 5321.865 MB/s( -0.52%)
65536 5162.919 MB/s 3731.408 MB/s(-27.73%) 5245.021 MB/s( 1.59%)
==== Latency ====
MsgSize Origin SMC TCP SMC with patches
1 10.540 us 11.938 us( 13.26%) 10.356 us( -1.75%)
2 10.996 us 11.992 us( 9.06%) 10.073 us( -8.39%)
4 10.229 us 11.687 us( 14.25%) 9.996 us( -2.28%)
8 10.203 us 11.653 us( 14.21%) 10.063 us( -1.37%)
16 10.530 us 11.313 us( 7.44%) 10.013 us( -4.91%)
32 10.241 us 11.586 us( 13.13%) 10.081 us( -1.56%)
64 10.693 us 11.652 us( 8.97%) 9.986 us( -6.61%)
128 10.597 us 11.579 us( 9.27%) 10.262 us( -3.16%)
256 10.409 us 11.957 us( 14.87%) 10.148 us( -2.51%)
512 11.088 us 12.505 us( 12.78%) 10.206 us( -7.95%)
1024 11.240 us 12.255 us( 9.03%) 10.631 us( -5.42%)
2048 11.485 us 16.970 us( 47.76%) 10.981 us( -4.39%)
4096 12.077 us 13.948 us( 15.49%) 11.847 us( -1.90%)
8192 13.683 us 16.693 us( 22.00%) 13.336 us( -2.54%)
16384 16.470 us 23.615 us( 43.38%) 16.519 us( 0.30%)
32768 22.540 us 40.966 us( 81.75%) 22.452 us( -0.39%)
65536 34.192 us 73.003 us(113.51%) 33.916 us( -0.81%)
------------
Test environment notes:
1. Testing is run on 2 VMs within the same physical host
2. The NIC is ConnectX-4Lx, using SRIOV, and passing through 2 VFs to the
2 VMs respectively.
3. To decrease jitter, VM's vCPU are binded to each physical CPU, and those
physical CPUs are all isolated using boot parameter `isolcpus=xxx`
4. The queue number are set to 1, and interrupt from the queue is binded to
CPU0 in the guest
Dust Li (7):
net/smc: add sysctl interface for SMC
net/smc: add autocorking support
net/smc: add sysctl for autocorking
net/smc: send directly on setting TCP_NODELAY
net/smc: correct settings of RMB window update limit
net/smc: don't req_notify until all CQEs drained
net/smc: don't send in the BH context if sock_owned_by_user
Documentation/networking/smc-sysctl.rst | 23 +++++
include/net/netns/smc.h | 4 +
net/smc/Makefile | 2 +-
net/smc/af_smc.c | 30 ++++++-
net/smc/smc.h | 6 ++
net/smc/smc_cdc.c | 24 ++++--
net/smc/smc_core.c | 2 +-
net/smc/smc_sysctl.c | 80 ++++++++++++++++++
net/smc/smc_sysctl.h | 32 +++++++
net/smc/smc_tx.c | 107 +++++++++++++++++++++---
net/smc/smc_wr.c | 49 ++++++-----
11 files changed, 317 insertions(+), 42 deletions(-)
create mode 100644 Documentation/networking/smc-sysctl.rst
create mode 100644 net/smc/smc_sysctl.c
create mode 100644 net/smc/smc_sysctl.h
--
2.19.1.3.ge56e4f7
From: Dust Li <dust.li@linux.alibaba.com> Date: 2022-03-01 09:44:12
This patch add sysctl interface to support container environment
for SMC as we talk in the mail list.
Link: https://lore.kernel.org/netdev/20220224020253.GF5443@linux.alibaba.com
Co-developed-by: Tony Lu <tonylu@linux.alibaba.com>
Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
Signed-off-by: Dust Li <dust.li@linux.alibaba.com>
---
include/net/netns/smc.h | 3 ++
net/smc/Makefile | 2 +-
net/smc/af_smc.c | 10 ++++++
net/smc/smc_sysctl.c | 70 +++++++++++++++++++++++++++++++++++++++++
net/smc/smc_sysctl.h | 32 +++++++++++++++++++
5 files changed, 116 insertions(+), 1 deletion(-)
create mode 100644 net/smc/smc_sysctl.c
create mode 100644 net/smc/smc_sysctl.h
From: Dust Li <dust.li@linux.alibaba.com> Date: 2022-03-01 09:44:15
This patch adds autocorking support for SMC which could improve
throughput for small message by x3+.
The main idea is borrowed from TCP autocorking with some RDMA
specific modification:
1. The first message should never cork to make sure we won't
bring extra latency
2. If we have posted any Tx WRs to the NIC that have not
completed, cork the new messages until:
a) Receive CQE for the last Tx WR
b) We have corked enough message on the connection
3. Try to push the corked data out when we receive CQE of
the last Tx WR to prevent the corked messages hang in
the send queue.
Both SMC autocorking and TCP autocorking check the TX completion
to decide whether we should cork or not. The difference is
when we got a SMC Tx WR completion, the data have been confirmed
by the RNIC while TCP TX completion just tells us the data
have been sent out by the local NIC.
Add an atomic variable tx_pushing in smc_connection to make
sure only one can send to let it cork more and save CDC slot.
SMC autocorking should not bring extra latency since the first
message will always been sent out immediately.
The qperf tcp_bw test shows more than x4 increase under small
message size with Mellanox connectX4-Lx, same result with other
throughput benchmarks like sockperf/netperf.
The qperf tcp_lat test shows SMC autocorking has not increase any
ping-pong latency.
Test command:
client: smc_run taskset -c 1 qperf smc-server -oo msg_size:1:64K:*2 \
-t 30 -vu tcp_{bw|lat}
server: smc_run taskset -c 1 qperf
=== Bandwidth ====
MsgSize(Bytes) SMC-NoCork TCP SMC-AutoCorking
1 0.578 MB/s 2.392 MB/s(313.57%) 2.647 MB/s(357.72%)
2 1.159 MB/s 4.780 MB/s(312.53%) 5.153 MB/s(344.71%)
4 2.283 MB/s 10.266 MB/s(349.77%) 10.363 MB/s(354.02%)
8 4.668 MB/s 19.040 MB/s(307.86%) 21.215 MB/s(354.45%)
16 9.147 MB/s 38.904 MB/s(325.31%) 41.740 MB/s(356.32%)
32 18.369 MB/s 79.587 MB/s(333.25%) 82.392 MB/s(348.52%)
64 36.562 MB/s 148.668 MB/s(306.61%) 161.564 MB/s(341.89%)
128 72.961 MB/s 274.913 MB/s(276.80%) 325.363 MB/s(345.94%)
256 144.705 MB/s 512.059 MB/s(253.86%) 633.743 MB/s(337.96%)
512 288.873 MB/s 884.977 MB/s(206.35%) 1250.681 MB/s(332.95%)
1024 574.180 MB/s 1337.736 MB/s(132.98%) 2246.121 MB/s(291.19%)
2048 1095.192 MB/s 1865.952 MB/s( 70.38%) 2057.767 MB/s( 87.89%)
4096 2066.157 MB/s 2380.337 MB/s( 15.21%) 2173.983 MB/s( 5.22%)
8192 3717.198 MB/s 2733.073 MB/s(-26.47%) 3491.223 MB/s( -6.08%)
16384 4742.221 MB/s 2958.693 MB/s(-37.61%) 4637.692 MB/s( -2.20%)
32768 5349.550 MB/s 3061.285 MB/s(-42.77%) 5385.796 MB/s( 0.68%)
65536 5162.919 MB/s 3731.408 MB/s(-27.73%) 5223.890 MB/s( 1.18%)
==== Latency ====
MsgSize(Bytes) SMC-NoCork TCP SMC-AutoCorking
1 10.540 us 11.938 us( 13.26%) 10.573 us( 0.31%)
2 10.996 us 11.992 us( 9.06%) 10.269 us( -6.61%)
4 10.229 us 11.687 us( 14.25%) 10.240 us( 0.11%)
8 10.203 us 11.653 us( 14.21%) 10.402 us( 1.95%)
16 10.530 us 11.313 us( 7.44%) 10.599 us( 0.66%)
32 10.241 us 11.586 us( 13.13%) 10.223 us( -0.18%)
64 10.693 us 11.652 us( 8.97%) 10.251 us( -4.13%)
128 10.597 us 11.579 us( 9.27%) 10.494 us( -0.97%)
256 10.409 us 11.957 us( 14.87%) 10.710 us( 2.89%)
512 11.088 us 12.505 us( 12.78%) 10.547 us( -4.88%)
1024 11.240 us 12.255 us( 9.03%) 10.787 us( -4.03%)
2048 11.485 us 16.970 us( 47.76%) 11.256 us( -1.99%)
4096 12.077 us 13.948 us( 15.49%) 12.230 us( 1.27%)
8192 13.683 us 16.693 us( 22.00%) 13.786 us( 0.75%)
16384 16.470 us 23.615 us( 43.38%) 16.459 us( -0.07%)
32768 22.540 us 40.966 us( 81.75%) 23.284 us( 3.30%)
65536 34.192 us 73.003 us(113.51%) 34.233 us( 0.12%)
With SMC autocorking support, we can archive better throughput
than TCP in most message sizes without any latency trade-off.
Signed-off-by: Dust Li <dust.li@linux.alibaba.com>
---
v2: 1. Remove empty line in smc_connection
2. use Reverse Christmas tree style for local variable.
3. remove redundant container_of
v3: 1. use hex instead of decimal
2. Remove unintented removal of new line
3. Rename autocork to autocorking to be compliant with TCP
4. re-test the data, use SMC NoCork as baseline
---
net/smc/smc.h | 2 +
net/smc/smc_cdc.c | 11 +++--
net/smc/smc_tx.c | 107 ++++++++++++++++++++++++++++++++++++++++------
3 files changed, 105 insertions(+), 15 deletions(-)
@@ -48,9 +48,14 @@ static void smc_cdc_tx_handler(struct smc_wr_tx_pend_priv *pnd_snd,conn->tx_cdc_seq_fin=cdcpend->ctrl_seq;}-if(atomic_dec_and_test(&conn->cdc_pend_tx_wr)&&-unlikely(wq_has_sleeper(&conn->cdc_pend_tx_wq)))-wake_up(&conn->cdc_pend_tx_wq);+if(atomic_dec_and_test(&conn->cdc_pend_tx_wr)){+/* If this is the last pending WR complete, we must push to+*preventhangwhenautocorkenabled.+*/+smc_tx_sndbuf_nonempty(conn);+if(unlikely(wq_has_sleeper(&conn->cdc_pend_tx_wq)))+wake_up(&conn->cdc_pend_tx_wq);+}WARN_ON(atomic_read(&conn->cdc_pend_tx_wr)<0);smc_tx_sndbuf_nonfull(smc);
@@ -131,6 +131,51 @@ static bool smc_tx_is_corked(struct smc_sock *smc)return(tp->nonagle&TCP_NAGLE_CORK)?true:false;}+/* If we have pending CDC messages, do not send:+*BecauseCQEofthisCDCmessagewillhappenshortly,itgives+*achancetocoalescefuturesendmsg()payloadintooneRDMAWrite,+*withoutneedforatimer,andwithnolatencytradeoff.+*Algorithmhere:+*1.Firstmessageshouldnevercork+*2.IfwehavependingTxCDCmessages,waitforthefirstCDC+*message'scompletion+*3.Don'tcorktomuchdatainasingleRDMAWritetopreventburst+*traffic,totalcorkedmessageshouldnotexceedsendbuf/2+*/+staticboolsmc_should_autocork(structsmc_sock*smc)+{+structsmc_connection*conn=&smc->conn;+intcorking_size;++corking_size=min(SMC_AUTOCORKING_DEFAULT_SIZE,+conn->sndbuf_desc->len>>1);++if(atomic_read(&conn->cdc_pend_tx_wr)==0||+smc_tx_prepared_sends(conn)>corking_size)+returnfalse;+returntrue;+}++staticboolsmc_tx_should_cork(structsmc_sock*smc,structmsghdr*msg)+{+structsmc_connection*conn=&smc->conn;++if(smc_should_autocork(smc))+returntrue;++/* for a corked socket defer the RDMA writes if+*sndbuf_spaceisstillavailable.Theapplications+*shouldknownhow/whentouncorkit.+*/+if((msg->msg_flags&MSG_MORE||+smc_tx_is_corked(smc)||+msg->msg_flags&MSG_SENDPAGE_NOTLAST)&&+atomic_read(&conn->sndbuf_space))+returntrue;++returnfalse;+}+/* sndbuf producer: main API called by socket layer.*calledundersocklock.*/
@@ -235,13 +280,10 @@ int smc_tx_sendmsg(struct smc_sock *smc, struct msghdr *msg, size_t len)*/if((msg->msg_flags&MSG_OOB)&&!send_remaining)conn->urg_tx_pend=true;-/* for a corked socket defer the RDMA writes if-*sndbuf_spaceisstillavailable.Theapplications-*shouldknownhow/whentouncorkit.+/* If we need to cork, do nothing and wait for the next+*sendmsg()callorpushontxcompletion*/-if(!((msg->msg_flags&MSG_MORE||smc_tx_is_corked(smc)||-msg->msg_flags&MSG_SENDPAGE_NOTLAST)&&-atomic_read(&conn->sndbuf_space)))+if(!smc_tx_should_cork(smc,msg))smc_tx_sndbuf_nonempty(conn);trace_smc_tx_sendmsg(smc,copylen);
@@ -589,13 +631,26 @@ static int smcd_tx_sndbuf_nonempty(struct smc_connection *conn)returnrc;}-intsmc_tx_sndbuf_nonempty(structsmc_connection*conn)+staticint__smc_tx_sndbuf_nonempty(structsmc_connection*conn){-intrc;+structsmc_sock*smc=container_of(conn,structsmc_sock,conn);+intrc=0;++/* No data in the send queue */+if(unlikely(smc_tx_prepared_sends(conn)<=0))+gotoout;++/* Peer don't have RMBE space */+if(unlikely(atomic_read(&conn->peer_rmbe_space)<=0)){+SMC_STAT_RMB_TX_PEER_FULL(smc,!conn->lnk);+gotoout;+}if(conn->killed||-conn->local_rx_ctrl.conn_state_flags.peer_conn_abort)-return-EPIPE;/* connection being aborted */+conn->local_rx_ctrl.conn_state_flags.peer_conn_abort){+rc=-EPIPE;/* connection being aborted */+gotoout;+}if(conn->lgr->is_smcd)rc=smcd_tx_sndbuf_nonempty(conn);else
@@ -603,10 +658,38 @@ int smc_tx_sndbuf_nonempty(struct smc_connection *conn)if(!rc){/* trigger socket release if connection is closing */-structsmc_sock*smc=container_of(conn,structsmc_sock,-conn);smc_close_wake_tx_prepared(smc);}++out:+returnrc;+}++intsmc_tx_sndbuf_nonempty(structsmc_connection*conn)+{+intrc;++/* This make sure only one can send simultaneously to prevent wasting+*ofCPUandCDCslot.+*Recordwhethersomeonehastriedtopushwhilewearepushing.+*/+if(atomic_inc_return(&conn->tx_pushing)>1)+return0;++again:+atomic_set(&conn->tx_pushing,1);+smp_wmb();/* Make sure tx_pushing is 1 before real send */+rc=__smc_tx_sndbuf_nonempty(conn);++/* We need to check whether someone else have added some data into+*thesendqueueandtriedtopushbutfailedaftertheatomic_set()+*whenwearepushing.+*Ifso,weneedtopushagaintopreventthosedatahanginthesend+*queue.+*/+if(unlikely(!atomic_dec_and_test(&conn->tx_pushing)))+gotoagain;+returnrc;}
From: Dust Li <dust.li@linux.alibaba.com> Date: 2022-03-01 09:44:17
This add a new sysctl: net.smc.autocorking_size
We can dynamically change the behaviour of autocorking
by change the value of autocorking_size.
Setting to 0 disables autocorking in SMC
Signed-off-by: Dust Li <dust.li@linux.alibaba.com>
---
Documentation/networking/smc-sysctl.rst | 23 +++++++++++++++++++++++
include/net/netns/smc.h | 1 +
net/smc/smc_sysctl.c | 10 ++++++++++
net/smc/smc_tx.c | 2 +-
4 files changed, 35 insertions(+), 1 deletion(-)
create mode 100644 Documentation/networking/smc-sysctl.rst
@@ -0,0 +1,23 @@+.. SPDX-License-Identifier: GPL-2.0++=========+SMC Sysctl+=========++/proc/sys/net/smc/* Variables+==============================++autocorking_size - INTEGER+ Setting SMC auto corking size:+ SMC auto corking is like TCP auto corking from the application's+ perspective of view. When applications do consecutive small+ write()/sendmsg() system calls, we try to coalesce these small writes+ as much as possible, to lower total amount of CDC and RDMA Write been+ sent.+ autocorking_size limits the maximum corked bytes that can be sent to+ the under device in 1 single sending. If set to 0, the SMC auto corking+ is disabled.+ Applications can still use TCP_CORK for optimal behavior when they+ know how/when to uncork their sockets.++ Default: 64K
@@ -40,6 +48,8 @@ static __net_init int smc_sysctl_init_net(struct net *net)if(!net->smc.smc_hdr)gotoerr_reg;+net->smc.sysctl_autocorking_size=SMC_AUTOCORKING_DEFAULT_SIZE;+return0;err_reg:
From: Dust Li <dust.li@linux.alibaba.com> Date: 2022-03-01 09:44:27
rmbe_update_limit is used to limit announcing receive
window updating too frequently. RFC7609 request a minimal
increase in the window size of 10% of the receive buffer
space. But current implementation used:
min_t(int, rmbe_size / 10, SOCK_MIN_SNDBUF / 2)
and SOCK_MIN_SNDBUF / 2 == 2304 Bytes, which is almost
always less then 10% of the receive buffer space.
This causes the receiver always sending CDC message to
update its consumer cursor when it consumes more then 2K
of data. And as a result, we may encounter something like
"TCP silly window syndrome" when sending 2.5~8K message.
This patch fixes this using max(rmbe_size / 10, SOCK_MIN_SNDBUF / 2).
With this patch and SMC autocorking enabled, qperf 2K/4K/8K
tcp_bw test shows 45%/75%/40% increase in throughput respectively.
Signed-off-by: Dust Li <dust.li@linux.alibaba.com>
---
net/smc/smc_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -1988,7 +1988,7 @@ static struct smc_buf_desc *smc_buf_get_slot(int compressed_bufsize,*/staticinlineintsmc_rmb_wnd_update_limit(intrmbe_size){-returnmin_t(int,rmbe_size/10,SOCK_MIN_SNDBUF/2);+returnmax_t(int,rmbe_size/10,SOCK_MIN_SNDBUF/2);}/* map an rmb buf to a link */
From: Dust Li <dust.li@linux.alibaba.com> Date: 2022-03-01 09:44:28
When we are handling softirq workload, enable hardirq may
again interrupt the current routine of softirq, and then
try to raise softirq again. This only wastes CPU cycles
and won't have any real gain.
Since IB_CQ_REPORT_MISSED_EVENTS already make sure if
ib_req_notify_cq() returns 0, it is safe to wait for the
next event, with no need to poll the CQ again in this case.
This patch disables hardirq during the processing of softirq,
and re-arm the CQ after softirq is done. Somehow like NAPI.
Co-developed-by: Guangguan Wang <redacted>
Signed-off-by: Guangguan Wang <redacted>
Signed-off-by: Dust Li <dust.li@linux.alibaba.com>
---
net/smc/smc_wr.c | 49 +++++++++++++++++++++++++++---------------------
1 file changed, 28 insertions(+), 21 deletions(-)
@@ -137,25 +137,28 @@ static void smc_wr_tx_tasklet_fn(struct tasklet_struct *t){structsmc_ib_device*dev=from_tasklet(dev,t,send_tasklet);structib_wcwc[SMC_WR_MAX_POLL_CQE];-inti=0,rc;-intpolled=0;+inti,rc;again:-polled++;do{memset(&wc,0,sizeof(wc));rc=ib_poll_cq(dev->roce_cq_send,SMC_WR_MAX_POLL_CQE,wc);-if(polled==1){-ib_req_notify_cq(dev->roce_cq_send,-IB_CQ_NEXT_COMP|-IB_CQ_REPORT_MISSED_EVENTS);-}-if(!rc)-break;for(i=0;i<rc;i++)smc_wr_tx_process_cqe(&wc[i]);+if(rc<SMC_WR_MAX_POLL_CQE)+/* If < SMC_WR_MAX_POLL_CQE, the CQ should have been+*drained,noneedtopollagain.--GuangguanWang+*/+break;}while(rc>0);-if(polled==1)++/* IB_CQ_REPORT_MISSED_EVENTS make sure if ib_req_notify_cq() returns+*0,itissafetowaitforthenextevent.+*ElsewemustpolltheCQagaintomakesurewewon'tmissanyevent+*/+if(ib_req_notify_cq(dev->roce_cq_send,+IB_CQ_NEXT_COMP|+IB_CQ_REPORT_MISSED_EVENTS))gotoagain;}
@@ -478,24 +481,28 @@ static void smc_wr_rx_tasklet_fn(struct tasklet_struct *t){structsmc_ib_device*dev=from_tasklet(dev,t,recv_tasklet);structib_wcwc[SMC_WR_MAX_POLL_CQE];-intpolled=0;intrc;again:-polled++;do{memset(&wc,0,sizeof(wc));rc=ib_poll_cq(dev->roce_cq_recv,SMC_WR_MAX_POLL_CQE,wc);-if(polled==1){-ib_req_notify_cq(dev->roce_cq_recv,-IB_CQ_SOLICITED_MASK-|IB_CQ_REPORT_MISSED_EVENTS);-}-if(!rc)+if(rc>0)+smc_wr_rx_process_cqes(&wc[0],rc);+if(rc<SMC_WR_MAX_POLL_CQE)+/* If < SMC_WR_MAX_POLL_CQE, the CQ should have been+*drained,noneedtopollagain.--GuangguanWang+*/break;-smc_wr_rx_process_cqes(&wc[0],rc);}while(rc>0);-if(polled==1)++/* IB_CQ_REPORT_MISSED_EVENTS make sure if ib_req_notify_cq() returns+*0,itissafetowaitforthenextevent.+*ElsewemustpolltheCQagaintomakesurewewon'tmissanyevent+*/+if(ib_req_notify_cq(dev->roce_cq_recv,+IB_CQ_SOLICITED_MASK|+IB_CQ_REPORT_MISSED_EVENTS))gotoagain;}
From: Dust Li <dust.li@linux.alibaba.com> Date: 2022-03-01 09:44:30
In commit ea785a1a573b("net/smc: Send directly when
TCP_CORK is cleared"), we don't use delayed work
to implement cork.
This patch use the same algorithm, removes the
delayed work when setting TCP_NODELAY and send
directly in setsockopt(). This also makes the
TCP_NODELAY the same as TCP.
Cc: Tony Lu <tonylu@linux.alibaba.com>
Signed-off-by: Dust Li <dust.li@linux.alibaba.com>
---
net/smc/af_smc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
@@ -2796,8 +2796,8 @@ static int smc_setsockopt(struct socket *sock, int level, int optname,sk->sk_state!=SMC_CLOSED){if(val){SMC_STAT_INC(smc,ndly_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;
From: Dust Li <dust.li@linux.alibaba.com> Date: 2022-03-01 09:44:33
Send data all the way down to the RDMA device is a time
consuming operation(get a new slot, maybe do RDMA Write
and send a CDC, etc). Moving those operations from BH
to user context is good for performance.
If the sock_lock is hold by user, we don't try to send
data out in the BH context, but just mark we should
send. Since the user will release the sock_lock soon, we
can do the sending there.
Add smc_release_cb() which will be called in release_sock()
and try send in the callback if needed.
This patch moves the sending part out from BH if sock lock
is hold by user. In my testing environment, this saves about
20% softirq in the qperf 4K tcp_bw test in the sender side
with no noticeable throughput drop.
Signed-off-by: Dust Li <dust.li@linux.alibaba.com>
---
net/smc/af_smc.c | 16 ++++++++++++++++
net/smc/smc.h | 4 ++++
net/smc/smc_cdc.c | 19 ++++++++++++++-----
3 files changed, 34 insertions(+), 5 deletions(-)
@@ -193,12 +193,27 @@ void smc_unhash_sk(struct sock *sk)}EXPORT_SYMBOL_GPL(smc_unhash_sk);+/* This will be called before user really release sock_lock. So do the+*workwhichwedidn'tdobecauseofuserholdthesock_lockinthe+*BHcontext+*/+staticvoidsmc_release_cb(structsock*sk)+{+structsmc_sock*smc=smc_sk(sk);++if(smc->conn.tx_in_release_sock){+smc_tx_pending(&smc->conn);+smc->conn.tx_in_release_sock=false;+}+}+structprotosmc_proto={.name="SMC",.owner=THIS_MODULE,.keepalive=smc_set_keepalive,.hash=smc_hash_sk,.unhash=smc_unhash_sk,+.release_cb=smc_release_cb,.obj_size=sizeof(structsmc_sock),.h.smc_hash=&smc_v4_hashinfo,.slab_flags=SLAB_TYPESAFE_BY_RCU,
@@ -211,6 +226,7 @@ struct proto smc_proto6 = {.keepalive=smc_set_keepalive,.hash=smc_hash_sk,.unhash=smc_unhash_sk,+.release_cb=smc_release_cb,.obj_size=sizeof(structsmc_sock),.h.smc_hash=&smc_v6_hashinfo,.slab_flags=SLAB_TYPESAFE_BY_RCU,
@@ -49,10 +49,15 @@ static void smc_cdc_tx_handler(struct smc_wr_tx_pend_priv *pnd_snd,}if(atomic_dec_and_test(&conn->cdc_pend_tx_wr)){-/* If this is the last pending WR complete, we must push to-*preventhangwhenautocorkenabled.+/* If user owns the sock_lock, mark the connection need sending.+*Usercontextwilllatertrytosendwhenitreleasesock_lock+*insmc_release_cb()*/-smc_tx_sndbuf_nonempty(conn);+if(sock_owned_by_user(&smc->sk))+conn->tx_in_release_sock=true;+else+smc_tx_pending(conn);+if(unlikely(wq_has_sleeper(&conn->cdc_pend_tx_wq)))wake_up(&conn->cdc_pend_tx_wq);}
@@ -355,8 +360,12 @@ static void smc_cdc_msg_recv_action(struct smc_sock *smc,/* trigger sndbuf consumer: RDMA write into peer RMBE and CDC */if((diff_cons&&smc_tx_prepared_sends(conn))||conn->local_rx_ctrl.prod_flags.cons_curs_upd_req||-conn->local_rx_ctrl.prod_flags.urg_data_pending)-smc_tx_sndbuf_nonempty(conn);+conn->local_rx_ctrl.prod_flags.urg_data_pending){+if(!sock_owned_by_user(&smc->sk))+smc_tx_pending(conn);+else+conn->tx_in_release_sock=true;+}if(diff_cons&&conn->urg_tx_pend&&atomic_read(&conn->peer_rmbe_space)==conn->peer_rmbe_size){
From: Leon Romanovsky <leon@kernel.org> Date: 2022-03-01 10:14:23
On Tue, Mar 01, 2022 at 05:44:01PM +0800, Dust Li wrote:
quoted hunk
When we are handling softirq workload, enable hardirq may
again interrupt the current routine of softirq, and then
try to raise softirq again. This only wastes CPU cycles
and won't have any real gain.
Since IB_CQ_REPORT_MISSED_EVENTS already make sure if
ib_req_notify_cq() returns 0, it is safe to wait for the
next event, with no need to poll the CQ again in this case.
This patch disables hardirq during the processing of softirq,
and re-arm the CQ after softirq is done. Somehow like NAPI.
Co-developed-by: Guangguan Wang <redacted>
Signed-off-by: Guangguan Wang <redacted>
Signed-off-by: Dust Li <dust.li@linux.alibaba.com>
---
net/smc/smc_wr.c | 49 +++++++++++++++++++++++++++---------------------
1 file changed, 28 insertions(+), 21 deletions(-)
@@ -137,25 +137,28 @@ static void smc_wr_tx_tasklet_fn(struct tasklet_struct *t){structsmc_ib_device*dev=from_tasklet(dev,t,send_tasklet);structib_wcwc[SMC_WR_MAX_POLL_CQE];-inti=0,rc;-intpolled=0;+inti,rc;again:-polled++;do{memset(&wc,0,sizeof(wc));rc=ib_poll_cq(dev->roce_cq_send,SMC_WR_MAX_POLL_CQE,wc);-if(polled==1){-ib_req_notify_cq(dev->roce_cq_send,-IB_CQ_NEXT_COMP|-IB_CQ_REPORT_MISSED_EVENTS);-}-if(!rc)-break;for(i=0;i<rc;i++)smc_wr_tx_process_cqe(&wc[i]);+if(rc<SMC_WR_MAX_POLL_CQE)+/* If < SMC_WR_MAX_POLL_CQE, the CQ should have been+*drained,noneedtopollagain.--GuangguanWang
1. Please remove "--Guangguan Wang".
2. We already discussed that. SMC should be changed to use RDMA CQ pool API
drivers/infiniband/core/cq.c.
ib_poll_handler() has much better implementation (tracing, IRQ rescheduling,
proper error handling) than this SMC variant.
Thanks
quoted hunk
+ */
+ break;
} while (rc > 0);
- if (polled == 1)
+
+ /* IB_CQ_REPORT_MISSED_EVENTS make sure if ib_req_notify_cq() returns
+ * 0, it is safe to wait for the next event.
+ * Else we must poll the CQ again to make sure we won't miss any event
+ */
+ if (ib_req_notify_cq(dev->roce_cq_send,
+ IB_CQ_NEXT_COMP |
+ IB_CQ_REPORT_MISSED_EVENTS))
goto again;
}
@@ -478,24 +481,28 @@ static void smc_wr_rx_tasklet_fn(struct tasklet_struct *t) { struct smc_ib_device *dev = from_tasklet(dev, t, recv_tasklet); struct ib_wc wc[SMC_WR_MAX_POLL_CQE];- int polled = 0; int rc; again:- polled++; do { memset(&wc, 0, sizeof(wc)); rc = ib_poll_cq(dev->roce_cq_recv, SMC_WR_MAX_POLL_CQE, wc);- if (polled == 1) {- ib_req_notify_cq(dev->roce_cq_recv,- IB_CQ_SOLICITED_MASK- | IB_CQ_REPORT_MISSED_EVENTS);- }- if (!rc)+ if (rc > 0)+ smc_wr_rx_process_cqes(&wc[0], rc);+ if (rc < SMC_WR_MAX_POLL_CQE)+ /* If < SMC_WR_MAX_POLL_CQE, the CQ should have been+ * drained, no need to poll again. --Guangguan Wang+ */ break;- smc_wr_rx_process_cqes(&wc[0], rc); } while (rc > 0);- if (polled == 1)++ /* IB_CQ_REPORT_MISSED_EVENTS make sure if ib_req_notify_cq() returns+ * 0, it is safe to wait for the next event.+ * Else we must poll the CQ again to make sure we won't miss any event+ */+ if (ib_req_notify_cq(dev->roce_cq_recv,+ IB_CQ_SOLICITED_MASK |+ IB_CQ_REPORT_MISSED_EVENTS)) goto again; }
On Tue, Mar 01, 2022 at 12:14:15PM +0200, Leon Romanovsky wrote:
On Tue, Mar 01, 2022 at 05:44:01PM +0800, Dust Li wrote:
quoted
When we are handling softirq workload, enable hardirq may
again interrupt the current routine of softirq, and then
try to raise softirq again. This only wastes CPU cycles
and won't have any real gain.
Since IB_CQ_REPORT_MISSED_EVENTS already make sure if
ib_req_notify_cq() returns 0, it is safe to wait for the
next event, with no need to poll the CQ again in this case.
This patch disables hardirq during the processing of softirq,
and re-arm the CQ after softirq is done. Somehow like NAPI.
Co-developed-by: Guangguan Wang <redacted>
Signed-off-by: Guangguan Wang <redacted>
Signed-off-by: Dust Li <dust.li@linux.alibaba.com>
---
net/smc/smc_wr.c | 49 +++++++++++++++++++++++++++---------------------
1 file changed, 28 insertions(+), 21 deletions(-)
@@ -137,25 +137,28 @@ static void smc_wr_tx_tasklet_fn(struct tasklet_struct *t){structsmc_ib_device*dev=from_tasklet(dev,t,send_tasklet);structib_wcwc[SMC_WR_MAX_POLL_CQE];-inti=0,rc;-intpolled=0;+inti,rc;again:-polled++;do{memset(&wc,0,sizeof(wc));rc=ib_poll_cq(dev->roce_cq_send,SMC_WR_MAX_POLL_CQE,wc);-if(polled==1){-ib_req_notify_cq(dev->roce_cq_send,-IB_CQ_NEXT_COMP|-IB_CQ_REPORT_MISSED_EVENTS);-}-if(!rc)-break;for(i=0;i<rc;i++)smc_wr_tx_process_cqe(&wc[i]);+if(rc<SMC_WR_MAX_POLL_CQE)+/* If < SMC_WR_MAX_POLL_CQE, the CQ should have been+*drained,noneedtopollagain.--GuangguanWang
1. Please remove "--Guangguan Wang".
2. We already discussed that. SMC should be changed to use RDMA CQ pool API
drivers/infiniband/core/cq.c.
ib_poll_handler() has much better implementation (tracing, IRQ rescheduling,
proper error handling) than this SMC variant.
OK, I'll remove this patch in the next version.
Thanks
I think this broke the build:
In file included from ../include/linux/kernel.h:26,
from ../include/linux/random.h:11,
from ../include/linux/net.h:18,
from ../net/smc/smc_tx.c:16:
../net/smc/smc_tx.c: In function ‘smc_should_autocork’:
../include/linux/minmax.h:20:35: error: comparison of distinct pointer types lacks a cast [-Werror]
20 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
| ^~
../include/linux/minmax.h:26:18: note: in expansion of macro ‘__typecheck’
26 | (__typecheck(x, y) && __no_side_effects(x, y))
| ^~~~~~~~~~~
../include/linux/minmax.h:36:31: note: in expansion of macro ‘__safe_cmp’
36 | __builtin_choose_expr(__safe_cmp(x, y), \
| ^~~~~~~~~~
../include/linux/minmax.h:45:25: note: in expansion of macro ‘__careful_cmp’
45 | #define min(x, y) __careful_cmp(x, y, <)
| ^~~~~~~~~~~~~
../net/smc/smc_tx.c:150:24: note: in expansion of macro ‘min’
150 | corking_size = min(sock_net(&smc->sk)->smc.sysctl_autocorking_size,
| ^~~
On Tue, Mar 01, 2022 at 12:14:15PM +0200, Leon Romanovsky wrote:
quoted
On Tue, Mar 01, 2022 at 05:44:01PM +0800, Dust Li wrote:
1. Please remove "--Guangguan Wang".
2. We already discussed that. SMC should be changed to use RDMA CQ pool API
drivers/infiniband/core/cq.c.
ib_poll_handler() has much better implementation (tracing, IRQ rescheduling,
proper error handling) than this SMC variant.
OK, I'll remove this patch in the next version.
Looks like this one was accepted already, but per discussion (and I agree with that) -
please revert this patch. Thank you.
On Fri, Mar 04, 2022 at 09:19:27AM +0100, Karsten Graul wrote:
On 01/03/2022 11:53, dust.li wrote:
quoted
On Tue, Mar 01, 2022 at 12:14:15PM +0200, Leon Romanovsky wrote:
quoted
On Tue, Mar 01, 2022 at 05:44:01PM +0800, Dust Li wrote:
1. Please remove "--Guangguan Wang".
2. We already discussed that. SMC should be changed to use RDMA CQ pool API
drivers/infiniband/core/cq.c.
ib_poll_handler() has much better implementation (tracing, IRQ rescheduling,
proper error handling) than this SMC variant.
OK, I'll remove this patch in the next version.
Looks like this one was accepted already, but per discussion (and I agree with that) -
please revert this patch. Thank you.
Yes. No problem, I will send a revert patch.
Thanks