From 6366ef22404ef1bd0ba26364905e19f2ec7df2a4 Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Sun, 6 Dec 2020 12:53:40 -0500 Subject: [PATCH] Bring consistency to ABD chunk count types. With both abd_size and abd_nents being uint_t it makes no sense for abd_chunkcnt_for_bytes() to return size_t. Random mix of different types used to count chunks looks bad and makes compiler more difficult to optimize the code. In particular on FreeBSD this change allows compiler to completely optimize out abd_verify_scatter() when built without debug, removing pointless 64-bit division and even more pointless empty loop. Reviewed-by: Ryan Moeller Reviewed-by: Brian Behlendorf Signed-off-by: Alexander Motin Closes #11279 --- module/os/freebsd/zfs/abd_os.c | 43 ++++++++++++++++++++-------------- module/os/linux/zfs/abd_os.c | 2 +- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/module/os/freebsd/zfs/abd_os.c b/module/os/freebsd/zfs/abd_os.c index a7bda509b..0a323e885 100644 --- a/module/os/freebsd/zfs/abd_os.c +++ b/module/os/freebsd/zfs/abd_os.c @@ -106,13 +106,13 @@ abd_free_chunk(void *c) kmem_cache_free(abd_chunk_cache, c); } -static size_t +static uint_t abd_chunkcnt_for_bytes(size_t size) { return (P2ROUNDUP(size, zfs_abd_chunk_size) / zfs_abd_chunk_size); } -static inline size_t +static inline uint_t abd_scatter_chunkcnt(abd_t *abd) { ASSERT(!abd_is_linear(abd)); @@ -129,7 +129,7 @@ abd_size_alloc_linear(size_t size) void abd_update_scatter_stats(abd_t *abd, abd_stats_op_t op) { - size_t n = abd_scatter_chunkcnt(abd); + uint_t n = abd_scatter_chunkcnt(abd); ASSERT(op == ABDSTAT_INCR || op == ABDSTAT_DECR); int waste = n * zfs_abd_chunk_size - abd->abd_size; if (op == ABDSTAT_INCR) { @@ -161,25 +161,28 @@ abd_update_linear_stats(abd_t *abd, abd_stats_op_t op) void abd_verify_scatter(abd_t *abd) { + uint_t i, n; + /* * There is no scatter linear pages in FreeBSD so there is an * if an error if the ABD has been marked as a linear page. */ - VERIFY(!abd_is_linear_page(abd)); + ASSERT(!abd_is_linear_page(abd)); ASSERT3U(ABD_SCATTER(abd).abd_offset, <, zfs_abd_chunk_size); - size_t n = abd_scatter_chunkcnt(abd); - for (int i = 0; i < n; i++) { - ASSERT3P( - ABD_SCATTER(abd).abd_chunks[i], !=, NULL); + n = abd_scatter_chunkcnt(abd); + for (i = 0; i < n; i++) { + ASSERT3P(ABD_SCATTER(abd).abd_chunks[i], !=, NULL); } } void abd_alloc_chunks(abd_t *abd, size_t size) { - size_t n = abd_chunkcnt_for_bytes(size); - for (int i = 0; i < n; i++) { + uint_t i, n; + + n = abd_chunkcnt_for_bytes(size); + for (i = 0; i < n; i++) { void *c = kmem_cache_alloc(abd_chunk_cache, KM_PUSHPAGE); ASSERT3P(c, !=, NULL); ABD_SCATTER(abd).abd_chunks[i] = c; @@ -190,8 +193,10 @@ abd_alloc_chunks(abd_t *abd, size_t size) void abd_free_chunks(abd_t *abd) { - size_t n = abd_scatter_chunkcnt(abd); - for (int i = 0; i < n; i++) { + uint_t i, n; + + n = abd_scatter_chunkcnt(abd); + for (i = 0; i < n; i++) { abd_free_chunk(ABD_SCATTER(abd).abd_chunks[i]); } } @@ -199,7 +204,7 @@ abd_free_chunks(abd_t *abd) abd_t * abd_alloc_struct(size_t size) { - size_t chunkcnt = abd_chunkcnt_for_bytes(size); + uint_t chunkcnt = abd_chunkcnt_for_bytes(size); /* * In the event we are allocating a gang ABD, the size passed in * will be 0. We must make sure to set abd_size to the size of an @@ -221,9 +226,9 @@ abd_alloc_struct(size_t size) void abd_free_struct(abd_t *abd) { - size_t chunkcnt = abd_is_linear(abd) || abd_is_gang(abd) ? 0 : + uint_t chunkcnt = abd_is_linear(abd) || abd_is_gang(abd) ? 0 : abd_scatter_chunkcnt(abd); - int size = MAX(sizeof (abd_t), + ssize_t size = MAX(sizeof (abd_t), offsetof(abd_t, abd_u.abd_scatter.abd_chunks[chunkcnt])); mutex_destroy(&abd->abd_mtx); ASSERT(!list_link_active(&abd->abd_gang_link)); @@ -238,7 +243,9 @@ abd_free_struct(abd_t *abd) static void abd_alloc_zero_scatter(void) { - size_t n = abd_chunkcnt_for_bytes(SPA_MAXBLOCKSIZE); + uint_t i, n; + + n = abd_chunkcnt_for_bytes(SPA_MAXBLOCKSIZE); abd_zero_buf = kmem_zalloc(zfs_abd_chunk_size, KM_SLEEP); abd_zero_scatter = abd_alloc_struct(SPA_MAXBLOCKSIZE); @@ -251,7 +258,7 @@ abd_alloc_zero_scatter(void) ABD_SCATTER(abd_zero_scatter).abd_chunk_size = zfs_abd_chunk_size; - for (int i = 0; i < n; i++) { + for (i = 0; i < n; i++) { ABD_SCATTER(abd_zero_scatter).abd_chunks[i] = abd_zero_buf; } @@ -356,7 +363,7 @@ abd_get_offset_scatter(abd_t *sabd, size_t off) ASSERT3U(off, <=, sabd->abd_size); size_t new_offset = ABD_SCATTER(sabd).abd_offset + off; - size_t chunkcnt = abd_scatter_chunkcnt(sabd) - + uint_t chunkcnt = abd_scatter_chunkcnt(sabd) - (new_offset / zfs_abd_chunk_size); abd = abd_alloc_scatter_offset_chunkcnt(chunkcnt); diff --git a/module/os/linux/zfs/abd_os.c b/module/os/linux/zfs/abd_os.c index c2281449e..0abac2284 100644 --- a/module/os/linux/zfs/abd_os.c +++ b/module/os/linux/zfs/abd_os.c @@ -178,7 +178,7 @@ static struct page *abd_zero_page = NULL; static kmem_cache_t *abd_cache = NULL; static kstat_t *abd_ksp; -static size_t +static uint_t abd_chunkcnt_for_bytes(size_t size) { return (P2ROUNDUP(size, PAGESIZE) / PAGESIZE);