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

[Dave Chinner <david@fromorbit.com>]

On Mon, Feb 11, 2013 at 04:43:10PM -0600, Mark Tinguely wrote:
> On 02/10/13 23:05, Dave Chinner wrote:
> >From: Dave Chinner<dchinner@redhat.com>
> >
> >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@redhat.com>
> >---
> >  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).

My "misplaced whitepsace" syntax highlighting doesn't trip on any,
so I didn't notice it. i.e the whitespace is entirely spaces or
entirely tabs and not tabs/spaces in combination.

A quick search shows that they are mostly in the variable
declaration comment formatting - I tend simply to remove those
comments as I modify the surrounding code because, IMO, they
are almost entirely redundant as we have well named structures
and variables.

So, yeah, maybe a followup patch that kills the comments and fixes
the whitespace damage is in order.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs