Re: [PATCH 3/3] xfs: simplify validation of the unwritten extent bit

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Thu, Apr 20, 2017 at 04:38:57PM +0200, Christoph Hellwig wrote:
> [can you trim the quote?  Makes reading it and properly quoting it
>  so much easier..]
> 
> > > +static inline bool xfs_bmbt_validate_extent(struct xfs_mount *mp, int whichfork,
> > > +		struct xfs_bmbt_rec_host *ep)
> > 
> > Would be nice to have this function formatted the same way as most of
> > the rest of the xfs functions, even if it is static inline...
> 
> It's actually the usual style for our inlines.  But if you prefer it
> differently I can do that.

Nah, don't worry about it.

Though I do wonder why static inlines get different treatment; it's
rather nice to be able to search for ^xfs_function and find its
definition.

> > Wouldn't this be better off in xfs_iflush_int (like the inline dir
> > verifier) since we could prevent bad metadata from hitting the disk?
> > Rather than this, which doesn't do anything on non-debug kernels.
> 
> xfs_iflush_int actually calls xfs_iflush_fork which calls
> xfs_iextents_copy, so it's in the right spot already.  Converting it
> from an assert to an error would have to go through these layers
> that don't currently expect errors.  Note that we also call
> xfs_iextents_copy from xfs_inode_item_format_data_fork /
> xfs_inode_item_format_attr_fork, which are called earlier than
> xfs_iflush_int, where error propagation is even worse.

Fair enough.  The rest looks ok, so I'll go run it through testing.

--D