Messages in this thread | | | Date | Tue, 28 Feb 2023 18:08:18 +0800 | Subject | Re: [PATCH v3 0/8] make slab shrink lockless | From | Qi Zheng <> |
| |
On 2023/2/28 03:20, Kirill Tkhai wrote: > On 27.02.2023 18:08, Mike Rapoport wrote: >> Hi, >> >> On Mon, Feb 27, 2023 at 09:31:51PM +0800, Qi Zheng wrote: >>> >>> >>> On 2023/2/27 03:51, Andrew Morton wrote: >>>> On Sun, 26 Feb 2023 22:46:47 +0800 Qi Zheng <zhengqi.arch@bytedance.com> wrote: >>>> >>>>> Hi all, >>>>> >>>>> This patch series aims to make slab shrink lockless. >>>> >>>> What an awesome changelog. >>>> >>>>> 2. Survey >>>>> ========= >>>> >>>> Especially this part. >>>> >>>> Looking through all the prior efforts and at this patchset I am not >>>> immediately seeing any statements about the overall effect upon >>>> real-world workloads. For a good example, does this patchset >>>> measurably improve throughput or energy consumption on your servers? >>> >>> Hi Andrew, >>> >>> I re-tested with the following physical machines: >>> >>> Architecture: x86_64 >>> CPU(s): 96 >>> On-line CPU(s) list: 0-95 >>> Model name: Intel(R) Xeon(R) Platinum 8260 CPU @ 2.40GHz >>> >>> I found that the reason for the hotspot I described in cover letter is >>> wrong. The reason for the down_read_trylock() hotspot is not because of >>> the failure to trylock, but simply because of the atomic operation >>> (cmpxchg). And this will lead to a significant reduction in IPC (insn >>> per cycle). >> >> ... >> >>> Then we can use the following perf command to view hotspots: >>> >>> perf top -U -F 999 >>> >>> 1) Before applying this patchset: >>> >>> 32.31% [kernel] [k] down_read_trylock >>> 19.40% [kernel] [k] pv_native_safe_halt >>> 16.24% [kernel] [k] up_read >>> 15.70% [kernel] [k] shrink_slab >>> 4.69% [kernel] [k] _find_next_bit >>> 2.62% [kernel] [k] shrink_node >>> 1.78% [kernel] [k] shrink_lruvec >>> 0.76% [kernel] [k] do_shrink_slab >>> >>> 2) After applying this patchset: >>> >>> 27.83% [kernel] [k] _find_next_bit >>> 16.97% [kernel] [k] shrink_slab >>> 15.82% [kernel] [k] pv_native_safe_halt >>> 9.58% [kernel] [k] shrink_node >>> 8.31% [kernel] [k] shrink_lruvec >>> 5.64% [kernel] [k] do_shrink_slab >>> 3.88% [kernel] [k] mem_cgroup_iter >>> >>> 2. At the same time, we use the following perf command to capture IPC >>> information: >>> >>> perf stat -e cycles,instructions -G test -a --repeat 5 -- sleep 10 >>> >>> 1) Before applying this patchset: >>> >>> Performance counter stats for 'system wide' (5 runs): >>> >>> 454187219766 cycles test ( >>> +- 1.84% ) >>> 78896433101 instructions test # 0.17 insn per >>> cycle ( +- 0.44% ) >>> >>> 10.0020430 +- 0.0000366 seconds time elapsed ( +- 0.00% ) >>> >>> 2) After applying this patchset: >>> >>> Performance counter stats for 'system wide' (5 runs): >>> >>> 841954709443 cycles test ( >>> +- 15.80% ) (98.69%) >>> 527258677936 instructions test # 0.63 insn per >>> cycle ( +- 15.11% ) (98.68%) >>> >>> 10.01064 +- 0.00831 seconds time elapsed ( +- 0.08% ) >>> >>> We can see that IPC drops very seriously when calling >>> down_read_trylock() at high frequency. After using SRCU, >>> the IPC is at a normal level. >> >> The results you present do show improvement in IPC for an artificial test >> script. But more interesting would be to see how a real world workloads >> benefit from your changes. > > One of the real workloads from my experience is start of an overcommitted node > containing many starting containers after node crash (or many resuming containers > after reboot for kernel update). In these cases memory pressure is huge, and > the node goes round in long reclaim.
Thanks a lot for providing this real workload! :)
> > This patch patchset makes prealloc_memcg_shrinker() independent of do_shrink_slab(), > so prealloc_memcg_shrinker() won't have to wait till shrink_slab_memcg() completes its > current bit iteration, sees rwsem_is_contended() and the iteration breaks. > > Also, it's important to mention that currently we have the strange behavior: > > prealloc_memcg_shrinker() > down_write(&shrinker_rwsem) > idr_alloc() > reclaim > for each child memcg > shrink_slab_memcg() > down_read_trylock(&shrinker_rwsem) -> fail > > All the slab reclaim in this behavior is just a parasite work, and it just wastes > our cpu time, which does not look a good design. > > Kirill
-- Thanks, Qi
| |