Re: CVS commit: src/sys

["J. Hannken-Illjes" <hannken%eis.cs.tu-bs.de@localhost>]

> On 07 Nov 2016, at 11:54, Jaromír Doleček <jaromir.dolecek%gmail.com@localhost> wrote:
> 
> 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.

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 - hannken%eis.cs.tu-bs.de@localhost - TU Braunschweig (Germany)