xfs
[Top] [All Lists]

Re: [PATCH 2/2] xfs: rearrange some code in xfs_bmap for better locality

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 2/2] xfs: rearrange some code in xfs_bmap for better locality
From: Mark Tinguely <tinguely@xxxxxxx>
Date: Mon, 11 Feb 2013 16:43:10 -0600
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1360559102-20432-3-git-send-email-david@xxxxxxxxxxxxx>
References: <1360559102-20432-1-git-send-email-david@xxxxxxxxxxxxx> <1360559102-20432-3-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:9.0) Gecko/20120122 Thunderbird/9.0
On 02/10/13 23:05, Dave Chinner wrote:
From: Dave Chinner<dchinner@xxxxxxxxxx>

xfs_bmap.c is a big file, and some of the related code is spread all
throughout the file requiring function prototypes for static
function and jumping all through the file to follow a single call
path. Rearrange the code so that:

        a) related functionality is grouped together; and
        b) functions are grouped in call dependency order

While the diffstat is large, there are no code changes in the patch;
it is just moving the functionality around and removing the function
prototypes at the top of the file. The resulting layout of the code
is as follows (top of file to bottom):

        - miscellaneous helper functions
        - extent tree block counting routines
        - debug/sanity checking code
        - bmap free list manipulation functions
        - inode fork format manipulation functions
        - internal/external extent tree seach functions
        - extent tree manipulation functions used during allocation
        - functions used during extent read/allocate/removal
          operations (i.e. xfs_bmapi_write, xfs_bmapi_read,
          xfs_bunmapi and xfs_getbmap)

This means that following logic paths through the bmapi code is much
simpler - most of the code relevant to a specific operation is now
clustered together rather than spread all over the file....

Signed-off-by: Dave Chinner<dchinner@xxxxxxxxxx>
---
  fs/xfs/xfs_bmap.c |10659 ++++++++++++++++++++++++++---------------------------
  1 file changed, 5261 insertions(+), 5398 deletions(-)

The organization looks good to me. If the file is getting that many
changes, isn't it time to fix all those spaces in the file? (looks like there are over 2500 unnecessary spaces in place of tabs).

--Mark.

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