Re: CVS commit: src/sys

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

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

Good point. The same is however true for ALL the calls in the chain,
so then we should have to really use FORCE everywhere.

One solution is to always bwrite() or bdwrite() back even fully zeroed
block for wapbl case instead of brelse(BC_INVAL). I think that for
fsck to reliably recover from crash within truncate, this might
actually be needed also for !wapbl case.

Another way how to solve this would be to try to register the
deallocation and on error bail out, before the diving. It would
require cancelling the registration if the diving call returns EAGAIN
however.

> 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;

No, I mean the copy logic and big blocks with condition on
ip->i_ump->um_mountp->mnt_wapbl.

> I don't understand ... do you want to split into two diffs?

I prefer to split commits into incremental changes if reasonably
possible, so it's easier to review and revise. That's all. That's why
I prefer the fix for immediate corruption to go separately.

Jaromir