From: Christoph Hellwig <hch@infradead.org>
To: xfs@oss.sgi.com
Subject: Re: [PATCH 12/12] repair: cleanup inode record macros
Date: Wed, 11 Jan 2012 06:30:20 -0500 [thread overview]
Message-ID: <20120111113020.GD10932@infradead.org> (raw)
In-Reply-To: <20111202174743.697687824@bombadil.infradead.org>
ping?
On Fri, Dec 02, 2011 at 12:46:31PM -0500, Christoph Hellwig wrote:
> Remove indirections in the inode record bit manipulation macros and flatten
> them to a single level of inlines. Also use a common IREC_MASK define
> instead of duplicating it for every bitmask.
>
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> Index: xfsprogs-dev/repair/incore.h
> ===================================================================
> --- xfsprogs-dev.orig/repair/incore.h 2011-12-02 11:04:59.000000000 +0000
> +++ xfsprogs-dev/repair/incore.h 2011-12-02 11:17:31.000000000 +0000
> @@ -289,6 +289,9 @@ typedef struct ino_tree_node {
> } ino_un;
> } ino_tree_node_t;
>
> +#define INOS_PER_IREC (sizeof(__uint64_t) * NBBY)
> +#define IREC_MASK(i) ((__uint64_t)1 << (i))
> +
> typedef struct nlink_ops {
> const int nlink_size;
> void (*disk_nlink_set)(ino_tree_node_t *, int, __uint32_t);
> @@ -299,7 +302,6 @@ typedef struct nlink_ops {
> } nlink_ops_t;
>
>
> -#define INOS_PER_IREC (sizeof(__uint64_t) * NBBY)
> void add_ino_ex_data(xfs_mount_t *mp);
>
> /*
> @@ -381,92 +383,73 @@ void clear_uncertain_ino_cache(xfs_agn
> ((ino_tree_node_t *) ((ino_node_ptr)->avl_node.avl_forw))
>
> /*
> - * Bit manipulations for processed field
> + * Has an inode been processed for phase 6 (reference count checking)?
> + *
> + * add_inode_refchecked() is set on an inode when it gets traversed
> + * during the reference count phase (6). It's set so that if the inode
> + * is a directory, it's traversed (and it's links counted) only once.
> */
> -#define XFS_INOPROC_MASK(i) ((__uint64_t)1 << (i))
> -#define XFS_INOPROC_MASKN(i,n) ((__uint64_t)((1 << (n)) - 1) << (i))
> +static inline void add_inode_refchecked(struct ino_tree_node *irec, int offset)
> +{
> + irec->ino_un.ex_data->ino_processed |= IREC_MASK(offset);
> +}
>
> -#define XFS_INOPROC_IS_PROC(rp, i) \
> - (((rp)->ino_un.ex_data->ino_processed & XFS_INOPROC_MASK((i))) == 0LL \
> - ? 0 : 1)
> -#define XFS_INOPROC_SET_PROC(rp, i) \
> - ((rp)->ino_un.ex_data->ino_processed |= XFS_INOPROC_MASK((i)))
> -/*
> -#define XFS_INOPROC_CLR_PROC(rp, i) \
> - ((rp)->ino_un.ex_data->ino_processed &= ~XFS_INOPROC_MASK((i)))
> -*/
> +static inline int is_inode_refchecked(struct ino_tree_node *irec, int offset)
> +{
> + return (irec->ino_un.ex_data->ino_processed & IREC_MASK(offset)) != 0;
> +}
>
> /*
> - * same for ir_confirmed.
> + * set/test is inode known to be valid (although perhaps corrupt)
> */
> -#define XFS_INOCF_MASK(i) ((__uint64_t)1 << (i))
> -#define XFS_INOCF_MASKN(i,n) ((__uint64_t)((1 << (n)) - 1) << (i))
> -
> -#define XFS_INOCF_IS_CF(rp, i) \
> - (((rp)->ino_confirmed & XFS_INOCF_MASK((i))) == 0LL \
> - ? 0 : 1)
> -#define XFS_INOCF_SET_CF(rp, i) \
> - ((rp)->ino_confirmed |= XFS_INOCF_MASK((i)))
> -#define XFS_INOCF_CLR_CF(rp, i) \
> - ((rp)->ino_confirmed &= ~XFS_INOCF_MASK((i)))
> +static inline void set_inode_confirmed(struct ino_tree_node *irec, int offset)
> +{
> + irec->ino_confirmed |= IREC_MASK(offset);
> +}
>
> -/*
> - * same for backptr->ino_reached
> - */
> -#define XFS_INO_RCHD_MASK(i) ((__uint64_t)1 << (i))
> +static inline int is_inode_confirmed(struct ino_tree_node *irec, int offset)
> +{
> + return (irec->ino_confirmed & IREC_MASK(offset)) != 0;
> +}
>
> -#define XFS_INO_RCHD_IS_RCHD(rp, i) \
> - (((rp)->ino_un.ex_data->ino_reached & XFS_INO_RCHD_MASK((i))) == 0LL \
> - ? 0 : 1)
> -#define XFS_INO_RCHD_SET_RCHD(rp, i) \
> - ((rp)->ino_un.ex_data->ino_reached |= XFS_INO_RCHD_MASK((i)))
> -#define XFS_INO_RCHD_CLR_RCHD(rp, i) \
> - ((rp)->ino_un.ex_data->ino_reached &= ~XFS_INO_RCHD_MASK((i)))
> /*
> * set/clear/test is inode a directory inode
> */
> -#define XFS_INO_ISADIR_MASK(i) ((__uint64_t)1 << (i))
> -
> -#define inode_isadir(ino_rec, ino_offset) \
> - (((ino_rec)->ino_isa_dir & XFS_INO_ISADIR_MASK((ino_offset))) == 0LL \
> - ? 0 : 1)
> -#define set_inode_isadir(ino_rec, ino_offset) \
> - ((ino_rec)->ino_isa_dir |= XFS_INO_ISADIR_MASK((ino_offset)))
> -#define clear_inode_isadir(ino_rec, ino_offset) \
> - ((ino_rec)->ino_isa_dir &= ~XFS_INO_ISADIR_MASK((ino_offset)))
> -
> -
> -/*
> - * set/clear/test is inode known to be valid (although perhaps corrupt)
> - */
> -#define clear_inode_confirmed(ino_rec, ino_offset) \
> - XFS_INOCF_CLR_CF((ino_rec), (ino_offset))
> +static inline void set_inode_isadir(struct ino_tree_node *irec, int offset)
> +{
> + irec->ino_isa_dir |= IREC_MASK(offset);
> +}
>
> -#define set_inode_confirmed(ino_rec, ino_offset) \
> - XFS_INOCF_SET_CF((ino_rec), (ino_offset))
> +static inline void clear_inode_isadir(struct ino_tree_node *irec, int offset)
> +{
> + irec->ino_isa_dir &= ~IREC_MASK(offset);
> +}
>
> -#define is_inode_confirmed(ino_rec, ino_offset) \
> - XFS_INOCF_IS_CF(ino_rec, ino_offset)
> +static inline int inode_isadir(struct ino_tree_node *irec, int offset)
> +{
> + return (irec->ino_isa_dir & IREC_MASK(offset)) != 0;
> +}
>
> /*
> * set/clear/test is inode free or used
> */
> -#define set_inode_free(ino_rec, ino_offset) \
> - XFS_INOCF_SET_CF((ino_rec), (ino_offset)), \
> - XFS_INOBT_SET_FREE((ino_rec), (ino_offset))
> -
> -#define set_inode_used(ino_rec, ino_offset) \
> - XFS_INOCF_SET_CF((ino_rec), (ino_offset)), \
> - XFS_INOBT_CLR_FREE((ino_rec), (ino_offset))
> +static inline void set_inode_free(struct ino_tree_node *irec, int offset)
> +{
> + set_inode_confirmed(irec, offset);
> + irec->ir_free |= XFS_INOBT_MASK(offset);
>
> -#define XFS_INOBT_IS_FREE(ino_rec, ino_offset) \
> - (((ino_rec)->ir_free & XFS_INOBT_MASK(ino_offset)) != 0)
> +}
>
> -#define is_inode_used(ino_rec, ino_offset) \
> - !XFS_INOBT_IS_FREE((ino_rec), (ino_offset))
> +static inline void set_inode_used(struct ino_tree_node *irec, int offset)
> +{
> + set_inode_confirmed(irec, offset);
> + irec->ir_free &= ~XFS_INOBT_MASK(offset);
> +}
>
> -#define is_inode_free(ino_rec, ino_offset) \
> - XFS_INOBT_IS_FREE((ino_rec), (ino_offset))
> +static inline int is_inode_free(struct ino_tree_node *irec, int offset)
> +{
> + return (irec->ir_free & XFS_INOBT_MASK(offset)) != 0;
> +}
>
> /*
> * add_inode_reached() is set on inode I only if I has been reached
> @@ -478,88 +461,54 @@ void clear_uncertain_ino_cache(xfs_agn
> * an inode that we've counted is removed.
> */
>
> -static inline int
> -is_inode_reached(ino_tree_node_t *ino_rec, int ino_offset)
> -{
> - ASSERT(ino_rec->ino_un.ex_data != NULL);
> - return(XFS_INO_RCHD_IS_RCHD(ino_rec, ino_offset));
> -}
> -
> -static inline void
> -add_inode_reached(ino_tree_node_t *ino_rec, int ino_offset)
> +static inline void add_inode_ref(struct ino_tree_node *irec, int offset)
> {
> - ASSERT(ino_rec->ino_un.ex_data != NULL);
> -
> - (*ino_rec->nlinkops->counted_nlink_inc)(ino_rec, ino_offset);
> - XFS_INO_RCHD_SET_RCHD(ino_rec, ino_offset);
> + ASSERT(irec->ino_un.ex_data != NULL);
>
> - ASSERT(is_inode_reached(ino_rec, ino_offset));
> + irec->nlinkops->counted_nlink_inc(irec, offset);
> }
>
> -static inline void
> -add_inode_ref(ino_tree_node_t *ino_rec, int ino_offset)
> +static inline void drop_inode_ref(struct ino_tree_node *irec, int offset)
> {
> - ASSERT(ino_rec->ino_un.ex_data != NULL);
> + ASSERT(irec->ino_un.ex_data != NULL);
>
> - (*ino_rec->nlinkops->counted_nlink_inc)(ino_rec, ino_offset);
> + if (irec->nlinkops->counted_nlink_dec(irec, offset) == 0)
> + irec->ino_un.ex_data->ino_reached &= ~IREC_MASK(offset);
> }
>
> -static inline void
> -drop_inode_ref(ino_tree_node_t *ino_rec, int ino_offset)
> +static inline __uint32_t num_inode_references(struct ino_tree_node *irec,
> + int offset)
> {
> - ASSERT(ino_rec->ino_un.ex_data != NULL);
> + ASSERT(irec->ino_un.ex_data != NULL);
>
> - if ((*ino_rec->nlinkops->counted_nlink_dec)(ino_rec, ino_offset) == 0)
> - XFS_INO_RCHD_CLR_RCHD(ino_rec, ino_offset);
> + return irec->nlinkops->counted_nlink_get(irec, offset);
> }
>
> -static inline int
> -is_inode_referenced(ino_tree_node_t *ino_rec, int ino_offset)
> +static inline int is_inode_reached(struct ino_tree_node *irec, int offset)
> {
> - ASSERT(ino_rec->ino_un.ex_data != NULL);
> -
> - return (*ino_rec->nlinkops->counted_nlink_get)(ino_rec, ino_offset) > 0;
> + ASSERT(irec->ino_un.ex_data != NULL);
> + return (irec->ino_un.ex_data->ino_reached & IREC_MASK(offset)) != 0;
> }
>
> -static inline __uint32_t
> -num_inode_references(ino_tree_node_t *ino_rec, int ino_offset)
> +static inline void add_inode_reached(struct ino_tree_node *irec, int offset)
> {
> - ASSERT(ino_rec->ino_un.ex_data != NULL);
> -
> - return (*ino_rec->nlinkops->counted_nlink_get)(ino_rec, ino_offset);
> + add_inode_ref(irec, offset);
> + irec->ino_un.ex_data->ino_reached |= IREC_MASK(offset);
> }
>
> -static inline void
> -set_inode_disk_nlinks(ino_tree_node_t *ino_rec, int ino_offset, __uint32_t nlinks)
> +static inline void set_inode_disk_nlinks(struct ino_tree_node *irec, int offset,
> + __uint32_t nlinks)
> {
> - (*ino_rec->nlinkops->disk_nlink_set)(ino_rec, ino_offset, nlinks);
> + irec->nlinkops->disk_nlink_set(irec, offset, nlinks);
> }
>
> -static inline __uint32_t
> -get_inode_disk_nlinks(ino_tree_node_t *ino_rec, int ino_offset)
> +static inline __uint32_t get_inode_disk_nlinks(struct ino_tree_node *irec,
> + int offset)
> {
> - return (*ino_rec->nlinkops->disk_nlink_get)(ino_rec, ino_offset);
> + return irec->nlinkops->disk_nlink_get(irec, offset);
> }
>
> /*
> - * has an inode been processed for phase 6 (reference count checking)?
> - * add_inode_refchecked() is set on an inode when it gets traversed
> - * during the reference count phase (6). It's set so that if the inode
> - * is a directory, it's traversed (and it's links counted) only once.
> - */
> -#ifndef XR_INO_REF_DEBUG
> -#define add_inode_refchecked(ino, ino_rec, ino_offset) \
> - XFS_INOPROC_SET_PROC((ino_rec), (ino_offset))
> -#define is_inode_refchecked(ino, ino_rec, ino_offset) \
> - (XFS_INOPROC_IS_PROC(ino_rec, ino_offset) != 0LL)
> -#else
> -void add_inode_refchecked(xfs_ino_t ino,
> - ino_tree_node_t *ino_rec, int ino_offset);
> -int is_inode_refchecked(xfs_ino_t ino,
> - ino_tree_node_t *ino_rec, int ino_offset);
> -#endif /* XR_INO_REF_DEBUG */
> -
> -/*
> * set/get inode number of parent -- works for directory inodes only
> */
> void set_inode_parent(ino_tree_node_t *irec, int ino_offset,
> Index: xfsprogs-dev/repair/incore_ino.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/incore_ino.c 2011-12-02 11:04:59.000000000 +0000
> +++ xfsprogs-dev/repair/incore_ino.c 2011-12-02 11:05:19.000000000 +0000
> @@ -784,19 +784,3 @@ incore_ino_init(xfs_mount_t *mp)
>
> full_ino_ex_data = 0;
> }
> -
> -#ifdef XR_INO_REF_DEBUG
> -void
> -add_inode_refchecked(xfs_ino_t ino, ino_tree_node_t *ino_rec, int ino_offset)
> -{
> - XFS_INOPROC_SET_PROC((ino_rec), (ino_offset));
> -
> - ASSERT(is_inode_refchecked(ino, ino_rec, ino_offset));
> -}
> -
> -int
> -is_inode_refchecked(xfs_ino_t ino, ino_tree_node_t *ino_rec, int ino_offset)
> -{
> - return(XFS_INOPROC_IS_PROC(ino_rec, ino_offset) == 0LL ? 0 : 1);
> -}
> -#endif /* XR_INO_REF_DEBUG */
> Index: xfsprogs-dev/include/libxfs.h
> ===================================================================
> --- xfsprogs-dev.orig/include/libxfs.h 2011-12-02 11:04:59.000000000 +0000
> +++ xfsprogs-dev/include/libxfs.h 2011-12-02 11:05:19.000000000 +0000
> @@ -503,8 +503,6 @@ extern unsigned long libxfs_physmem(void
> #include <xfs/xfs_log.h>
> #include <xfs/xfs_log_priv.h>
>
> -#define XFS_INOBT_CLR_FREE(rp,i) ((rp)->ir_free &= ~XFS_INOBT_MASK(i))
> -#define XFS_INOBT_SET_FREE(rp,i) ((rp)->ir_free |= XFS_INOBT_MASK(i))
> #define XFS_INOBT_IS_FREE_DISK(rp,i) \
> ((be64_to_cpu((rp)->ir_free) & XFS_INOBT_MASK(i)) != 0)
>
> Index: xfsprogs-dev/repair/phase6.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/phase6.c 2011-12-02 11:04:59.000000000 +0000
> +++ xfsprogs-dev/repair/phase6.c 2011-12-02 11:05:19.000000000 +0000
> @@ -3258,7 +3258,7 @@ process_dir_inode(
> * remaining illegal directory entries.
> */
>
> - ASSERT(!is_inode_refchecked(ino, irec, ino_offset) || dotdot_update);
> + ASSERT(!is_inode_refchecked(irec, ino_offset) || dotdot_update);
>
> error = libxfs_iget(mp, NULL, ino, 0, &ip, 0);
> if (error) {
> @@ -3282,7 +3282,7 @@ process_dir_inode(
> }
> }
>
> - add_inode_refchecked(ino, irec, 0);
> + add_inode_refchecked(irec, 0);
> return;
> }
>
> @@ -3300,7 +3300,7 @@ process_dir_inode(
> add_inode_reached(irec, ino_offset);
> }
>
> - add_inode_refchecked(ino, irec, ino_offset);
> + add_inode_refchecked(irec, ino_offset);
>
> hashtab = dir_hash_init(ip->i_d.di_size);
>
> Index: xfsprogs-dev/repair/phase7.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/phase7.c 2011-12-02 11:16:58.000000000 +0000
> +++ xfsprogs-dev/repair/phase7.c 2011-12-02 11:17:27.000000000 +0000
> @@ -143,10 +143,9 @@ phase7(xfs_mount_t *mp)
> continue;
>
> ASSERT(no_modify || is_inode_reached(irec, j));
> - ASSERT(no_modify ||
> - is_inode_referenced(irec, j));
>
> nrefs = num_inode_references(irec, j);
> + ASSERT(no_modify || nrefs > 0);
>
> if (get_inode_disk_nlinks(irec, j) != nrefs)
> update_inode_nlinks(mp,
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
---end quoted text---
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2012-01-11 11:30 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-02 17:46 [PATCH 00/12] xfs_repair queue Christoph Hellwig
2011-12-02 17:46 ` [PATCH 01/12] repair: do not walk the unlinked inode list Christoph Hellwig
2011-12-12 22:55 ` Dave Chinner
2012-01-12 19:30 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 02/12] repair: allocate and free inode records individually Christoph Hellwig
2011-12-12 23:16 ` Dave Chinner
2012-01-12 22:38 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 03/12] repair: allocate and free extent " Christoph Hellwig
2011-12-12 23:21 ` Dave Chinner
2012-01-12 22:39 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 04/12] xfsprogs: allow linking against libtcmalloc Christoph Hellwig
2011-12-13 0:05 ` Dave Chinner
2011-12-18 22:47 ` Christoph Hellwig
2011-12-02 17:46 ` [PATCH 05/12] repair: update extent count after zapping duplicate blocks Christoph Hellwig
2011-12-13 2:12 ` Dave Chinner
2012-02-02 12:39 ` [PATCH v2] " Christoph Hellwig
2012-02-02 18:19 ` Mark Tinguely
2012-01-13 17:18 ` [PATCH 05/12] " Mark Tinguely
2011-12-02 17:46 ` [PATCH 06/12] repair: use recursive buffer locking Christoph Hellwig
2011-12-13 2:22 ` Dave Chinner
2011-12-18 22:54 ` Christoph Hellwig
2012-01-13 20:10 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 07/12] repair: fix another ABBA deadlock in inode prefetching Christoph Hellwig
2011-12-13 2:35 ` Dave Chinner
2012-01-13 18:51 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 08/12] repair: handle filesystems with the log in allocation group 0 Christoph Hellwig
2011-12-13 2:36 ` Dave Chinner
2012-01-13 15:18 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 09/12] repair: kill check_inode_block Christoph Hellwig
2012-01-11 11:29 ` Christoph Hellwig
2012-01-11 21:28 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 10/12] repair: mark local functions static Christoph Hellwig
2012-01-11 11:30 ` Christoph Hellwig
2012-01-11 21:37 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 11/12] repair: move extern declarations to headers Christoph Hellwig
2012-01-11 11:30 ` Christoph Hellwig
2012-01-11 21:40 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 12/12] repair: cleanup inode record macros Christoph Hellwig
2012-01-11 11:30 ` Christoph Hellwig [this message]
2012-01-12 17:05 ` Mark Tinguely
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120111113020.GD10932@infradead.org \
--to=hch@infradead.org \
--cc=xfs@oss.sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.