Re: [PATCH 06/10 v2] xfs: format log items write directly into the linear CIL buffer

Dave Chinner
2013-12-12T00:06:43Z

On Wed, Dec 11, 2013 at 04:03:41AM -0800, Christoph Hellwig wrote:
> Instead of setting up pointers to memory locations in iop_format which then
> get copied into the CIL linear buffer after return move the copy into
> the individual inode items. This avoids the need to always have a memory
> block in the exact same layout that gets written into the log around, and
> allow the log items to be much more flexible in their in-memory layouts.
>
> The only caveat is that we need to properly align the data for each
> iovec so that don't have structures misaligned in subsequent iovecs.

.....

> diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
> index 384c6c4..65e054a 100644
> --- a/fs/xfs/xfs_log.h
> +++ b/fs/xfs/xfs_log.h
> @@ -31,18 +31,49 @@ struct xfs_log_vec {
> #define XFS_LOG_VEC_ORDERED (-1)
>
> static inline void *
> -xlog_copy_iovec(struct xfs_log_iovec **vecp, uint type, void *data, int len)
> +xlog_prepare_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec **vecp,
> + uint type)
> {
> struct xfs_log_iovec *vec = *vecp;
>
> + if (vec) {
> + ASSERT(vec - lv->lv_iovecp < lv->lv_niovecs);
> + vec++;
> + } else {
> + vec = &lv->lv_iovecp[0];
> + }
> +
> vec->i_type = type;
> - vec->i_addr = data;
> - vec->i_len = len;
> + vec->i_addr = lv->lv_buf + lv->lv_buf_len;
>
> - *vecp = vec + 1;
> + *vecp = vec;
> return vec->i_addr;

Can you add an assert here like this:

ASSERT(IS_ALIGNED(vec->i_addr, sizeof(uint64_t));

So we catch any situation where the alignment ends up wrong?

Otherwise, it looks good, so consider it:

Reviewed-by: Dave Chinner <dchinner@redhat.com>

--
Dave Chinner
david@fromorbit.com