abd using wrong size on free

[lundman]

panic(cpu 0): VERIFY3(size == abd->abd_orig_size) failed (120 == 128)

abd_alloc_scatter_offset_chunkcnt: abd 0xffffff90ca886080 chunkcnt 4 abd_size 128
abd_free_struct: abd 0xffffff90ca886080 ->abd_size 8192 chunkcnt 2 size 120
VERIFY3(size == abd->abd_orig_size) failed (120 == 128)

Ie, abd_alloc_scatter_offset_chunkcnt() allocates with 4 chunkcnt, but abd_free_struct uses 2 when calling free.

as discussed in openzfs/zfs#10431

zfstests: zpool_upgrade

[lundman]

Inviting @bwatkinson for assistance when they have time

[bwatkinson]

@lundman could you point me to you OSX branch? I was going to look over the abd_os.c to see if I can spot anything.

[lundman]

sorted2 - is the one to use.

https://github.com/openzfsonosx/openzfs/blob/sorted2/module/os/macos/zfs/abd_os.c

#  diff -ub module/os/freebsd/zfs/abd_os.c module/os/macos/zfs/abd_os.c
--- module/os/freebsd/zfs/abd_os.c      2020-06-22 13:50:15.000000000 +0900
+++ module/os/macos/zfs/abd_os.c        2020-06-23 15:35:00.000000000 +0900
@@ -12,6 +12,7 @@
 /*
  * Copyright (c) 2014 by Chunwei Chen. All rights reserved.
  * Copyright (c) 2016 by Delphix. All rights reserved.
+ * Copyright (c) 2020 by Jorgen Lundman. All rights reserved.
  */

 /*
@@ -78,18 +79,10 @@
  */
 size_t zfs_abd_chunk_size = 4096;

-#if defined(_KERNEL)
-SYSCTL_DECL(_vfs_zfs);
-
-SYSCTL_INT(_vfs_zfs, OID_AUTO, abd_scatter_enabled, CTLFLAG_RWTUN,
-       &zfs_abd_scatter_enabled, 0, "Enable scattered ARC data buffers");
-SYSCTL_ULONG(_vfs_zfs, OID_AUTO, abd_chunk_size, CTLFLAG_RDTUN,
-       &zfs_abd_chunk_size, 0, "The size of the chunks ABD allocates");
-#endif
-
 kmem_cache_t *abd_chunk_cache;
 static kstat_t *abd_ksp;

+
 /*
  * We use a scattered SPA_MAXBLOCKSIZE sized ABD whose chunks are
  * just a single zero'd sized zfs_abd_chunk_size buffer. This
@@ -198,17 +191,11 @@
 abd_alloc_struct(size_t size)
 {
        size_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
-        * ABD struct as opposed to an ABD scatter with 0 chunks. The gang
-        * ABD struct allocation accounts for an additional 24 bytes over
-        * a scatter ABD with 0 chunks.
-        */
-       size_t abd_size = MAX(sizeof (abd_t),
-           offsetof(abd_t, abd_u.abd_scatter.abd_chunks[chunkcnt]));
-       abd_t *abd = kmem_alloc(abd_size, KM_PUSHPAGE);
+       size_t abd_size = offsetof(abd_t,
+           abd_u.abd_scatter.abd_chunks[chunkcnt]);
+       abd_t *abd = kmem_alloc(MAX(abd_size, sizeof(abd_t)), KM_PUSHPAGE);
        ASSERT3P(abd, !=, NULL);
+       abd->abd_orig_size = MAX(abd_size, sizeof(abd_t));
        list_link_init(&abd->abd_gang_link);
        mutex_init(&abd->abd_mtx, NULL, MUTEX_DEFAULT, NULL);
        ABDSTAT_INCR(abdstat_struct_size, abd_size);
@@ -219,13 +206,12 @@
 void
 abd_free_struct(abd_t *abd)
 {
-       size_t chunkcnt = abd_is_linear(abd) || abd_is_gang(abd) ? 0 :
-           abd_scatter_chunkcnt(abd);
-       int size = MAX(sizeof (abd_t),
-           offsetof(abd_t, abd_u.abd_scatter.abd_chunks[chunkcnt]));
+       size_t chunkcnt = abd_is_linear(abd) ? 0 : abd_scatter_chunkcnt(abd);
+       int size = offsetof(abd_t, abd_u.abd_scatter.abd_chunks[chunkcnt]);
        mutex_destroy(&abd->abd_mtx);
        ASSERT(!list_link_active(&abd->abd_gang_link));
-       kmem_free(abd, size);
+
+       kmem_free(abd, abd->abd_orig_size);
        ABDSTAT_INCR(abdstat_struct_size, -size);
 }

@@ -336,8 +322,9 @@
 {
        size_t abd_size = offsetof(abd_t,
            abd_u.abd_scatter.abd_chunks[chunkcnt]);
-       abd_t *abd = kmem_alloc(abd_size, KM_PUSHPAGE);
+       abd_t *abd = kmem_alloc(MAX(abd_size, sizeof(abd_t)), KM_PUSHPAGE);
        ASSERT3P(abd, !=, NULL);
+       abd->abd_orig_size = MAX(abd_size, sizeof(abd_t));
        list_link_init(&abd->abd_gang_link);
        mutex_init(&abd->abd_mtx, NULL, MUTEX_DEFAULT, NULL);
        ABDSTAT_INCR(abdstat_struct_size, abd_size);
@@ -492,5 +479,8 @@
 void
 abd_cache_reap_now(void)
 {
-       kmem_cache_reap_soon(abd_chunk_cache);
+       kmem_cache_reap_now(abd_chunk_cache);
 }
+
+
+

[lundman]

Personally, I am guessing that if you kept abd_orig_size on other platforms, you will have the same problem :)

[lundman]

8a6d444

revert
f8e04c2