> On 10 Nov 2016, at 21:56, Jaromir Dolecek <jdole...@netbsd.org> wrote: > > Module Name: src > Committed By: jdolecek > Date: Thu Nov 10 20:56:32 UTC 2016 > > Modified Files: > src/sys/kern: vfs_wapbl.c > src/sys/sys: wapbl.h > src/sys/ufs/ffs: ffs_inode.c ffs_wapbl.c > src/sys/ufs/ufs: ufs_wapbl.h > > Log Message: > during truncate with wapbl, register deallocation for upper indirect block > before recursing into lower blocks, to make sure that it will be removed after > all its referenced blocks are removed > > fixes 'ffs_blkfree_common: freeing free block' panic triggered by > ufs_truncate_retry() when just the upper indirect block registration failed, > code tried to free the lower blocks again after wapbl flush > > problem found by hannken@, thank you > > > To generate a diff of this commit: > cvs rdiff -u -r1.85 -r1.86 src/sys/kern/vfs_wapbl.c
- SIMPLEQ_HEAD(, wapbl_dealloc) wl_dealloclist; /* lm: list head */ + TAILQ_HEAD(, wapbl_dealloc) wl_dealloclist; /* lm: list head */ This should have been committed on its own. It makes reading the diff quite difficult. Why do we need a TAILQ here, the number of deletions after the first element is nearly zero and the list is quite short. +wapbl_deallocation_free(struct wapbl *wl, struct wapbl_dealloc *wd, + bool locked) Better to let the caller always take/release the lock. +wapbl_register_deallocation(struct wapbl *wl, daddr_t blk, int len, bool force, + void **cookiep) + if (cookiep) + *cookiep = wd; +wapbl_unregister_deallocation(struct wapbl *wl, void *cookie) Returning a pointer to an arbitrary list element and using it later is bad design. Would be better to define as: wapbl_unregister_deallocation(struct wapbl *wl, daddr_t blk, int len) { struct wapbl_dealloc *wd; mutex_enter(&wl->wl_mtx); TAILQ_FOREACH(wd, &wl->wl_dealloclist, wd_entries) { if (wd->wd_blkno == blk && wd->wd_len == len) break; } KASSERT(wd != NULL); wapbl_deallocation_free(wl, wd); mutex_exit(&wl->wl_mtx); } > cvs rdiff -u -r1.19 -r1.20 src/sys/sys/wapbl.h > cvs rdiff -u -r1.121 -r1.122 src/sys/ufs/ffs/ffs_inode.c > cvs rdiff -u -r1.35 -r1.36 src/sys/ufs/ffs/ffs_wapbl.c > cvs rdiff -u -r1.12 -r1.13 src/sys/ufs/ufs/ufs_wapbl.h -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)