> On 07 Nov 2016, at 11:54, Jaromír Doleček <jaromir.dole...@gmail.com> wrote: > > 2016-11-07 10:25 GMT+01:00 J. Hannken-Illjes <hann...@eis.cs.tu-bs.de>: >> The first part should not be necessary. After the loop we should have >> "i == last" -- from a quick look "i < last" is impossible. > > Yes, I know - it's just to make the diff vs 1.117 smaller and hence > easier to review. I want to commit this separately. > >> The second part is right and responsible for most of the panics. > > Right. > >> Your patch still doesn't address my second observation, if the >> machine crashs inside ffs_truncate we leave the file system in a >> state fsck can't handle. The blocks we freed get attached to other >> nodes before they get cleared from our on-disk copy leading to >> duplicate blocks. > > That patch is not really very ideal. > > One is UFS_WAPBL_REGISTER_DEALLOCATION_FORCE(), which should only be > used as desperate measure.
It gets used when a block of block pointers has been deallocated and brelsed, that is NOT written to disk. If we allow the deallocation of this block to fail and ufs_truncate_retry() runs ffs_truncate again it will read the block from disk and free already freed blocks. > Second is that the patch adds IMO too much difference for wapbl and > !wapbl case, to already quite complicated function. Also it's slightly > confusing that the !wapbl change now doesn't support failure in the > middle any more - it will just leave the block inconsistent. I need to > think a little about this, maybe there is some better way. The wapbl and !wapbl cases ARE different. You mean this: error = ffs_indirtrunc(ip, nlbn, FFS_FSBTODB(fs, nb), (daddr_t)-1, level - 1, countp); - if (error) - goto out; + if (error == EAGAIN) + goto out; + else if (error && !allerror) + allerror = error; > Anyway, IMO eventual change to allow fsck to DTRT after crash in > ffs_truncate() should go separately from the crash fix. I don't understand ... do you want to split into two diffs? -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)