xfs
[Top] [All Lists]

Re: [PATCH 02/10] xfs: separate buffer indexing from block map

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 02/10] xfs: separate buffer indexing from block map
From: Mark Tinguely <tinguely@xxxxxxx>
Date: Tue, 01 May 2012 08:16:59 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20120430232410.GP7015@dastard>
References: <1335249220-22274-1-git-send-email-david@xxxxxxxxxxxxx> <1335249220-22274-3-git-send-email-david@xxxxxxxxxxxxx> <4F9EE7EC.4030203@xxxxxxx> <20120430232410.GP7015@dastard>
User-agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:9.0) Gecko/20120122 Thunderbird/9.0
On 04/30/12 18:24, Dave Chinner wrote:
On Mon, Apr 30, 2012 at 02:28:44PM -0500, Mark Tinguely wrote:
On 04/24/12 01:33, Dave Chinner wrote:
From: Dave Chinner<dchinner@xxxxxxxxxx>

To support discontiguous buffers in the buffer cache, we need to
separate the cache index variables from the I/O map. While this is
currently a 1:1 mapping, discontiguous buffer support will break
this relationship.

However, for caching purposes, we can still treat them the same as a
contiguous buffer - the block number of the first block and the
length of the buffer - as that is still a unique representation.
Also, the only way we will ever access the discontiguous regions of
buffers is via bulding the complete buffer in the first place, so
using the initial block number and entire buffer length is a sane
way to index the buffers.

Add a block mapping vector construct to the xfs_buf and use it in
the places where we are doing IO instead of the current
b_bn/b_length variables.

Signed-off-by: Dave Chinner<dchinner@xxxxxxxxxx>
...
+struct xfs_buf_map {
+       xfs_daddr_t             bm_bn;  /* block number for I/O */
+       int                     bm_len; /* size of I/O */
+};
+
  typedef struct xfs_buf {
        /*
         * first cacheline holds all the fields needed for an uncontended cache
@@ -107,7 +114,7 @@ typedef struct xfs_buf {
         * fast-path on locking.
         */
        struct rb_node          b_rbnode;       /* rbtree node */
-       xfs_daddr_t             b_bn;           /* block number for I/O */
+       xfs_daddr_t             b_bn;           /* block number of buffer */
        int                     b_length;       /* size of buffer in BBs */

Looks good.
Do you plan to eventually remove b_bn and b_length from xfs_buf?

No. b_bn is a fast way of identifying unique buffers for cache
lookups and is located in the same cacheline as the tree node so we
don't take an extra cache miss on every buffer we traverse during
tree walks in _xfs_buf_find(). Also, b_length is used so often it is
much cleaner to keep it around than it s to iterate over all the
maps to calculate it every time it is needed.

Cheers,

Dave.

Thanks for the reply. I was thinking that maybe the "inline" map could do both of those things. It is not used if there are multiple maps and is the same value as the original variables if there is only one map.

--Mark Tinguely.

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