Skip to content

Commit 05de879

Browse files
rottegiftlundman
authored andcommitted
Fix abd leak, kmem_free correct size of abd_t
... for macOS and Freebsd, and improve macOS abd performance (#56) (macOS continuation to this is in 47b408f) * Cleanup in macos abd_os.c to fix abd_t leak Fix a leak of abd_t that manifested mostly when using raidzN with at least as many columns as N (e.g. a four-disk raidz2 but not a three-disk raidz2). Sufficiently heavy raidz use would eventually run a system out of memory. The leak was introduced as a fix for a panic caused by calculating the wrong size of an abd_t at free time if the abd_t had been made using abd_get_offset_impl, since it carried along the unnecessary tails of large ABDs, leading to a mismatch between abd->abd_size and the original allocation size of the abd_t. This would feed kmem_free a bad size, which produces a heap corruption panic. The fix now carries only the necessary chunk pointers, leading to smaller abd_ts (especially those of abd_get_zeros() ABDs) and a performance gain from the reduction in copying and allocation activity. We now calculate the correct size for the abd_t at free time. This requires passing the number of bytes wanted in a scatter ABD to abd_get_offset_scatter(). Additionally: * Switch abd_cache arena to FIRSTFIT, which empirically improves perofrmance. * Make abd_chunk_cache more performant and debuggable. * Allocate the abd_zero_buf from abd_chunk_cache rather than the heap. * Don't try to reap non-existent qcaches in abd_cache arena. * KM_PUSHPAGE->KM_SLEEP when allocating chunks from their own arena - having fixed the abd leaks, return to using KMF_LITE, but leave a commented example of audit kmem debugging - having made this work, abd_orig_size is no longer needed as a way to track the size originally kmem_zalloc-ed for a scatter abd_t * Update FreeBSD abd_os.c with the fix, and let Linux build * Minimal change to fix FreeBSD's abd_get_offset_scatter() carrying too many chunks for the desired ABD size * A size argument is added to abd_get_offset_scatter() for FreeBSD and macOS, which is unused by Linux Signed-off-by: Jorgen Lundman <[email protected]>
1 parent f20fb19 commit 05de879

File tree

4 files changed

+10
-6
lines changed

4 files changed

+10
-6
lines changed

include/sys/abd_impl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ void abd_free_struct(abd_t *);
6464
*/
6565

6666
abd_t *abd_alloc_struct_impl(size_t);
67-
abd_t *abd_get_offset_scatter(abd_t *, abd_t *, size_t);
67+
abd_t *abd_get_offset_scatter(abd_t *, abd_t *, size_t, size_t);
6868
void abd_free_struct_impl(abd_t *);
6969
void abd_alloc_chunks(abd_t *, size_t);
7070
void abd_free_chunks(abd_t *);

module/os/freebsd/zfs/abd_os.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -374,14 +374,17 @@ abd_alloc_for_io(size_t size, boolean_t is_metadata)
374374
}
375375

376376
abd_t *
377-
abd_get_offset_scatter(abd_t *abd, abd_t *sabd, size_t off)
377+
abd_get_offset_scatter(abd_t *abd, abd_t *sabd, size_t off,
378+
size_t size)
378379
{
379380
abd_verify(sabd);
380381
ASSERT3U(off, <=, sabd->abd_size);
381382

382383
size_t new_offset = ABD_SCATTER(sabd).abd_offset + off;
383-
uint_t chunkcnt = abd_scatter_chunkcnt(sabd) -
384-
(new_offset / zfs_abd_chunk_size);
384+
size_t chunkcnt = abd_chunkcnt_for_bytes(
385+
(new_offset % zfs_abd_chunk_size) + size);
386+
387+
ASSERT3U(chunkcnt, <=, abd_scatter_chunkcnt(sabd));
385388

386389
/*
387390
* If an abd struct is provided, it is only the minimum size. If we

module/os/linux/zfs/abd_os.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -835,7 +835,8 @@ abd_alloc_for_io(size_t size, boolean_t is_metadata)
835835
}
836836

837837
abd_t *
838-
abd_get_offset_scatter(abd_t *abd, abd_t *sabd, size_t off)
838+
abd_get_offset_scatter(abd_t *abd, abd_t *sabd, size_t off,
839+
size_t size)
839840
{
840841
int i = 0;
841842
struct scatterlist *sg = NULL;

module/zfs/abd.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,7 @@ abd_get_offset_impl(abd_t *abd, abd_t *sabd, size_t off, size_t size)
531531
}
532532
ASSERT3U(left, ==, 0);
533533
} else {
534-
abd = abd_get_offset_scatter(abd, sabd, off);
534+
abd = abd_get_offset_scatter(abd, sabd, off, size);
535535
}
536536

537537
ASSERT3P(abd, !=, NULL);

0 commit comments

Comments
 (0)