| Message ID | 1669218890-115854-1-git-send-email-alibuda@linux.alibaba.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | optimize the parallelism of SMC-R connections | expand |
On 11/23/22 11:54 PM, D.Wythe wrote: > From: "D.Wythe" <alibuda@linux.alibaba.com> > > This patch set attempts to optimize the parallelism of SMC-R connections, > mainly to reduce unnecessary blocking on locks, and to fix exceptions that > occur after thoses optimization. > > D. Wythe (10): > net/smc: Fix potential panic dues to unprotected > smc_llc_srv_add_link() > net/smc: fix application data exception > net/smc: fix SMC_CLC_DECL_ERR_REGRMB without smc_server_lgr_pending > net/smc: remove locks smc_client_lgr_pending and > smc_server_lgr_pending > net/smc: allow confirm/delete rkey response deliver multiplex > net/smc: make SMC_LLC_FLOW_RKEY run concurrently > net/smc: llc_conf_mutex refactor, replace it with rw_semaphore > net/smc: use read semaphores to reduce unnecessary blocking in > smc_buf_create() & smcr_buf_unuse() > net/smc: reduce unnecessary blocking in smcr_lgr_reg_rmbs() > net/smc: replace mutex rmbs_lock and sndbufs_lock with rw_semaphore > > net/smc/af_smc.c | 74 ++++---- > net/smc/smc_core.c | 541 +++++++++++++++++++++++++++++++++++++++++++++++------ > net/smc/smc_core.h | 53 +++++- > net/smc/smc_llc.c | 285 ++++++++++++++++++++-------- > net/smc/smc_llc.h | 6 + > net/smc/smc_wr.c | 10 - > net/smc/smc_wr.h | 10 + > 7 files changed, 801 insertions(+), 178 deletions(-) > Hi Jan and Wenjia, I'm wondering whether the bug fix patches need to be put together in this series. I'm considering sending these bug fix patches separately now, which may be better, in case that our patch might have other problems. These bug fix patches are mainly independent, even without my other patches, they may be triggered theoretically. Of course, these bug fix patches may need to ahead before the other PATCH, otherwise the probability of the problems they fixed may be amplified in an intermediate version. What do you think? Best Wishes. D. Wythe
On 24/11/2022 06:55, D. Wythe wrote: > > > On 11/23/22 11:54 PM, D.Wythe wrote: >> From: "D.Wythe" <alibuda@linux.alibaba.com> >> >> This patch set attempts to optimize the parallelism of SMC-R connections, >> mainly to reduce unnecessary blocking on locks, and to fix exceptions >> that >> occur after thoses optimization. >> > >> D. Wythe (10): >> net/smc: Fix potential panic dues to unprotected >> smc_llc_srv_add_link() >> net/smc: fix application data exception >> net/smc: fix SMC_CLC_DECL_ERR_REGRMB without smc_server_lgr_pending >> net/smc: remove locks smc_client_lgr_pending and >> smc_server_lgr_pending >> net/smc: allow confirm/delete rkey response deliver multiplex >> net/smc: make SMC_LLC_FLOW_RKEY run concurrently >> net/smc: llc_conf_mutex refactor, replace it with rw_semaphore >> net/smc: use read semaphores to reduce unnecessary blocking in >> smc_buf_create() & smcr_buf_unuse() >> net/smc: reduce unnecessary blocking in smcr_lgr_reg_rmbs() >> net/smc: replace mutex rmbs_lock and sndbufs_lock with rw_semaphore >> >> net/smc/af_smc.c | 74 ++++---- >> net/smc/smc_core.c | 541 >> +++++++++++++++++++++++++++++++++++++++++++++++------ >> net/smc/smc_core.h | 53 +++++- >> net/smc/smc_llc.c | 285 ++++++++++++++++++++-------- >> net/smc/smc_llc.h | 6 + >> net/smc/smc_wr.c | 10 - >> net/smc/smc_wr.h | 10 + >> 7 files changed, 801 insertions(+), 178 deletions(-) >> > > Hi Jan and Wenjia, > > I'm wondering whether the bug fix patches need to be put together in > this series. I'm considering > sending these bug fix patches separately now, which may be better, in > case that our patch > might have other problems. These bug fix patches are mainly independent, > even without my other > patches, they may be triggered theoretically. Hi D. Wenjia and i just talked about that. For us it would be better separating the fixes and the new logic. If the fixes are independent feel free to post them to net. > > Of course, these bug fix patches may need to ahead before the other PATCH, > otherwise the probability of the problems they fixed may be amplified in > an intermediate version. True. Thanks for pointing that out. Thank you - Jan > > What do you think? > > Best Wishes. > D. Wythe
On 23/11/2022 16:54, D.Wythe wrote: > From: "D.Wythe" <alibuda@linux.alibaba.com> > > This patch set attempts to optimize the parallelism of SMC-R connections, > mainly to reduce unnecessary blocking on locks, and to fix exceptions that > occur after thoses optimization. > > According to Off-CPU graph, SMC worker's off-CPU as that: > > smc_close_passive_work (1.09%) > smcr_buf_unuse (1.08%) > smc_llc_flow_initiate (1.02%) > > smc_listen_work (48.17%) > __mutex_lock.isra.11 (47.96%) > > > An ideal SMC-R connection process should only block on the IO events > of the network, but it's quite clear that the SMC-R connection now is > queued on the lock most of the time. > > The goal of this patchset is to achieve our ideal situation where > network IO events are blocked for the majority of the connection lifetime. > > There are three big locks here: > > 1. smc_client_lgr_pending & smc_server_lgr_pending > > 2. llc_conf_mutex > > 3. rmbs_lock & sndbufs_lock > > And an implementation issue: > > 1. confirm/delete rkey msg can't be sent concurrently while > protocol allows indeed. > > Unfortunately,The above problems together affect the parallelism of > SMC-R connection. If any of them are not solved. our goal cannot > be achieved. > > After this patch set, we can get a quite ideal off-CPU graph as > following: > > smc_close_passive_work (41.58%) > smcr_buf_unuse (41.57%) > smc_llc_do_delete_rkey (41.57%) > > smc_listen_work (39.10%) > smc_clc_wait_msg (13.18%) > tcp_recvmsg_locked (13.18) > smc_listen_find_device (25.87%) > smcr_lgr_reg_rmbs (25.87%) > smc_llc_do_confirm_rkey (25.87%) > > We can see that most of the waiting times are waiting for network IO > events. This also has a certain performance improvement on our > short-lived conenction wrk/nginx benchmark test: > > +--------------+------+------+-------+--------+------+--------+ > |conns/qps |c4 | c8 | c16 | c32 | c64 | c200 | > +--------------+------+------+-------+--------+------+--------+ > |SMC-R before |9.7k | 10k | 10k | 9.9k | 9.1k | 8.9k | > +--------------+------+------+-------+--------+------+--------+ > |SMC-R now |13k | 19k | 18k | 16k | 15k | 12k | > +--------------+------+------+-------+--------+------+--------+ > |TCP |15k | 35k | 51k | 80k | 100k | 162k | > +--------------+------+------+-------+--------+------+--------+ > > The reason why the benefit is not obvious after the number of connections > has increased dues to workqueue. If we try to change workqueue to UNBOUND, > we can obtain at least 4-5 times performance improvement, reach up to half > of TCP. However, this is not an elegant solution, the optimization of it > will be much more complicated. But in any case, we will submit relevant > optimization patches as soon as possible. > > Please note that the premise here is that the lock related problem > must be solved first, otherwise, no matter how we optimize the workqueue, > there won't be much improvement. > > Because there are a lot of related changes to the code, if you have > any questions or suggestions, please let me know. > > Thanks > D. Wythe Thank you for your submission. I'm going to test the new patch. Please give us some time to do so. Thank you - Jan > > v1 -> v2: > > 1. Fix panic in SMC-D scenario > 2. Fix lnkc related hashfn calculation exception, caused by operator > priority > 3. Only wake up one connection if the lnk is not active > 4. Delete obsolete unlock logic in smc_listen_work() > 5. PATCH format, do Reverse Christmas tree > 6. PATCH format, change all xxx_lnk_xxx function to xxx_link_xxx > 7. PATCH format, add correct fix tag for the patches for fixes. > 8. PATCH format, fix some spelling error > 9. PATCH format, rename slow to do_slow > > v2 -> v3: > > 1. add SMC-D support, remove the concept of link cluster since SMC-D has > no link at all. Replace it by lgr decision maker, who provides suggestions > to SMC-D and SMC-R on whether to create new link group. > > 2. Fix the corruption problem described by PATCH 'fix application > data exception' on SMC-D. > > v3 -> v4: > > 1. Fix panic caused by uninitialization map. > > v4 -> v5: > > 1. Make SMC-D buf creation be serial to avoid Potential error > 2. Add a flag to synchronize the success of the first contact > with the ready of the link group, including SMC-D and SMC-R. > 3. Fixed possible reference count leak in smc_llc_flow_start(). > 4. reorder the patch, make bugfix PATCH be ahead. > > D. Wythe (10): > net/smc: Fix potential panic dues to unprotected > smc_llc_srv_add_link() > net/smc: fix application data exception > net/smc: fix SMC_CLC_DECL_ERR_REGRMB without smc_server_lgr_pending > net/smc: remove locks smc_client_lgr_pending and > smc_server_lgr_pending > net/smc: allow confirm/delete rkey response deliver multiplex > net/smc: make SMC_LLC_FLOW_RKEY run concurrently > net/smc: llc_conf_mutex refactor, replace it with rw_semaphore > net/smc: use read semaphores to reduce unnecessary blocking in > smc_buf_create() & smcr_buf_unuse() > net/smc: reduce unnecessary blocking in smcr_lgr_reg_rmbs() > net/smc: replace mutex rmbs_lock and sndbufs_lock with rw_semaphore > > net/smc/af_smc.c | 74 ++++---- > net/smc/smc_core.c | 541 +++++++++++++++++++++++++++++++++++++++++++++++------ > net/smc/smc_core.h | 53 +++++- > net/smc/smc_llc.c | 285 ++++++++++++++++++++-------- > net/smc/smc_llc.h | 6 + > net/smc/smc_wr.c | 10 - > net/smc/smc_wr.h | 10 + > 7 files changed, 801 insertions(+), 178 deletions(-) >
On 11/24/22 4:33 PM, Jan Karcher wrote: > > > On 24/11/2022 06:55, D. Wythe wrote: >> >> >> On 11/23/22 11:54 PM, D.Wythe wrote: >>> From: "D.Wythe" <alibuda@linux.alibaba.com> >>> >>> This patch set attempts to optimize the parallelism of SMC-R connections, >>> mainly to reduce unnecessary blocking on locks, and to fix exceptions that >>> occur after thoses optimization. >>> >> >>> D. Wythe (10): >>> net/smc: Fix potential panic dues to unprotected >>> smc_llc_srv_add_link() >>> net/smc: fix application data exception >>> net/smc: fix SMC_CLC_DECL_ERR_REGRMB without smc_server_lgr_pending >>> net/smc: remove locks smc_client_lgr_pending and >>> smc_server_lgr_pending >>> net/smc: allow confirm/delete rkey response deliver multiplex >>> net/smc: make SMC_LLC_FLOW_RKEY run concurrently >>> net/smc: llc_conf_mutex refactor, replace it with rw_semaphore >>> net/smc: use read semaphores to reduce unnecessary blocking in >>> smc_buf_create() & smcr_buf_unuse() >>> net/smc: reduce unnecessary blocking in smcr_lgr_reg_rmbs() >>> net/smc: replace mutex rmbs_lock and sndbufs_lock with rw_semaphore >>> >>> net/smc/af_smc.c | 74 ++++---- >>> net/smc/smc_core.c | 541 +++++++++++++++++++++++++++++++++++++++++++++++------ >>> net/smc/smc_core.h | 53 +++++- >>> net/smc/smc_llc.c | 285 ++++++++++++++++++++-------- >>> net/smc/smc_llc.h | 6 + >>> net/smc/smc_wr.c | 10 - >>> net/smc/smc_wr.h | 10 + >>> 7 files changed, 801 insertions(+), 178 deletions(-) >>> >> >> Hi Jan and Wenjia, >> >> I'm wondering whether the bug fix patches need to be put together in this series. I'm considering >> sending these bug fix patches separately now, which may be better, in case that our patch >> might have other problems. These bug fix patches are mainly independent, even without my other >> patches, they may be triggered theoretically. > > Hi D. > > Wenjia and i just talked about that. For us it would be better separating the fixes and the new logic. > If the fixes are independent feel free to post them to net. Got it, I will remove those bug fix patches in the next series and send them separately. And thanks a lot for your test, no matter what the final test results are, I will send a new series to separate them after your test finished. Thanks D. Wythe
On 24/11/2022 09:53, D. Wythe wrote: > > > On 11/24/22 4:33 PM, Jan Karcher wrote: >> >> >> On 24/11/2022 06:55, D. Wythe wrote: >>> >>> >>> On 11/23/22 11:54 PM, D.Wythe wrote: >>>> From: "D.Wythe" <alibuda@linux.alibaba.com> >>>> >>>> This patch set attempts to optimize the parallelism of SMC-R >>>> connections, >>>> mainly to reduce unnecessary blocking on locks, and to fix >>>> exceptions that >>>> occur after thoses optimization. >>>> >>> >>>> D. Wythe (10): >>>> net/smc: Fix potential panic dues to unprotected >>>> smc_llc_srv_add_link() >>>> net/smc: fix application data exception >>>> net/smc: fix SMC_CLC_DECL_ERR_REGRMB without smc_server_lgr_pending >>>> net/smc: remove locks smc_client_lgr_pending and >>>> smc_server_lgr_pending >>>> net/smc: allow confirm/delete rkey response deliver multiplex >>>> net/smc: make SMC_LLC_FLOW_RKEY run concurrently >>>> net/smc: llc_conf_mutex refactor, replace it with rw_semaphore >>>> net/smc: use read semaphores to reduce unnecessary blocking in >>>> smc_buf_create() & smcr_buf_unuse() >>>> net/smc: reduce unnecessary blocking in smcr_lgr_reg_rmbs() >>>> net/smc: replace mutex rmbs_lock and sndbufs_lock with rw_semaphore >>>> >>>> net/smc/af_smc.c | 74 ++++---- >>>> net/smc/smc_core.c | 541 >>>> +++++++++++++++++++++++++++++++++++++++++++++++------ >>>> net/smc/smc_core.h | 53 +++++- >>>> net/smc/smc_llc.c | 285 ++++++++++++++++++++-------- >>>> net/smc/smc_llc.h | 6 + >>>> net/smc/smc_wr.c | 10 - >>>> net/smc/smc_wr.h | 10 + >>>> 7 files changed, 801 insertions(+), 178 deletions(-) >>>> >>> >>> Hi Jan and Wenjia, >>> >>> I'm wondering whether the bug fix patches need to be put together in >>> this series. I'm considering >>> sending these bug fix patches separately now, which may be better, in >>> case that our patch >>> might have other problems. These bug fix patches are mainly >>> independent, even without my other >>> patches, they may be triggered theoretically. >> >> Hi D. >> >> Wenjia and i just talked about that. For us it would be better >> separating the fixes and the new logic. >> If the fixes are independent feel free to post them to net. > > > Got it, I will remove those bug fix patches in the next series and send > them separately. > And thanks a lot for your test, no matter what the final test results > are, I will send a new series > to separate them after your test finished. Hi D., I have some troubles applying your patches. error: sha1 information is lacking or useless (net/smc/smc_core.c). error: could not build fake ancestor Patch failed at 0001 optimize the parallelism of SMC-R connections Before merging them by hand could you please send the v6 with the fixes separated and verify that you are basing on the latest net / net-next tree? That would make it easier for us to test them. Thank you - Jan > > Thanks > D. Wythe > > >
On 11/24/22 9:30 PM, Jan Karcher wrote: > > > On 24/11/2022 09:53, D. Wythe wrote: >> >> >> On 11/24/22 4:33 PM, Jan Karcher wrote: >>> >>> >>> On 24/11/2022 06:55, D. Wythe wrote: >>>> >>>> >>>> On 11/23/22 11:54 PM, D.Wythe wrote: >>>>> From: "D.Wythe" <alibuda@linux.alibaba.com> >>>>> >>>>> This patch set attempts to optimize the parallelism of SMC-R connections, >>>>> mainly to reduce unnecessary blocking on locks, and to fix exceptions that >>>>> occur after thoses optimization. >>>>> >>>> >>>>> D. Wythe (10): >>>>> net/smc: Fix potential panic dues to unprotected >>>>> smc_llc_srv_add_link() >>>>> net/smc: fix application data exception >>>>> net/smc: fix SMC_CLC_DECL_ERR_REGRMB without smc_server_lgr_pending >>>>> net/smc: remove locks smc_client_lgr_pending and >>>>> smc_server_lgr_pending >>>>> net/smc: allow confirm/delete rkey response deliver multiplex >>>>> net/smc: make SMC_LLC_FLOW_RKEY run concurrently >>>>> net/smc: llc_conf_mutex refactor, replace it with rw_semaphore >>>>> net/smc: use read semaphores to reduce unnecessary blocking in >>>>> smc_buf_create() & smcr_buf_unuse() >>>>> net/smc: reduce unnecessary blocking in smcr_lgr_reg_rmbs() >>>>> net/smc: replace mutex rmbs_lock and sndbufs_lock with rw_semaphore >>>>> >>>>> net/smc/af_smc.c | 74 ++++---- >>>>> net/smc/smc_core.c | 541 +++++++++++++++++++++++++++++++++++++++++++++++------ >>>>> net/smc/smc_core.h | 53 +++++- >>>>> net/smc/smc_llc.c | 285 ++++++++++++++++++++-------- >>>>> net/smc/smc_llc.h | 6 + >>>>> net/smc/smc_wr.c | 10 - >>>>> net/smc/smc_wr.h | 10 + >>>>> 7 files changed, 801 insertions(+), 178 deletions(-) >>>>> >>>> >>>> Hi Jan and Wenjia, >>>> >>>> I'm wondering whether the bug fix patches need to be put together in this series. I'm considering >>>> sending these bug fix patches separately now, which may be better, in case that our patch >>>> might have other problems. These bug fix patches are mainly independent, even without my other >>>> patches, they may be triggered theoretically. >>> >>> Hi D. >>> >>> Wenjia and i just talked about that. For us it would be better separating the fixes and the new logic. >>> If the fixes are independent feel free to post them to net. >> >> >> Got it, I will remove those bug fix patches in the next series and send them separately. >> And thanks a lot for your test, no matter what the final test results are, I will send a new series >> to separate them after your test finished. > > Hi D., > > I have some troubles applying your patches. > > error: sha1 information is lacking or useless (net/smc/smc_core.c). > error: could not build fake ancestor > Patch failed at 0001 optimize the parallelism of SMC-R connections > > Before merging them by hand could you please send the v6 with the fixes separated and verify that you are basing on the latest net / net-next tree? > > That would make it easier for us to test them. > > Thank you > - Jan > Hi Jan, Sorry to have this troubles. I'll send v6 right away. Thanks D. Wythe
On 11/24/22 9:30 PM, Jan Karcher wrote: > > > On 24/11/2022 09:53, D. Wythe wrote: >> >> >> On 11/24/22 4:33 PM, Jan Karcher wrote: >>> >>> >>> On 24/11/2022 06:55, D. Wythe wrote: >>>> >>>> >>>> On 11/23/22 11:54 PM, D.Wythe wrote: >>>>> From: "D.Wythe" <alibuda@linux.alibaba.com> >>>>> >>>>> This patch set attempts to optimize the parallelism of SMC-R connections, >>>>> mainly to reduce unnecessary blocking on locks, and to fix exceptions that >>>>> occur after thoses optimization. >>>>> >>>> >>>>> D. Wythe (10): >>>>> net/smc: Fix potential panic dues to unprotected >>>>> smc_llc_srv_add_link() >>>>> net/smc: fix application data exception >>>>> net/smc: fix SMC_CLC_DECL_ERR_REGRMB without smc_server_lgr_pending >>>>> net/smc: remove locks smc_client_lgr_pending and >>>>> smc_server_lgr_pending >>>>> net/smc: allow confirm/delete rkey response deliver multiplex >>>>> net/smc: make SMC_LLC_FLOW_RKEY run concurrently >>>>> net/smc: llc_conf_mutex refactor, replace it with rw_semaphore >>>>> net/smc: use read semaphores to reduce unnecessary blocking in >>>>> smc_buf_create() & smcr_buf_unuse() >>>>> net/smc: reduce unnecessary blocking in smcr_lgr_reg_rmbs() >>>>> net/smc: replace mutex rmbs_lock and sndbufs_lock with rw_semaphore >>>>> >>>>> net/smc/af_smc.c | 74 ++++---- >>>>> net/smc/smc_core.c | 541 +++++++++++++++++++++++++++++++++++++++++++++++------ >>>>> net/smc/smc_core.h | 53 +++++- >>>>> net/smc/smc_llc.c | 285 ++++++++++++++++++++-------- >>>>> net/smc/smc_llc.h | 6 + >>>>> net/smc/smc_wr.c | 10 - >>>>> net/smc/smc_wr.h | 10 + >>>>> 7 files changed, 801 insertions(+), 178 deletions(-) >>>>> >>>> >>>> Hi Jan and Wenjia, >>>> >>>> I'm wondering whether the bug fix patches need to be put together in this series. I'm considering >>>> sending these bug fix patches separately now, which may be better, in case that our patch >>>> might have other problems. These bug fix patches are mainly independent, even without my other >>>> patches, they may be triggered theoretically. >>> >>> Hi D. >>> >>> Wenjia and i just talked about that. For us it would be better separating the fixes and the new logic. >>> If the fixes are independent feel free to post them to net. >> >> >> Got it, I will remove those bug fix patches in the next series and send them separately. >> And thanks a lot for your test, no matter what the final test results are, I will send a new series >> to separate them after your test finished. > > Hi D., > > I have some troubles applying your patches. > > error: sha1 information is lacking or useless (net/smc/smc_core.c). > error: could not build fake ancestor > Patch failed at 0001 optimize the parallelism of SMC-R connections > > Before merging them by hand could you please send the v6 with the fixes separated and verify that you are basing on the latest net / net-next tree? > > That would make it easier for us to test them. > > Thank you > - Jan > Hi Jan, It's quite weird, it seems that my patch did based on the latest net-next tree. And I try apply it the latest net tree, it's seems work to me too. Maybe there is something wrong with the mirror I use. Can you show me the conflict described in the .rej file？ Thanks. D. Wythe
On 24/11/2022 20:53, D. Wythe wrote: > > > On 11/24/22 9:30 PM, Jan Karcher wrote: >> >> >> On 24/11/2022 09:53, D. Wythe wrote: >>> >>> >>> On 11/24/22 4:33 PM, Jan Karcher wrote: >>>> >>>> >>>> On 24/11/2022 06:55, D. Wythe wrote: >>>>> >>>>> >>>>> On 11/23/22 11:54 PM, D.Wythe wrote: >>>>>> From: "D.Wythe" <alibuda@linux.alibaba.com> >>>>>> >>>>>> This patch set attempts to optimize the parallelism of SMC-R >>>>>> connections, >>>>>> mainly to reduce unnecessary blocking on locks, and to fix >>>>>> exceptions that >>>>>> occur after thoses optimization. >>>>>> >>>>> >>>>>> D. Wythe (10): >>>>>> net/smc: Fix potential panic dues to unprotected >>>>>> smc_llc_srv_add_link() >>>>>> net/smc: fix application data exception >>>>>> net/smc: fix SMC_CLC_DECL_ERR_REGRMB without >>>>>> smc_server_lgr_pending >>>>>> net/smc: remove locks smc_client_lgr_pending and >>>>>> smc_server_lgr_pending >>>>>> net/smc: allow confirm/delete rkey response deliver multiplex >>>>>> net/smc: make SMC_LLC_FLOW_RKEY run concurrently >>>>>> net/smc: llc_conf_mutex refactor, replace it with rw_semaphore >>>>>> net/smc: use read semaphores to reduce unnecessary blocking in >>>>>> smc_buf_create() & smcr_buf_unuse() >>>>>> net/smc: reduce unnecessary blocking in smcr_lgr_reg_rmbs() >>>>>> net/smc: replace mutex rmbs_lock and sndbufs_lock with >>>>>> rw_semaphore >>>>>> >>>>>> net/smc/af_smc.c | 74 ++++---- >>>>>> net/smc/smc_core.c | 541 >>>>>> +++++++++++++++++++++++++++++++++++++++++++++++------ >>>>>> net/smc/smc_core.h | 53 +++++- >>>>>> net/smc/smc_llc.c | 285 ++++++++++++++++++++-------- >>>>>> net/smc/smc_llc.h | 6 + >>>>>> net/smc/smc_wr.c | 10 - >>>>>> net/smc/smc_wr.h | 10 + >>>>>> 7 files changed, 801 insertions(+), 178 deletions(-) >>>>>> >>>>> >>>>> Hi Jan and Wenjia, >>>>> >>>>> I'm wondering whether the bug fix patches need to be put together >>>>> in this series. I'm considering >>>>> sending these bug fix patches separately now, which may be better, >>>>> in case that our patch >>>>> might have other problems. These bug fix patches are mainly >>>>> independent, even without my other >>>>> patches, they may be triggered theoretically. >>>> >>>> Hi D. >>>> >>>> Wenjia and i just talked about that. For us it would be better >>>> separating the fixes and the new logic. >>>> If the fixes are independent feel free to post them to net. >>> >>> >>> Got it, I will remove those bug fix patches in the next series and >>> send them separately. >>> And thanks a lot for your test, no matter what the final test results >>> are, I will send a new series >>> to separate them after your test finished. >> >> Hi D., >> >> I have some troubles applying your patches. >> >> error: sha1 information is lacking or useless (net/smc/smc_core.c). >> error: could not build fake ancestor >> Patch failed at 0001 optimize the parallelism of SMC-R connections >> >> Before merging them by hand could you please send the v6 with the >> fixes separated and verify that you are basing on the latest net / >> net-next tree? >> >> That would make it easier for us to test them. >> >> Thank you >> - Jan >> > > Hi Jan, > > It's quite weird, it seems that my patch did based on the latest > net-next tree. > And I try apply it the latest net tree, it's seems work to me too. Maybe > there > is something wrong with the mirror I use. Can you show me the conflict > described > in the .rej file？ Hi D., sorry for the delayed reply: I just re-tried it with path instead of git am and i think i messed it up yesterday. Mea culpa. With patch your changes *can* be applied to the latest net-next. I'm very sorry for the inconvenience. Could you still please send the v6. That way i can verify the fixes separate and we can - if the tests succeed - already apply them. Sorry and thank you - Jan > > Thanks. > D. Wythe > > > > > > >
On 11/25/22 2:54 PM, Jan Karcher wrote: > > > On 24/11/2022 20:53, D. Wythe wrote: >> >> >> On 11/24/22 9:30 PM, Jan Karcher wrote: >>> >>> >>> On 24/11/2022 09:53, D. Wythe wrote: >>>> >>>> >>>> On 11/24/22 4:33 PM, Jan Karcher wrote: >>>>> >>>>> >>>>> On 24/11/2022 06:55, D. Wythe wrote: >>>>>> >>>>>> >>>>>> On 11/23/22 11:54 PM, D.Wythe wrote: >>>>>>> From: "D.Wythe" <alibuda@linux.alibaba.com> >>>>>>> >>>>>>> This patch set attempts to optimize the parallelism of SMC-R connections, >>>>>>> mainly to reduce unnecessary blocking on locks, and to fix exceptions that >>>>>>> occur after thoses optimization. >>>>>>> >>>>>> >>>>>>> D. Wythe (10): >>>>>>> net/smc: Fix potential panic dues to unprotected >>>>>>> smc_llc_srv_add_link() >>>>>>> net/smc: fix application data exception >>>>>>> net/smc: fix SMC_CLC_DECL_ERR_REGRMB without smc_server_lgr_pending >>>>>>> net/smc: remove locks smc_client_lgr_pending and >>>>>>> smc_server_lgr_pending >>>>>>> net/smc: allow confirm/delete rkey response deliver multiplex >>>>>>> net/smc: make SMC_LLC_FLOW_RKEY run concurrently >>>>>>> net/smc: llc_conf_mutex refactor, replace it with rw_semaphore >>>>>>> net/smc: use read semaphores to reduce unnecessary blocking in >>>>>>> smc_buf_create() & smcr_buf_unuse() >>>>>>> net/smc: reduce unnecessary blocking in smcr_lgr_reg_rmbs() >>>>>>> net/smc: replace mutex rmbs_lock and sndbufs_lock with rw_semaphore >>>>>>> >>>>>>> net/smc/af_smc.c | 74 ++++---- >>>>>>> net/smc/smc_core.c | 541 +++++++++++++++++++++++++++++++++++++++++++++++------ >>>>>>> net/smc/smc_core.h | 53 +++++- >>>>>>> net/smc/smc_llc.c | 285 ++++++++++++++++++++-------- >>>>>>> net/smc/smc_llc.h | 6 + >>>>>>> net/smc/smc_wr.c | 10 - >>>>>>> net/smc/smc_wr.h | 10 + >>>>>>> 7 files changed, 801 insertions(+), 178 deletions(-) >>>>>>> >>>>>> >>>>>> Hi Jan and Wenjia, >>>>>> >>>>>> I'm wondering whether the bug fix patches need to be put together in this series. I'm considering >>>>>> sending these bug fix patches separately now, which may be better, in case that our patch >>>>>> might have other problems. These bug fix patches are mainly independent, even without my other >>>>>> patches, they may be triggered theoretically. >>>>> >>>>> Hi D. >>>>> >>>>> Wenjia and i just talked about that. For us it would be better separating the fixes and the new logic. >>>>> If the fixes are independent feel free to post them to net. >>>> >>>> >>>> Got it, I will remove those bug fix patches in the next series and send them separately. >>>> And thanks a lot for your test, no matter what the final test results are, I will send a new series >>>> to separate them after your test finished. >>> >>> Hi D., >>> >>> I have some troubles applying your patches. >>> >>> error: sha1 information is lacking or useless (net/smc/smc_core.c). >>> error: could not build fake ancestor >>> Patch failed at 0001 optimize the parallelism of SMC-R connections >>> >>> Before merging them by hand could you please send the v6 with the fixes separated and verify that you are basing on the latest net / net-next tree? >>> >>> That would make it easier for us to test them. >>> >>> Thank you >>> - Jan >>> >> >> Hi Jan, >> >> It's quite weird, it seems that my patch did based on the latest net-next tree. >> And I try apply it the latest net tree, it's seems work to me too. Maybe there >> is something wrong with the mirror I use. Can you show me the conflict described >> in the .rej file？ > > Hi D., > > sorry for the delayed reply: > I just re-tried it with path instead of git am and i think i messed it up yesterday. > Mea culpa. With patch your changes *can* be applied to the latest net-next. > I'm very sorry for the inconvenience. Could you still please send the v6. That way i can verify the fixes separate and we can - if the tests succeed - already apply them. > > Sorry and thank you > - Jan Hi Jan, I have sent the v6 with the fixes patches separated, if you have any suggestion or advise, please let us know. Thanks. D. Wythe
On 26/11/2022 10:08, D. Wythe wrote: > > > On 11/25/22 2:54 PM, Jan Karcher wrote: >> >> >> On 24/11/2022 20:53, D. Wythe wrote: >>> >>> >>> On 11/24/22 9:30 PM, Jan Karcher wrote: >>>> >>>> >>>> On 24/11/2022 09:53, D. Wythe wrote: >>>>> >>>>> >>>>> On 11/24/22 4:33 PM, Jan Karcher wrote: >>>>>> >>>>>> >>>>>> On 24/11/2022 06:55, D. Wythe wrote: >>>>>>> >>>>>>> >>>>>>> On 11/23/22 11:54 PM, D.Wythe wrote: >>>>>>>> From: "D.Wythe" <alibuda@linux.alibaba.com> >>>>>>>> >>>>>>>> This patch set attempts to optimize the parallelism of SMC-R >>>>>>>> connections, >>>>>>>> mainly to reduce unnecessary blocking on locks, and to fix >>>>>>>> exceptions that >>>>>>>> occur after thoses optimization. >>>>>>>> >>>>>>> >>>>>>>> D. Wythe (10): >>>>>>>> net/smc: Fix potential panic dues to unprotected >>>>>>>> smc_llc_srv_add_link() >>>>>>>> net/smc: fix application data exception >>>>>>>> net/smc: fix SMC_CLC_DECL_ERR_REGRMB without >>>>>>>> smc_server_lgr_pending >>>>>>>> net/smc: remove locks smc_client_lgr_pending and >>>>>>>> smc_server_lgr_pending >>>>>>>> net/smc: allow confirm/delete rkey response deliver multiplex >>>>>>>> net/smc: make SMC_LLC_FLOW_RKEY run concurrently >>>>>>>> net/smc: llc_conf_mutex refactor, replace it with rw_semaphore >>>>>>>> net/smc: use read semaphores to reduce unnecessary blocking in >>>>>>>> smc_buf_create() & smcr_buf_unuse() >>>>>>>> net/smc: reduce unnecessary blocking in smcr_lgr_reg_rmbs() >>>>>>>> net/smc: replace mutex rmbs_lock and sndbufs_lock with >>>>>>>> rw_semaphore >>>>>>>> >>>>>>>> net/smc/af_smc.c | 74 ++++---- >>>>>>>> net/smc/smc_core.c | 541 >>>>>>>> +++++++++++++++++++++++++++++++++++++++++++++++------ >>>>>>>> net/smc/smc_core.h | 53 +++++- >>>>>>>> net/smc/smc_llc.c | 285 ++++++++++++++++++++-------- >>>>>>>> net/smc/smc_llc.h | 6 + >>>>>>>> net/smc/smc_wr.c | 10 - >>>>>>>> net/smc/smc_wr.h | 10 + >>>>>>>> 7 files changed, 801 insertions(+), 178 deletions(-) >>>>>>>> >>>>>>> >>>>>>> Hi Jan and Wenjia, >>>>>>> >>>>>>> I'm wondering whether the bug fix patches need to be put together >>>>>>> in this series. I'm considering >>>>>>> sending these bug fix patches separately now, which may be >>>>>>> better, in case that our patch >>>>>>> might have other problems. These bug fix patches are mainly >>>>>>> independent, even without my other >>>>>>> patches, they may be triggered theoretically. >>>>>> >>>>>> Hi D. >>>>>> >>>>>> Wenjia and i just talked about that. For us it would be better >>>>>> separating the fixes and the new logic. >>>>>> If the fixes are independent feel free to post them to net. >>>>> >>>>> >>>>> Got it, I will remove those bug fix patches in the next series and >>>>> send them separately. >>>>> And thanks a lot for your test, no matter what the final test >>>>> results are, I will send a new series >>>>> to separate them after your test finished. >>>> >>>> Hi D., >>>> >>>> I have some troubles applying your patches. >>>> >>>> error: sha1 information is lacking or useless >>>> (net/smc/smc_core.c). >>>> error: could not build fake ancestor >>>> Patch failed at 0001 optimize the parallelism of SMC-R connections >>>> >>>> Before merging them by hand could you please send the v6 with the >>>> fixes separated and verify that you are basing on the latest net / >>>> net-next tree? >>>> >>>> That would make it easier for us to test them. >>>> >>>> Thank you >>>> - Jan >>>> >>> >>> Hi Jan, >>> >>> It's quite weird, it seems that my patch did based on the latest >>> net-next tree. >>> And I try apply it the latest net tree, it's seems work to me too. >>> Maybe there >>> is something wrong with the mirror I use. Can you show me the >>> conflict described >>> in the .rej file？ >> >> Hi D., >> >> sorry for the delayed reply: >> I just re-tried it with path instead of git am and i think i messed it >> up yesterday. >> Mea culpa. With patch your changes *can* be applied to the latest >> net-next. >> I'm very sorry for the inconvenience. Could you still please send the >> v6. That way i can verify the fixes separate and we can - if the tests >> succeed - already apply them. >> >> Sorry and thank you >> - Jan > > > Hi Jan, > > I have sent the v6 with the fixes patches separated, if you have any > suggestion or > advise, please let us know. Hi D., we are reviewing and testing the fixes and the series. Thank you - Jan > > Thanks. > D. Wythe > >
From: "D.Wythe" <alibuda@linux.alibaba.com> This patch set attempts to optimize the parallelism of SMC-R connections, mainly to reduce unnecessary blocking on locks, and to fix exceptions that occur after thoses optimization. According to Off-CPU graph, SMC worker's off-CPU as that: smc_close_passive_work (1.09%) smcr_buf_unuse (1.08%) smc_llc_flow_initiate (1.02%) smc_listen_work (48.17%) __mutex_lock.isra.11 (47.96%) An ideal SMC-R connection process should only block on the IO events of the network, but it's quite clear that the SMC-R connection now is queued on the lock most of the time. The goal of this patchset is to achieve our ideal situation where network IO events are blocked for the majority of the connection lifetime. There are three big locks here: 1. smc_client_lgr_pending & smc_server_lgr_pending 2. llc_conf_mutex 3. rmbs_lock & sndbufs_lock And an implementation issue: 1. confirm/delete rkey msg can't be sent concurrently while protocol allows indeed. Unfortunately,The above problems together affect the parallelism of SMC-R connection. If any of them are not solved. our goal cannot be achieved. After this patch set, we can get a quite ideal off-CPU graph as following: smc_close_passive_work (41.58%) smcr_buf_unuse (41.57%) smc_llc_do_delete_rkey (41.57%) smc_listen_work (39.10%) smc_clc_wait_msg (13.18%) tcp_recvmsg_locked (13.18) smc_listen_find_device (25.87%) smcr_lgr_reg_rmbs (25.87%) smc_llc_do_confirm_rkey (25.87%) We can see that most of the waiting times are waiting for network IO events. This also has a certain performance improvement on our short-lived conenction wrk/nginx benchmark test: +--------------+------+------+-------+--------+------+--------+ |conns/qps |c4 | c8 | c16 | c32 | c64 | c200 | +--------------+------+------+-------+--------+------+--------+ |SMC-R before |9.7k | 10k | 10k | 9.9k | 9.1k | 8.9k | +--------------+------+------+-------+--------+------+--------+ |SMC-R now |13k | 19k | 18k | 16k | 15k | 12k | +--------------+------+------+-------+--------+------+--------+ |TCP |15k | 35k | 51k | 80k | 100k | 162k | +--------------+------+------+-------+--------+------+--------+ The reason why the benefit is not obvious after the number of connections has increased dues to workqueue. If we try to change workqueue to UNBOUND, we can obtain at least 4-5 times performance improvement, reach up to half of TCP. However, this is not an elegant solution, the optimization of it will be much more complicated. But in any case, we will submit relevant optimization patches as soon as possible. Please note that the premise here is that the lock related problem must be solved first, otherwise, no matter how we optimize the workqueue, there won't be much improvement. Because there are a lot of related changes to the code, if you have any questions or suggestions, please let me know. Thanks D. Wythe v1 -> v2: 1. Fix panic in SMC-D scenario 2. Fix lnkc related hashfn calculation exception, caused by operator priority 3. Only wake up one connection if the lnk is not active 4. Delete obsolete unlock logic in smc_listen_work() 5. PATCH format, do Reverse Christmas tree 6. PATCH format, change all xxx_lnk_xxx function to xxx_link_xxx 7. PATCH format, add correct fix tag for the patches for fixes. 8. PATCH format, fix some spelling error 9. PATCH format, rename slow to do_slow v2 -> v3: 1. add SMC-D support, remove the concept of link cluster since SMC-D has no link at all. Replace it by lgr decision maker, who provides suggestions to SMC-D and SMC-R on whether to create new link group. 2. Fix the corruption problem described by PATCH 'fix application data exception' on SMC-D. v3 -> v4: 1. Fix panic caused by uninitialization map. v4 -> v5: 1. Make SMC-D buf creation be serial to avoid Potential error 2. Add a flag to synchronize the success of the first contact with the ready of the link group, including SMC-D and SMC-R. 3. Fixed possible reference count leak in smc_llc_flow_start(). 4. reorder the patch, make bugfix PATCH be ahead. D. Wythe (10): net/smc: Fix potential panic dues to unprotected smc_llc_srv_add_link() net/smc: fix application data exception net/smc: fix SMC_CLC_DECL_ERR_REGRMB without smc_server_lgr_pending net/smc: remove locks smc_client_lgr_pending and smc_server_lgr_pending net/smc: allow confirm/delete rkey response deliver multiplex net/smc: make SMC_LLC_FLOW_RKEY run concurrently net/smc: llc_conf_mutex refactor, replace it with rw_semaphore net/smc: use read semaphores to reduce unnecessary blocking in smc_buf_create() & smcr_buf_unuse() net/smc: reduce unnecessary blocking in smcr_lgr_reg_rmbs() net/smc: replace mutex rmbs_lock and sndbufs_lock with rw_semaphore net/smc/af_smc.c | 74 ++++---- net/smc/smc_core.c | 541 +++++++++++++++++++++++++++++++++++++++++++++++------ net/smc/smc_core.h | 53 +++++- net/smc/smc_llc.c | 285 ++++++++++++++++++++-------- net/smc/smc_llc.h | 6 + net/smc/smc_wr.c | 10 - net/smc/smc_wr.h | 10 + 7 files changed, 801 insertions(+), 178 deletions(-)