On Tue, May 01, 2012 at 08:16:59AM -0500, Mark Tinguely wrote:
> 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.
>
> 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.
I thought about doing that, but then the code gets harder to follow.
e.g. why do we use bp->b_map.b_bn in some places, and
bp->b_maps[0].b_bn in others when most of the time they both point
to the same thing? It just maps it too easy to
misunderstand/misuse/confuse the difference between the buffer
identifiers and the real IO block addresses. I prefer the clarity of
purpose that retaining b_bn/b_length gives, even though there is a
slight increase in memory usage....
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
