Re: ffs_balloc_ufs1 error handling

[Joerg Sonnenberger <joerg%britannica.bec.de@localhost>]

On Sun, Dec 07, 2008 at 10:11:14PM +0000, Andrew Doran wrote:
> On Sun, Dec 07, 2008 at 09:05:20PM +0100, Joerg Sonnenberger wrote:
> 
> > from my understand of the code and the history of ffs_balloc_ufs1,
> > the first if after fail: is meant to flush the vnode content to disk to
> > break up dependencies. From FreeBSD's history, this is the cheap
> > approach to handle erroring out for softdep filesystems when they become
> > full.
> 
> 1. It doesn't work for softdep, it hangs.

Huh? For softdep the check is invariant.

> 2. It does what the comment above it says.

Assuming you talk about the whole if (unwindidx >= 0) block:
No, it doesn't. The deallocation of indirect blocks happens in the for
loop after the if block.

> 3. It also attempts to invalidate blocks associated with the vnode.
>    It is not clear to me at a glance if that works as intended.

Same assumption. This was added for the sake of softdep to simplify the
dependency tracking. It was originally a VOP_FLUSH. I don't see any
point in doing this for the !softdep cache -- it just forces more sync
io than necessary.

> Yes, it will corrupt full file systems if they are not using softdep.

What base do you have for this conclusion? Testing does not reveal any
corruption. As I said -- until the softdep merge nothing at all was done
here, just freeing of the indirect blocks.

Joerg