Re: Review: Be smarter about handling ENOSPC during writeback

[David Chinner <dgc@sgi.com>]

On Fri, Jun 08, 2007 at 03:28:14PM +1000, Timothy Shimmin wrote:
> Hi Dave,
> 
> Putting the xfs_reserve_blocks discussion to the side....
> (discussed separately)

*nod*

BTW, did you try that patch I sent?

> > fs/xfs/xfs_fsops.c |   10 +++++++---
> 
> * allow xfs_reserve_blocks() to handle a null outval so that
>  we can call xfs_reserve_blocks other than thru ioctl,
>  where we don't care about outval
> * xfs_growfs_data_private() or's in XFS_TRANS_RESERVE like we do for root 
> EAs
>  -> allow growfs transaction to dip in to reserve space

Yes, and so now you can grow a completely full filesystem :)

> > fs/xfs/xfs_mount.c |   37 +++++++++++++++++++++++++++++++++++--
> 
> * xfs_mountfs(): cleanup - restrict a variable (ret64) to the block its 
> used in
> * xfs_mountfs(): do our xfs_reserve_blocks() for what we think we'll need
>  - pass NULL for 2nd param to it as we don't care (why we changed 
>  xfs_fsops.c)
>  - defaults to min(1024 FSBs, 5% dblocks)
>    -> not sure how one would choose this but it sounds big enough

It's a SWAG. I think it's sufficient to begin with. If it proves to
be a problem, then we can change it later....

> * xfs_unmountfs(): xfs_reserve_blocks of zero and so restoring the sb free 
> counter
> 
> Q: so I guess, for DMF systems which presumably turn this stuff on using 
> the ioctl;

yeah - the rope is long enough ;)

>   we should tell them to stop doing this - they could stuff us up by 
>   overriding it
>   maybe and they don't need to.

All they need to do is check first before setting a new value...

> > fs/xfs/xfs_iomap.c |   22 ++++++++--------------
> 
> * some whitespace cleanup
> xfs_iomap_write_allocate():
> * delalloc extent conversion - mark transaction for reserved blocks space
> * don't handle ENOSPC here, as we shouldn't get it now I presume

We still can, just much more unlikely. I need to do another set of
patches for ENOSPC notification but I haven't had a chance yet.

> xfs_iomap_write_unwritten
> * unwritten extent conversion - mark trans for reserved blocks

Ditto.

> Seems simple enough :)

It's just one of a few to begin with?

> Will we get questions from people about reduced space from df? :)

If we do, I think you just volunteered to write the FAQ entry ;)

Thanks for the review.

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group