Re: [PATCH 04/12] xfs: Clean up XFS_BLI_* flag namespace

Alex Elder
2010-05-19T19:07:03Z

On Tue, 2010-05-18 at 09:24 +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Clean up the buffer log format (XFS_BLI_*) flags because they have a
> polluted namespace. They XFS_BLI_ prefix is used for both in-memory
> and on-disk flag feilds, but have overlapping values for different
> flags. Rename the buffer log format flags to use the XFS_BLF_*
> prefix to avoid confusing them with the in-memory XFS_BLI_* prefixed
> flags.

This is a good change. That XFS_BLI_INODE_BUF thing was nasty.

One little comment below, but otherwise looks good.

> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/linux-2.6/xfs_super.c | 2 +-
> fs/xfs/quota/xfs_dquot.c | 6 ++--

. . .

> diff --git a/fs/xfs/xfs_buf_item.h b/fs/xfs/xfs_buf_item.h
> index df44545..8cbb82b 100644
> --- a/fs/xfs/xfs_buf_item.h
> +++ b/fs/xfs/xfs_buf_item.h
> @@ -41,22 +41,22 @@ typedef struct xfs_buf_log_format {
> * This flag indicates that the buffer contains on disk inodes
> * and requires special recovery handling.
> */
> -#define XFS_BLI_INODE_BUF 0x1
> +#define XFS_BLF_INODE_BUF 0x1
> /*
> * This flag indicates that the buffer should not be replayed
> * during recovery because its blocks are being freed.
> */
> -#define XFS_BLI_CANCEL 0x2
> +#define XFS_BLF_CANCEL 0x2
> /*
> * This flag indicates that the buffer contains on disk
> * user or group dquots and may require special recovery handling.
> */
> -#define XFS_BLI_UDQUOT_BUF 0x4
> -#define XFS_BLI_PDQUOT_BUF 0x8
> -#define XFS_BLI_GDQUOT_BUF 0x10
> +#define XFS_BLF_UDQUOT_BUF 0x4
> +#define XFS_BLF_PDQUOT_BUF 0x8
> +#define XFS_BLF_GDQUOT_BUF 0x10

I know this isn't part of your change, but I think
a small comment here would make it more obvious
that the following are numeric values, distinct
from the bits defined just above (and therefore
not part of the same name space for flags).

Not a big deal--I'll make a note of it and may
do it myself someday.

> -#define XFS_BLI_CHUNK 128
> -#define XFS_BLI_SHIFT 7
> +#define XFS_BLF_CHUNK 128
> +#define XFS_BLF_SHIFT 7