Skip to content

Commit ec1fea4

Browse files
authored
Use percpu_counter for obj_alloc counter of Linux-backed caches
A previous commit enabled the tracking of object allocations in Linux-backed caches from the SPL layer for debuggability. The commit is: 9a170fc Unfortunately, it also introduced minor performance regressions that were highlighted by the ZFS perf test-suite. Within Delphix we found that the regression would be from -1%, all the way up to -8% for some workloads. This commit brings performance back up to par by creating a separate counter for those caches and making it a percpu in order to avoid lock-contention. The initial performance testing was done by myself, and the final round was conducted by @tonynguien who was also the one that discovered the regression and highlighted the culprit. Reviewed-by: Matt Ahrens <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Serapheim Dimitropoulos <[email protected]> Closes #10397
1 parent 7b232e9 commit ec1fea4

File tree

7 files changed

+100
-8
lines changed

7 files changed

+100
-8
lines changed

config/kernel-percpu.m4

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
dnl #
2+
dnl # 3.18 API change,
3+
dnl # The function percpu_counter_init now must be passed a GFP mask.
4+
dnl #
5+
AC_DEFUN([ZFS_AC_KERNEL_SRC_PERCPU_COUNTER_INIT], [
6+
ZFS_LINUX_TEST_SRC([percpu_counter_init_with_gfp], [
7+
#include <linux/gfp.h>
8+
#include <linux/percpu_counter.h>
9+
],[
10+
struct percpu_counter counter;
11+
int error;
12+
13+
error = percpu_counter_init(&counter, 0, GFP_KERNEL);
14+
])
15+
])
16+
17+
AC_DEFUN([ZFS_AC_KERNEL_PERCPU_COUNTER_INIT], [
18+
AC_MSG_CHECKING([whether percpu_counter_init() wants gfp_t])
19+
ZFS_LINUX_TEST_RESULT([percpu_counter_init_with_gfp], [
20+
AC_MSG_RESULT(yes)
21+
AC_DEFINE(HAVE_PERCPU_COUNTER_INIT_WITH_GFP, 1,
22+
[percpu_counter_init() wants gfp_t])
23+
],[
24+
AC_MSG_RESULT(no)
25+
])
26+
])
27+
28+
AC_DEFUN([ZFS_AC_KERNEL_SRC_PERCPU], [
29+
ZFS_AC_KERNEL_SRC_PERCPU_COUNTER_INIT
30+
])
31+
32+
AC_DEFUN([ZFS_AC_KERNEL_PERCPU], [
33+
ZFS_AC_KERNEL_PERCPU_COUNTER_INIT
34+
])

