tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: ffs_balloc_ufs1 error handling

On Mon, Dec 08, 2008 at 01:50:25PM +0100, Joerg Sonnenberger wrote:

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

Sorry but you're wrong on all 4 counts. Please read the code more closely.
With your patch the deallocated indirect blocks are left dangling, producing
a corrupted file system.


Home | Main Index | Thread Index | Old Index