Re: ffs_balloc_ufs1 error handling

[Antti Kantee <pooka%cs.hut.fi@localhost>]

On Sun Dec 07 2008 at 21:05:20 +0100, Joerg Sonnenberger wrote:
> Hi all,
> 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.
> 
> Does anyone object the attached patch to conditionalize this logic on
> softdep?

At least in its current form obviously yes.

1) did you even read the block?  Why don't you propose removing the
   softdep conditionals from inside of it?
2) you do not state why you want to do this
3) you do not explain why the same hypothetical situation would not be
   valid for ufs2
4) you do not state where unwinding is done now

In other words, your change might be correct, but I can't sure of that
without going on a meditational crusade.  Since you are proposing
the change, *you* should do the work and provide all the necessary
information.

One situation where block allocation can fail even without the file
system being full is with B_CONTIG.  Does that case still work?

> Index: ffs_balloc.c
> ===================================================================
> RCS file: /home/joerg/repo/netbsd/src/sys/ufs/ffs/ffs_balloc.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 ffs_balloc.c
> --- ffs_balloc.c      31 Jul 2008 05:38:06 -0000      1.51
> +++ ffs_balloc.c      7 Dec 2008 19:59:55 -0000
> @@ -468,7 +468,7 @@ fail:
>        * have to deallocate any indirect blocks that we have allocated.
>        */
>  
> -     if (unwindidx >= 0) {
> +     if (DOINGSOFTDEP(vp) && unwindidx >= 0) {
>  
>               /*
>                * First write out any buffers we've created to resolve their