config/kernel.m4

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ AC_DEFUN([ZFS_AC_KERNEL_TEST_SRC], [
121121
ZFS_AC_KERNEL_SRC_TOTALRAM_PAGES_FUNC
122122
ZFS_AC_KERNEL_SRC_TOTALHIGH_PAGES
123123
ZFS_AC_KERNEL_SRC_KSTRTOUL
124+
ZFS_AC_KERNEL_SRC_PERCPU
124125
125126
AC_MSG_CHECKING([for available kernel interfaces])
126127
ZFS_LINUX_TEST_COMPILE_ALL([kabi])
@@ -216,6 +217,7 @@ AC_DEFUN([ZFS_AC_KERNEL_TEST_RESULT], [
216217
ZFS_AC_KERNEL_TOTALRAM_PAGES_FUNC
217218
ZFS_AC_KERNEL_TOTALHIGH_PAGES
218219
ZFS_AC_KERNEL_KSTRTOUL
220+
ZFS_AC_KERNEL_PERCPU
219221
])
220222

221223
dnl #

include/os/linux/kernel/linux/Makefile.am

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ KERNEL_H = \
55
blkdev_compat.h \
66
utsname_compat.h \
77
kmap_compat.h \
8+
percpu_compat.h \
89
simd.h \
910
simd_x86.h \
1011
simd_aarch64.h \
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*
2+
* CDDL HEADER START
3+
*
4+
* The contents of this file are subject to the terms of the
5+
* Common Development and Distribution License (the "License").
6+
* You may not use this file except in compliance with the License.
7+
*
8+
* You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
9+
* or http://www.opensolaris.org/os/licensing.
10+
* See the License for the specific language governing permissions
11+
* and limitations under the License.
12+
*
13+
* When distributing Covered Code, include this CDDL HEADER in each
14+
* file and include the License file at usr/src/OPENSOLARIS.LICENSE.
15+
* If applicable, add the following below this CDDL HEADER, with the
16+
* fields enclosed by brackets "[]" replaced with your own identifying
17+
* information: Portions Copyright [yyyy] [name of copyright owner]
18+
*
19+
* CDDL HEADER END
20+
*/
21+
22+
/*
23+
* Copyright (c) 2020 by Delphix. All rights reserved.
24+
*/
25+
26+
#ifndef _ZFS_PERCPU_H
27+
#define _ZFS_PERCPU_H
28+
29+
#include <linux/percpu_counter.h>
30+
31+
/*
32+
* 3.18 API change,
33+
* percpu_counter_init() now must be passed a gfp mask which will be
34+
* used for the dynamic allocation of the actual counter.
35+
*/
36+
#ifdef HAVE_PERCPU_COUNTER_INIT_WITH_GFP
37+
#define percpu_counter_init_common(counter, n, gfp) \
38+
percpu_counter_init(counter, n, gfp)
39+
#else
40+
#define percpu_counter_init_common(counter, n, gfp) \
41+
percpu_counter_init(counter, n)
42+
#endif
43+
44+
#endif /* _ZFS_PERCPU_H */

include/os/linux/spl/sys/kmem_cache.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ typedef struct spl_kmem_cache {
202202
uint64_t skc_slab_max; /* Slab max historic */
203203
uint64_t skc_obj_total; /* Obj total current */
204204
uint64_t skc_obj_alloc; /* Obj alloc current */
205+
struct percpu_counter skc_linux_alloc; /* Linux-backed Obj alloc */
205206
uint64_t skc_obj_max; /* Obj max historic */
206207
uint64_t skc_obj_deadlock; /* Obj emergency deadlocks */
207208
uint64_t skc_obj_emergency; /* Obj emergency current */

module/os/linux/spl/spl-kmem-cache.c

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include <sys/wait.h>
3232
#include <linux/slab.h>
3333
#include <linux/swap.h>
34+
#include <linux/percpu_compat.h>
3435
#include <linux/prefetch.h>
3536

3637
/*
@@ -948,6 +949,13 @@ spl_kmem_cache_create(char *name, size_t size, size_t align,
948949
skc->skc_obj_emergency = 0;
949950
skc->skc_obj_emergency_max = 0;
950951

952+
rc = percpu_counter_init_common(&skc->skc_linux_alloc, 0,
953+
GFP_KERNEL);
954+
if (rc != 0) {
955+
kfree(skc);
956+
return (NULL);
957+
}
958+
951959
/*
952960
* Verify the requested alignment restriction is sane.
953961
*/
@@ -1047,6 +1055,7 @@ spl_kmem_cache_create(char *name, size_t size, size_t align,
10471055
return (skc);
10481056
out:
10491057
kfree(skc->skc_name);
1058+
percpu_counter_destroy(&skc->skc_linux_alloc);
10501059
kfree(skc);
10511060
return (NULL);
10521061
}
@@ -1117,6 +1126,9 @@ spl_kmem_cache_destroy(spl_kmem_cache_t *skc)
11171126
ASSERT3U(skc->skc_obj_emergency, ==, 0);
11181127
ASSERT(list_empty(&skc->skc_complete_list));
11191128

1129+
ASSERT3U(percpu_counter_sum(&skc->skc_linux_alloc), ==, 0);
1130+
percpu_counter_destroy(&skc->skc_linux_alloc);
1131+
11201132
spin_unlock(&skc->skc_lock);
11211133

11221134
kfree(skc->skc_name);
@@ -1473,9 +1485,7 @@ spl_kmem_cache_alloc(spl_kmem_cache_t *skc, int flags)
14731485
* how many objects we've allocated in it for
14741486
* better debuggability.
14751487
*/
1476-
spin_lock(&skc->skc_lock);
1477-
skc->skc_obj_alloc++;
1478-
spin_unlock(&skc->skc_lock);
1488+
percpu_counter_inc(&skc->skc_linux_alloc);
14791489
}
14801490
goto ret;
14811491
}
@@ -1550,9 +1560,7 @@ spl_kmem_cache_free(spl_kmem_cache_t *skc, void *obj)
15501560
*/
15511561
if (skc->skc_flags & KMC_SLAB) {
15521562
kmem_cache_free(skc->skc_linux_cache, obj);
1553-
spin_lock(&skc->skc_lock);
1554-
skc->skc_obj_alloc--;
1555-
spin_unlock(&skc->skc_lock);
1563+
percpu_counter_dec(&skc->skc_linux_alloc);
15561564
return;
15571565
}
15581566

module/os/linux/spl/spl-proc.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -446,16 +446,18 @@ slab_seq_show(struct seq_file *f, void *p)
446446
* the underlying Linux cache please refer to /proc/slabinfo.
447447
*/
448448
spin_lock(&skc->skc_lock);
449+
uint64_t objs_allocated =
450+
percpu_counter_sum(&skc->skc_linux_alloc);
449451
seq_printf(f, "%-36s ", skc->skc_name);
450452
seq_printf(f, "0x%05lx %9s %9lu %8s %8u "
451453
"%5s %5s %5s %5s %5lu %5s %5s %5s %5s\n",
452454
(long unsigned)skc->skc_flags,
453455
"-",
454-
(long unsigned)(skc->skc_obj_size * skc->skc_obj_alloc),
456+
(long unsigned)(skc->skc_obj_size * objs_allocated),
455457
"-",
456458
(unsigned)skc->skc_obj_size,
457459
"-", "-", "-", "-",
458-
(long unsigned)skc->skc_obj_alloc,
460+
(long unsigned)objs_allocated,
459461
"-", "-", "-", "-");
460462
spin_unlock(&skc->skc_lock);
461463
return (0);

0 commit comments

Comments
 (0)