Re: [PATCH 22/23] xfs: add pre-write metadata buffer verifier callbacks

Christoph Hellwig
2012-10-13T16:00:39Z

> +xfs_agfl_write_verify(
> + struct xfs_buf *bp)
> +{
> + xfs_agfl_verify(bp);
> +}
> +
> +void
> +xfs_agfl_read_verify(
> + struct xfs_buf *bp)
> +{
> + xfs_agfl_verify(bp);
> + bp->b_pre_io = xfs_agfl_write_verify;
> bp->b_iodone = NULL;
> xfs_buf_ioend(bp, 0);

I have to say I hate the way the API works for the buffer callbacks
even more now that I see the write side. I know you're a bit annoyed
about churn from review requests, but I'd really prefer if this
was redone.

Two ways to do this nicer come to mind:

- Have one commone b_verify callback, which gets a bool for_write
argument to key of differences. The xfs_buf_ioend call for reads
remains in the caller, b_iodone is not touched at all. This will
remove the boilerplate code a lot in the current version.
- Expecting to have some more difference between the read and the
write side when we actually do the crcs from my work on the previous
iteration of it it might make sense to have two callbacks, but I'd
again prefer to not overload b_iodone. Maybe just pass a:

struct xfs_buf_ops {
int (*verify_read)(struct xfs_buf *);
int (*verify_write)(struct xfs_buf *);
};

to all the buffer read/write API calls, and let that deal with it.