xfs
[Top] [All Lists]

Re: [PATCH 1/7] xfs: let iop_format write directly into the linear buffe

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 1/7] xfs: let iop_format write directly into the linear buffer
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Mon, 25 Nov 2013 05:37:55 -0800
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20131125091527.GD8803@dastard>
References: <20131123151151.716201348@xxxxxxxxxxxxxxxxxxxxxx> <20131123151533.726941044@xxxxxxxxxxxxxxxxxxxxxx> <20131125091527.GD8803@dastard>
User-agent: Mutt/1.5.21 (2010-09-15)
On Mon, Nov 25, 2013 at 08:15:27PM +1100, Dave Chinner wrote:
>       - xfs_buf_item_straddle() factoring

This could probably be split out.

>       - removal of the special cases for no endian swapping around
>         xfs_inode_item_format_extents()

This can only be done after we have the new iop_format, or rather must
be because the old way doesn't work really well.  I'll have to see if
it can be a separate one, but it would have to be after the actual
iop_format change.

>       - a separate patch to introduce xlog_first/next/last_iovec(),
>         as I had to find those first to understand how the new
>         code worked

What's the point of a separate patch if we don't make use of it yet?

>       - a new xlog_copy_iovec() function instead of open coding
>         the same 3 lines of code in 14 different places:
> 
> static inline void
> xlog_copy_iovec(
>       struct xfs_log_iovec    *vec,
>       void                    *src,
>       int                     len,
>       int                     type)
> {
>       memcpy(vec->i_addr, src_ptr, len);
>       vec->i_len = len;
>       vec->i_type = type;
> }

I went forth and back between having this a couple of times and found
having the helper more confusing than not.  If there's enough strong
opinion to have it I can add it back.

<Prev in Thread] Current Thread [Next in Thread>