Date: Sat, 1 Oct 2016 18:40:31 +0200 From: Jaromír Dole ek <jaromir.dole...@gmail.com>
> Thanks for taking a shot at this! But I think it needs a little more > time for review -- certainly I can't digest it in the 24 hours you're > giving. Sure. OK, thanks! I'll try to find some time to review this more carefully in the next few days. For now, one comment: > From a quick glance at the patch, I see one bug immediately in > vfs_wapbl.c that must have been introduced in a recent change: > pool_get(PR_WAITOK) is forbidden while holding a mutex, but > wapbl_register_deallocation does just that. I'll recheck this, thank you. I run it under rump only, I think I had it compiled LOCKDEBUG, so should trigger the same assertions as LOCKDEBUG kernel. But apparently not. Since wl->wl_mtx is an adaptive lock, not a spin lock, nothing automatically detects this. But in general, with minor exceptions, one is not supposed to sleep while holding an adaptive lock. (Exception example: waiting for a cheap cross-call to complete in pserialize_perform(9).) It's also suboptimal that we sleep while holding rwlocks for vnode locks, since rw_enter is uninterruptable, so if a wait inside the kernel hangs with a vnode lock held, anyone else trying to examine that vnode will hang uninterruptably too. But it will take some effort to undo that state of affairs by teaching all callers of vn_lock to accept failure. The same goes for the the long-term file system transaction lock wl->wl_rwlock. However, suboptimality of sleeping with those long-term locks aside, certainly one should not sleep while holding the short-term wl->wl_mtx.