| Message ID | 20240321121457.362921-1-thibault.ferrante@canonical.com |
|---|---|
| Headers | show |
| Series | ipc/msg: mitigate the lock contention with percpu counter | expand |
On 21.03.24 13:14, Thibault Ferrante wrote: > BugLink: https://bugs.launchpad.net/bugs/2058485 > > [Impact] > > The msg_bytes and msg_hdrs atomic counters are frequently updated when IPC msg queue is in heavy use, causing heavy > cache bounce and overhead. Change them to percpu_counter greatly improve the performance. Since there is one percpu > struct per namespace, additional memory cost is minimal. Reading of the count done in msgctl call, which is infrequent. > So the need to sum up the counts in each CPU is infrequent. > > [Fix] > > Backport: > 72d1e611082e ipc/msg: mitigate the lock contention with percpu counter > > For clean backport/build, those are also required: > > 5d0ce3595ab75 percpu: add percpu_counter_add_local and percpu_counter_sub_local > 38cd5b12b7854 ipc: Remove extra braces > 0889f44e28103 ipc: Check permissions for checkpoint_restart sysctls at open time > dd141a4955d5e ipc: Remove extra1 field abuse to pass ipc namespace > def7343ff03bb ipc: Use the same namespace to modify and validate > 1f5c135ee509e ipc: Store ipc sysctls in the ipc namespace > dc55e35f9e810 ipc: Store mqueue sysctls in the ipc namespace > 0e9beb8a96f21 ipc/ipc_sysctl.c: remove fallback for !CONFIG_PROC_SYSCTL > 5563cabdde7ee ipc: check checkpoint_restore_ns_capable() to modify C/R proc files > > > [Test Plan] > > Test as the original patch, with pts/stress-ng message passing > and compare performance. > > [Where problems could occur] > > Performance regression in IPC communication/workload. > > Alexey Gladkov (5): > ipc: Store mqueue sysctls in the ipc namespace > ipc: Store ipc sysctls in the ipc namespace > ipc: Use the same namespace to modify and validate > ipc: Remove extra1 field abuse to pass ipc namespace > ipc: Check permissions for checkpoint_restart sysctls at open time > > Jiebin Sun (2): > percpu: add percpu_counter_add_local and percpu_counter_sub_local > ipc/msg: mitigate the lock contention with percpu counter > > Manfred Spraul (1): > ipc/ipc_sysctl.c: remove fallback for !CONFIG_PROC_SYSCTL > > Michal Clapinski (1): > ipc: check checkpoint_restore_ns_capable() to modify C/R proc files > > include/linux/ipc_namespace.h | 42 ++++++- > include/linux/percpu_counter.h | 32 ++++++ > ipc/ipc_sysctl.c | 204 ++++++++++++++++++++------------- > ipc/mq_sysctl.c | 121 ++++++++++--------- > ipc/mqueue.c | 10 +- > ipc/msg.c | 48 +++++--- > ipc/namespace.c | 15 ++- > ipc/util.h | 4 +- > 8 files changed, 315 insertions(+), 161 deletions(-) > As those are all cherry picks and already (confirmed via back channel) regression tested with stress-ng and the improvements are valuable for distro-users. Acked-by: Stefan Bader <stefan.bader@canonical.com>
On 21/03/2024 13:14, Thibault Ferrante wrote: > BugLink: https://bugs.launchpad.net/bugs/2058485 > > [Impact] > > The msg_bytes and msg_hdrs atomic counters are frequently updated when IPC msg queue is in heavy use, causing heavy > cache bounce and overhead. Change them to percpu_counter greatly improve the performance. Since there is one percpu > struct per namespace, additional memory cost is minimal. Reading of the count done in msgctl call, which is infrequent. > So the need to sum up the counts in each CPU is infrequent. > > [Fix] > > Backport: > 72d1e611082e ipc/msg: mitigate the lock contention with percpu counter > > For clean backport/build, those are also required: > > 5d0ce3595ab75 percpu: add percpu_counter_add_local and percpu_counter_sub_local > 38cd5b12b7854 ipc: Remove extra braces > 0889f44e28103 ipc: Check permissions for checkpoint_restart sysctls at open time > dd141a4955d5e ipc: Remove extra1 field abuse to pass ipc namespace > def7343ff03bb ipc: Use the same namespace to modify and validate > 1f5c135ee509e ipc: Store ipc sysctls in the ipc namespace > dc55e35f9e810 ipc: Store mqueue sysctls in the ipc namespace > 0e9beb8a96f21 ipc/ipc_sysctl.c: remove fallback for !CONFIG_PROC_SYSCTL > 5563cabdde7ee ipc: check checkpoint_restore_ns_capable() to modify C/R proc files > > > [Test Plan] > > Test as the original patch, with pts/stress-ng message passing > and compare performance. > > [Where problems could occur] > > Performance regression in IPC communication/workload. > > Alexey Gladkov (5): > ipc: Store mqueue sysctls in the ipc namespace > ipc: Store ipc sysctls in the ipc namespace > ipc: Use the same namespace to modify and validate > ipc: Remove extra1 field abuse to pass ipc namespace > ipc: Check permissions for checkpoint_restart sysctls at open time > > Jiebin Sun (2): > percpu: add percpu_counter_add_local and percpu_counter_sub_local > ipc/msg: mitigate the lock contention with percpu counter > > Manfred Spraul (1): > ipc/ipc_sysctl.c: remove fallback for !CONFIG_PROC_SYSCTL > > Michal Clapinski (1): > ipc: check checkpoint_restore_ns_capable() to modify C/R proc files > > include/linux/ipc_namespace.h | 42 ++++++- > include/linux/percpu_counter.h | 32 ++++++ > ipc/ipc_sysctl.c | 204 ++++++++++++++++++++------------- > ipc/mq_sysctl.c | 121 ++++++++++--------- > ipc/mqueue.c | 10 +- > ipc/msg.c | 48 +++++--- > ipc/namespace.c | 15 ++- > ipc/util.h | 4 +- > 8 files changed, 315 insertions(+), 161 deletions(-) Acked-by: Roxana Nicolescu <roxana.nicolescu@canonical.com>
On 21.03.24 13:14, Thibault Ferrante wrote: > BugLink: https://bugs.launchpad.net/bugs/2058485 > > [Impact] > > The msg_bytes and msg_hdrs atomic counters are frequently updated when IPC msg queue is in heavy use, causing heavy > cache bounce and overhead. Change them to percpu_counter greatly improve the performance. Since there is one percpu > struct per namespace, additional memory cost is minimal. Reading of the count done in msgctl call, which is infrequent. > So the need to sum up the counts in each CPU is infrequent. > > [Fix] > > Backport: > 72d1e611082e ipc/msg: mitigate the lock contention with percpu counter > > For clean backport/build, those are also required: > > 5d0ce3595ab75 percpu: add percpu_counter_add_local and percpu_counter_sub_local > 38cd5b12b7854 ipc: Remove extra braces > 0889f44e28103 ipc: Check permissions for checkpoint_restart sysctls at open time > dd141a4955d5e ipc: Remove extra1 field abuse to pass ipc namespace > def7343ff03bb ipc: Use the same namespace to modify and validate > 1f5c135ee509e ipc: Store ipc sysctls in the ipc namespace > dc55e35f9e810 ipc: Store mqueue sysctls in the ipc namespace > 0e9beb8a96f21 ipc/ipc_sysctl.c: remove fallback for !CONFIG_PROC_SYSCTL > 5563cabdde7ee ipc: check checkpoint_restore_ns_capable() to modify C/R proc files > > > [Test Plan] > > Test as the original patch, with pts/stress-ng message passing > and compare performance. > > [Where problems could occur] > > Performance regression in IPC communication/workload. > > Alexey Gladkov (5): > ipc: Store mqueue sysctls in the ipc namespace > ipc: Store ipc sysctls in the ipc namespace > ipc: Use the same namespace to modify and validate > ipc: Remove extra1 field abuse to pass ipc namespace > ipc: Check permissions for checkpoint_restart sysctls at open time > > Jiebin Sun (2): > percpu: add percpu_counter_add_local and percpu_counter_sub_local > ipc/msg: mitigate the lock contention with percpu counter > > Manfred Spraul (1): > ipc/ipc_sysctl.c: remove fallback for !CONFIG_PROC_SYSCTL > > Michal Clapinski (1): > ipc: check checkpoint_restore_ns_capable() to modify C/R proc files > > include/linux/ipc_namespace.h | 42 ++++++- > include/linux/percpu_counter.h | 32 ++++++ > ipc/ipc_sysctl.c | 204 ++++++++++++++++++++------------- > ipc/mq_sysctl.c | 121 ++++++++++--------- > ipc/mqueue.c | 10 +- > ipc/msg.c | 48 +++++--- > ipc/namespace.c | 15 ++- > ipc/util.h | 4 +- > 8 files changed, 315 insertions(+), 161 deletions(-) > Applied to jammy:linux/master-next. Thanks. -Stefan