Re: CVS commit: src/sys

[Jaromír Doleček <jaromir.dolecek%gmail.com@localhost>]

2016-11-07 10:25 GMT+01:00 J. Hannken-Illjes <hannken%eis.cs.tu-bs.de@localhost>:
> 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.

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.

Anyway, IMO eventual change to allow fsck to DTRT after crash in
ffs_truncate() should go separately from the crash fix.

Jaromir