Created attachment 159915 [details] patch for sa.c After r286575, there is a possible assert fail in: sa_handle_get_from_db() -> dmu_buf_init_user() -> ASSERT(dbu->dbu_evict_func == NULL); When the sa_cache zone first allocates memory from VM, sa_cache_constructor() does not initialize the dbu_evict_func that contains garbage. This will trigger the assert. Only after the memory is freed, the sa_cache_destructor() set dbu_evict_func to NULL and returns it to the zone for next use. So, the next time sa_handle_get_from_db() allocates the same memory from the zone, the dbu_evict_func is NULL.
(In reply to luke.tw from comment #0) I think that you correctly identified the problem. And what you describe seems to be only the part of the problem. It seems that illumos kmem cache API is used incorrectly for sa_cache. Its usage resembles how FreeBSD uma(9) could be used and that almost works with FreeBSD's emulation of kmem cache. The difference is that kmem_cache_create() is not as flexible as uma_create(): whereas the latter supports two ways of initialization an object - via init/fini and constructor/destructor, the former has only constructor/destructor support: http://illumos.org/man/9F/kmem_cache_create But the kmem cache's constructor and destructor work similarly to how the init and fini work in uma(9). And, apparently, that is a source of the confusion here. I am surprised that there are no bug reports about this API misuse from illumos users yet. It seems that the problem was introduced as a part of bigger changes in base r286575 which is an import of illumos/illumos-gate@bc9014e6a81272073b9854d9f65dd59e18d18c35
Created attachment 160132 [details] proposed patch
You are right. The proposed patch looks good and works.
There seems to have been a bad merge from illumos to FreeBSD. After https://github.com/illumos/illumos-gate/commit/0fda3cc5c1c5a1d9bdea6d52637bef6e781549c9, handle->sa_dbu.dbu_evict_func is initialized to NULL after the handle is allocated, thus avoiding the assert. The assert in sa_cache_destructor() was also removed. You can see the full discussion for this change here: https://reviews.csiden.org/r/154/ I think FreeBSD should just sync up with illumos rather than diverge further.
(In reply to Justin T. Gibbs from comment #4) Justin, thank you very much for this information. That explains why illumos users do not complain about the issue at hand. I sure missed the commit that you've pointed out, not sure how I missed it though. I will import the fix.
A commit references this bug: Author: avg Date: Fri Aug 21 08:04:57 UTC 2015 New revision: 286983 URL: https://svnweb.freebsd.org/changeset/base/286983 Log: fix a mismerge in r286539 (MFV 286538: 5562 ZFS sa_handle's violate...) PR: 202358 X-MFC with: r286539 X-MFC attn: mav Changes: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sa.c