I've incorporated the mutex fix, here is the final patch relative to trunk. I'd like to commit this sometime next week. Jaromir 2016-10-01 19:00 GMT+02:00 Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost>: > Date: Sat, 1 Oct 2016 18:40:31 +0200 > From: Jaromír Dole ek <jaromir.dolecek%gmail.com@localhost> > > > 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.
Attachment:
wapbl_fix_dealloc_2.patch
Description: Binary data