Change Details

In spite of never being heavily involved with ZFS I personally have fixed 3 deadlocks in it over the years. A recent deadlock during testing by a customer caused a major setback for FreeBSD. Thus I'd very much like to move towards leveraging WITNESS. It currently isn't possible to use WITNESS for much of ZFS because the lock name doesn't provide WITNESS with enough context to distinguish between what are in fact two unique locking orders. This change will, at _enrollment_ time mark a lock pair as "blessed". Thus at lock acquisition there is no added cost over the existing code - unlike the previous bless implementation. The added enrollment cost is only borne by locks in the classes that are being blessed (in the case of ZFS sx and lockmgr). The added overhead of enrollment for participating lock types is an extra string compare for each lock that is in a white listed pair. The intent is that with WITNESS enabled, we can incrementally determine how to give WITNESS the context it needs for each lock pair. And even if we aren't reporting reorders, enrolling witness locks mean that we can now inspect them in ddb when we get a lockup. Note that there appears to still be an issue in that the '"zfsvfs->z_hold_mtx[i]"(sx) and "zfs"(lockmgr)!' is still reported.

In spite of never being heavily involved with ZFS I personally have fixed 3 deadlocks in it over the years. A recent deadlock during testing by a customer caused a major setback for FreeBSD. Thus I'd very much like to move towards leveraging WITNESS. It currently isn't possible to use WITNESS for much of ZFS because the lock name doesn't provide WITNESS with enough context to distinguish between what are in fact two unique locking orders. This change will, at _enrollment_ time mark a lock pair as "blessed". Thus at lock acquisition there is no added cost over the existing code - unlike the previous bless implementation. The added enrollment cost is only borne by locks in the classes that are being blessed (in the case of ZFS sx and lockmgr). The added overhead of enrollment for participating lock types is an extra string compare for each lock that is in a white listed pair. The intent is that with WITNESS enabled, we can incrementally determine how to give WITNESS the context it needs for each lock pair. And even if we aren't reporting reorders, enrolling witness locks mean that we can now inspect them in ddb when we get a lockup. Obviously the real solution is to fix the opensolaris shim layer to provide the context / naming that WITNESS needs to provide accurate LORs. Nonetheless, this prevents regressions on new locks in the present. Note that there appears to still be an issue in that the '"zfsvfs->z_hold_mtx[i]"(sx) and "zfs"(lockmgr)!' is still reported.

In spite of never being heavily involved with ZFS I personally have fixed 3 deadlocks in it over the years. A recent deadlock during testing by a customer caused a major setback for FreeBSD. Thus I'd very much like to move towards leveraging WITNESS. It currently isn't possible to use WITNESS for much of ZFS because the lock name doesn't provide WITNESS with enough context to distinguish between what are in fact two unique locking orders. This change will, at _enrollment_ time mark a lock pair as "blessed". Thus at lock acquisition there is no added cost over the existing code - unlike the previous bless implementation. The added enrollment cost is only borne by locks in the classes that are being blessed (in the case of ZFS sx and lockmgr). The added overhead of enrollment for participating lock types is an extra string compare for each lock that is in a white listed pair. The intent is that with WITNESS enabled, we can incrementally determine how to give WITNESS the context it needs for each lock pair. And even if we aren't reporting reorders, enrolling witness locks mean that we can now inspect them in ddb when we get a lockup. Obviously the real solution is to fix the opensolaris shim layer to provide the context / naming that WITNESS needs to provide accurate LORs. Nonetheless, this prevents regressions on new locks in the present. Note that there appears to still be an issue in that the '"zfsvfs->z_hold_mtx[i]"(sx) and "zfs"(lockmgr)!' is still reported.