On Sun, 2011-09-18 at 16:41 -0400, Christoph Hellwig wrote:
> plain text document attachment (xfs-bmalloca-shrink)
> Minimise the stack overhead of the remaining stack variables and
> structures placed on the stack by packing them without holes. pahole
> is used to optimise allocation args structures, stack variables are
> done manually.
>
> [hch: various updates while forward porting the changes]
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
I don't object to it, but I do comment on something
below that I think is not an improvement. I'll
take this as-is anyway, unless you care to re-submit
it.
Reviewed-by: Alex Elder <aelder@xxxxxxx>
. . .
> Index: xfs/fs/xfs/xfs_bmap.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_bmap.c 2011-09-11 08:47:11.743121736 -0400
> +++ xfs/fs/xfs/xfs_bmap.c 2011-09-11 09:05:37.706448434 -0400
. . .
> @@ -4899,19 +4886,21 @@ xfs_bmapi_write(
> bma.userdata = 0;
> bma.flist = flist;
> bma.firstblock = firstblock;
> + bma.eof = n;
> + bma.conv = !!(flags & XFS_BMAPI_CONVERT);
>
> + n = 0;
> + end = bno + len;
> + obno = bno;
> while (bno < end && n < *nmap) {
> - inhole = eof || bma.got.br_startoff > bno;
> - wasdelay = !inhole && isnullstartblock(bma.got.br_startblock);
> -
> /*
> - * First, deal with the hole before the allocated space
> - * that we found, if any.
> + * If we are past EOF, in a hole in a delayed allocation call
> + * the allocator to get us a real allocation first.
> */
> - if (inhole || wasdelay) {
> - bma.eof = eof;
> - bma.conv = !!(flags & XFS_BMAPI_CONVERT);
> - bma.wasdel = wasdelay;
> + if (bma.eof || bma.got.br_startoff > bno ||
> + isnullstartblock(bma.got.br_startblock)) {
> + bma.wasdel = isnullstartblock(bma.got.br_startblock) &&
> + !bma.eof && bma.got.br_startoff <= bno;
I think this detracts from readability and simplicity
rather than adding to it. I understand the desire to
get rid of the local variables, and I do follow what
each piece of this conditional means, but "inhole"
and "wasdelay" were actually quite nice shorthands
for what that logic represents.
> bma.length = len;
> bma.offset = bno;
>
. . .
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
