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

[Dave Chinner <david@fromorbit.com>]

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

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs