xfs
[Top] [All Lists]

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

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 1/7] xfs: let iop_format write directly into the linear buffer
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Mon, 25 Nov 2013 20:15:27 +1100
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20131123151533.726941044@xxxxxxxxxxxxxxxxxxxxxx>
References: <20131123151151.716201348@xxxxxxxxxxxxxxxxxxxxxx> <20131123151533.726941044@xxxxxxxxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Sat, Nov 23, 2013 at 07:11:52AM -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.

There's a lot of fair intricate manipulations in this patch that
could be separated and hence easier to understand. I think you could
break this up in several patches:

        - xfs_buf_item_straddle() factoring
        - removal of the special cases for no endian swapping around
          xfs_inode_item_format_extents()
        - a separate patch to introduce xlog_first/next/last_iovec(),
          as I had to find those first to understand how the new
          code worked
        - 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;
}

        - and finally all the conversions. The addition of an
          xlog_copy_vec() helper will make the final patch that does
          the switchover much smaller and easier to verify....


Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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