Re: Review: Be smarter about handling ENOSPC during writeback

David Chinner
2007-06-08T07:33:51Z

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.