* [PATCH 1/2] Make stuff static
@ 2006-09-29 3:28 sandeen
2006-10-14 4:31 ` Eric Sandeen
2006-10-16 9:12 ` Timothy Shimmin
0 siblings, 2 replies; 24+ messages in thread
From: sandeen @ 2006-09-29 3:28 UTC (permalink / raw)
To: xfs
Make things static which can be.
linux-2.4/xfs_vfs.c | 2 -
linux-2.4/xfs_vfs.h | 1
linux-2.4/xfs_vnode.c | 2 -
linux-2.6/xfs_vfs.c | 2 -
linux-2.6/xfs_vfs.h | 1
linux-2.6/xfs_vnode.c | 2 -
quota/xfs_dquot.c | 2 -
quota/xfs_qm_bhv.c | 2 -
xfs_attr.c | 3 +-
xfs_attr.h | 4 ---
xfs_bmap.c | 5 +---
xfs_bmap_btree.c | 52 ++++++++++++++++++++++++++------------------------
xfs_bmap_btree.h | 10 ---------
xfs_btree.c | 4 ++-
xfs_btree.h | 12 +----------
xfs_dir2.h | 4 ---
xfs_dir2_data.h | 2 -
xfs_dir2_node.h | 2 -
xfs_inode.c | 49 ++++++++++++++++++++++++++++++-----------------
xfs_inode.h | 29 +--------------------------
xfs_log_priv.h | 7 ------
xfs_log_recover.c | 10 ++++-----
xfs_mount.h | 2 -
xfs_quota.h | 2 -
xfs_trans_buf.c | 2 -
25 files changed, 84 insertions(+), 129 deletions(-)
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Index: xfs-linux/linux-2.4/xfs_vnode.c
===================================================================
--- xfs-linux.orig/linux-2.4/xfs_vnode.c
+++ xfs-linux/linux-2.4/xfs_vnode.c
@@ -17,7 +17,7 @@
#include "xfs.h"
uint64_t vn_generation; /* vnode generation number */
-spinlock_t vnumber_lock = SPIN_LOCK_UNLOCKED;
+static spinlock_t vnumber_lock = SPIN_LOCK_UNLOCKED;
/*
* Dedicated vnode inactive/reclaim sync semaphores.
Index: xfs-linux/linux-2.6/xfs_vnode.c
===================================================================
--- xfs-linux.orig/linux-2.6/xfs_vnode.c
+++ xfs-linux/linux-2.6/xfs_vnode.c
@@ -18,7 +18,7 @@
#include "xfs.h"
uint64_t vn_generation; /* vnode generation number */
-DEFINE_SPINLOCK(vnumber_lock);
+static DEFINE_SPINLOCK(vnumber_lock);
/*
* Dedicated vnode inactive/reclaim sync semaphores.
Index: xfs-linux/xfs_inode.c
===================================================================
--- xfs-linux.orig/xfs_inode.c
+++ xfs-linux/xfs_inode.c
@@ -65,7 +65,22 @@ STATIC int xfs_iflush_int(xfs_inode_t *,
STATIC int xfs_iformat_local(xfs_inode_t *, xfs_dinode_t *, int, int);
STATIC int xfs_iformat_extents(xfs_inode_t *, xfs_dinode_t *, int);
STATIC int xfs_iformat_btree(xfs_inode_t *, xfs_dinode_t *, int);
-
+STATIC void xfs_iext_add_indirect_multi(xfs_ifork_t *, int, xfs_extnum_t, int);
+STATIC void xfs_iext_remove_inline(xfs_ifork_t *, xfs_extnum_t, int);
+STATIC void xfs_iext_remove_direct(xfs_ifork_t *, xfs_extnum_t, int);
+STATIC void xfs_iext_remove_indirect(xfs_ifork_t *, xfs_extnum_t, int);
+STATIC void xfs_iext_inline_to_direct(xfs_ifork_t *, int);
+STATIC void xfs_iext_realloc_direct(xfs_ifork_t *, int);
+STATIC void xfs_iext_realloc_indirect(xfs_ifork_t *, int);
+STATIC void xfs_iext_direct_to_inline(xfs_ifork_t *, xfs_extnum_t);
+STATIC void xfs_iext_irec_init(xfs_ifork_t *);
+STATIC void xfs_iext_irec_remove(xfs_ifork_t *, int);
+STATIC void xfs_iext_irec_compact(xfs_ifork_t *);
+STATIC void xfs_iext_irec_compact_pages(xfs_ifork_t *);
+STATIC void xfs_iext_irec_compact_full(xfs_ifork_t *);
+STATIC void xfs_iext_irec_update_extoffs(xfs_ifork_t *, int, int);
+STATIC xfs_ext_irec_t *xfs_iext_bno_to_irec(xfs_ifork_t *, xfs_fileoff_t, int *);
+STATIC xfs_ext_irec_t *xfs_iext_irec_new(xfs_ifork_t *, int);
#ifdef DEBUG
/*
@@ -105,7 +120,7 @@ xfs_validate_extents(
* unlinked field of 0.
*/
#if defined(DEBUG)
-void
+STATIC void
xfs_inobp_check(
xfs_mount_t *mp,
xfs_buf_t *bp)
@@ -1268,7 +1283,7 @@ xfs_ialloc(
* at least do it for regular files.
*/
#ifdef DEBUG
-void
+STATIC void
xfs_isize_check(
xfs_mount_t *mp,
xfs_inode_t *ip,
@@ -3621,7 +3636,7 @@ xfs_iaccess(
/*
* xfs_iroundup: round up argument to next power of two
*/
-uint
+STATIC uint
xfs_iroundup(
uint v)
{
@@ -3834,7 +3849,7 @@ xfs_iext_add(
* | count | | nex2 | nex2 - number of extents after idx + count
* |-------| |-------|
*/
-void
+STATIC void
xfs_iext_add_indirect_multi(
xfs_ifork_t *ifp, /* inode fork pointer */
int erp_idx, /* target extent irec index */
@@ -3971,7 +3986,7 @@ xfs_iext_remove(
* This removes ext_diff extents from the inline buffer, beginning
* at extent index idx.
*/
-void
+STATIC void
xfs_iext_remove_inline(
xfs_ifork_t *ifp, /* inode fork pointer */
xfs_extnum_t idx, /* index to begin removing exts */
@@ -4008,7 +4023,7 @@ xfs_iext_remove_inline(
* at idx + ext_diff up in the list to overwrite the records being
* removed, then remove the extra space via kmem_realloc.
*/
-void
+STATIC void
xfs_iext_remove_direct(
xfs_ifork_t *ifp, /* inode fork pointer */
xfs_extnum_t idx, /* index to begin removing exts */
@@ -4060,7 +4075,7 @@ xfs_iext_remove_direct(
* | | | nex2 | nex2 - number of extents after idx + count
* |-------| |-------|
*/
-void
+STATIC void
xfs_iext_remove_indirect(
xfs_ifork_t *ifp, /* inode fork pointer */
xfs_extnum_t idx, /* index to begin removing extents */
@@ -4126,7 +4141,7 @@ xfs_iext_remove_indirect(
/*
* Create, destroy, or resize a linear (direct) block of extents.
*/
-void
+STATIC void
xfs_iext_realloc_direct(
xfs_ifork_t *ifp, /* inode fork pointer */
int new_size) /* new size of extents */
@@ -4187,7 +4202,7 @@ xfs_iext_realloc_direct(
/*
* Switch from linear (direct) extent records to inline buffer.
*/
-void
+STATIC void
xfs_iext_direct_to_inline(
xfs_ifork_t *ifp, /* inode fork pointer */
xfs_extnum_t nextents) /* number of extents in file */
@@ -4214,7 +4229,7 @@ xfs_iext_direct_to_inline(
* if_bytes here. It is the caller's responsibility to update
* if_bytes upon return.
*/
-void
+STATIC void
xfs_iext_inline_to_direct(
xfs_ifork_t *ifp, /* inode fork pointer */
int new_size) /* number of extents in file */
@@ -4234,7 +4249,7 @@ xfs_iext_inline_to_direct(
/*
* Resize an extent indirection array to new_size bytes.
*/
-void
+STATIC void
xfs_iext_realloc_indirect(
xfs_ifork_t *ifp, /* inode fork pointer */
int new_size) /* new indirection array size */
@@ -4259,7 +4274,7 @@ xfs_iext_realloc_indirect(
/*
* Switch from indirection array to linear (direct) extent allocations.
*/
-void
+STATIC void
xfs_iext_indirect_to_direct(
xfs_ifork_t *ifp) /* inode fork pointer */
{
@@ -4386,7 +4401,7 @@ xfs_iext_bno_to_ext(
* extent record for filesystem block bno. Store the index of the
* target irec in *erp_idxp.
*/
-xfs_ext_irec_t * /* pointer to found extent record */
+STATIC xfs_ext_irec_t * /* pointer to found extent record */
xfs_iext_bno_to_irec(
xfs_ifork_t *ifp, /* inode fork pointer */
xfs_fileoff_t bno, /* block number to search for */
@@ -4611,7 +4626,7 @@ xfs_iext_irec_remove(
* Partial Compaction: Extents occupy > 10% and < 50% of allocated space
* No Compaction: Extents occupy at least 50% of allocated space
*/
-void
+STATIC void
xfs_iext_irec_compact(
xfs_ifork_t *ifp) /* inode fork pointer */
{
@@ -4639,7 +4654,7 @@ xfs_iext_irec_compact(
/*
* Combine extents from neighboring extent pages.
*/
-void
+STATIC void
xfs_iext_irec_compact_pages(
xfs_ifork_t *ifp) /* inode fork pointer */
{
@@ -4676,7 +4691,7 @@ xfs_iext_irec_compact_pages(
/*
* Fully compact the extent records managed by the indirection array.
*/
-void
+STATIC void
xfs_iext_irec_compact_full(
xfs_ifork_t *ifp) /* inode fork pointer */
{
Index: xfs-linux/xfs_inode.h
===================================================================
--- xfs-linux.orig/xfs_inode.h
+++ xfs-linux/xfs_inode.h
@@ -460,7 +460,6 @@ int xfs_iextents_copy(xfs_inode_t *, xf
int xfs_iflush(xfs_inode_t *, uint);
void xfs_iflush_all(struct xfs_mount *);
int xfs_iaccess(xfs_inode_t *, mode_t, cred_t *);
-uint xfs_iroundup(uint);
void xfs_ichgtime(xfs_inode_t *, int);
xfs_fsize_t xfs_file_last_byte(xfs_inode_t *);
void xfs_lock_inodes(xfs_inode_t **, int, int, uint);
@@ -473,41 +472,17 @@ xfs_bmbt_rec_t *xfs_iext_get_ext(xfs_ifo
void xfs_iext_insert(xfs_ifork_t *, xfs_extnum_t, xfs_extnum_t,
xfs_bmbt_irec_t *);
void xfs_iext_add(xfs_ifork_t *, xfs_extnum_t, int);
-void xfs_iext_add_indirect_multi(xfs_ifork_t *, int, xfs_extnum_t, int);
void xfs_iext_remove(xfs_ifork_t *, xfs_extnum_t, int);
-void xfs_iext_remove_inline(xfs_ifork_t *, xfs_extnum_t, int);
-void xfs_iext_remove_direct(xfs_ifork_t *, xfs_extnum_t, int);
-void xfs_iext_remove_indirect(xfs_ifork_t *, xfs_extnum_t, int);
-void xfs_iext_realloc_direct(xfs_ifork_t *, int);
-void xfs_iext_realloc_indirect(xfs_ifork_t *, int);
-void xfs_iext_indirect_to_direct(xfs_ifork_t *);
-void xfs_iext_direct_to_inline(xfs_ifork_t *, xfs_extnum_t);
-void xfs_iext_inline_to_direct(xfs_ifork_t *, int);
void xfs_iext_destroy(xfs_ifork_t *);
xfs_bmbt_rec_t *xfs_iext_bno_to_ext(xfs_ifork_t *, xfs_fileoff_t, int *);
-xfs_ext_irec_t *xfs_iext_bno_to_irec(xfs_ifork_t *, xfs_fileoff_t, int *);
xfs_ext_irec_t *xfs_iext_idx_to_irec(xfs_ifork_t *, xfs_extnum_t *, int *, int);
-void xfs_iext_irec_init(xfs_ifork_t *);
-xfs_ext_irec_t *xfs_iext_irec_new(xfs_ifork_t *, int);
-void xfs_iext_irec_remove(xfs_ifork_t *, int);
-void xfs_iext_irec_compact(xfs_ifork_t *);
-void xfs_iext_irec_compact_pages(xfs_ifork_t *);
-void xfs_iext_irec_compact_full(xfs_ifork_t *);
-void xfs_iext_irec_update_extoffs(xfs_ifork_t *, int, int);
#define xfs_ipincount(ip) ((unsigned int) atomic_read(&ip->i_pincount))
-#ifdef DEBUG
-void xfs_isize_check(struct xfs_mount *, xfs_inode_t *, xfs_fsize_t);
-#else /* DEBUG */
+#ifndef DEBUG
#define xfs_isize_check(mp, ip, isize)
-#endif /* DEBUG */
-
-#if defined(DEBUG)
-void xfs_inobp_check(struct xfs_mount *, struct xfs_buf *);
-#else
#define xfs_inobp_check(mp, bp)
-#endif /* DEBUG */
+#endif /* !DEBUG */
extern struct kmem_zone *xfs_chashlist_zone;
extern struct kmem_zone *xfs_ifork_zone;
Index: xfs-linux/xfs_attr.h
===================================================================
--- xfs-linux.orig/xfs_attr.h
+++ xfs-linux/xfs_attr.h
@@ -59,7 +59,6 @@ typedef struct attrnames {
#define ATTR_NAMECOUNT 4
extern struct attrnames attr_user;
extern struct attrnames attr_secure;
-extern struct attrnames attr_system;
extern struct attrnames attr_trusted;
extern struct attrnames *attr_namespaces[ATTR_NAMECOUNT];
@@ -161,11 +160,8 @@ struct xfs_da_args;
*/
int xfs_attr_get(bhv_desc_t *, const char *, char *, int *, int, struct cred *);
int xfs_attr_set(bhv_desc_t *, const char *, char *, int, int, struct cred *);
-int xfs_attr_set_int(struct xfs_inode *, const char *, int, char *, int, int);
int xfs_attr_remove(bhv_desc_t *, const char *, int, struct cred *);
-int xfs_attr_remove_int(struct xfs_inode *, const char *, int, int);
int xfs_attr_list(bhv_desc_t *, char *, int, int, struct attrlist_cursor_kern *, struct cred *);
-int xfs_attr_list_int(struct xfs_attr_list_context *);
int xfs_attr_inactive(struct xfs_inode *dp);
int xfs_attr_shortform_getvalue(struct xfs_da_args *);
Index: xfs-linux/xfs_bmap_btree.c
===================================================================
--- xfs-linux.orig/xfs_bmap_btree.c
+++ xfs-linux/xfs_bmap_btree.c
@@ -60,7 +60,17 @@ STATIC int xfs_bmbt_rshift(xfs_btree_cur
STATIC int xfs_bmbt_split(xfs_btree_cur_t *, int, xfs_fsblock_t *,
__uint64_t *, xfs_btree_cur_t **, int *);
STATIC int xfs_bmbt_updkey(xfs_btree_cur_t *, xfs_bmbt_key_t *, int);
-
+STATIC xfs_bmbt_block_t *xfs_bmbt_get_block(struct xfs_btree_cur *cur,
+ int, struct xfs_buf **bpp);
+#ifndef XFS_NATIVE_HOST
+STATIC void xfs_bmbt_disk_set_all(xfs_bmbt_rec_t *r, xfs_bmbt_irec_t *s);
+STATIC void xfs_bmbt_disk_set_allf(xfs_bmbt_rec_t *r, xfs_fileoff_t o,
+ xfs_fsblock_t b, xfs_filblks_t c, xfs_exntst_t v);
+#ifdef DEBUG
+STATIC xfs_fsblock_t xfs_bmbt_disk_get_startblock(xfs_bmbt_rec_t *r);
+STATIC xfs_exntst_t xfs_bmbt_disk_get_state(xfs_bmbt_rec_t *r);
+#endif
+#endif
#if defined(XFS_BMBT_TRACE)
@@ -693,18 +703,14 @@ xfs_bmbt_get_rec(
{
xfs_bmbt_block_t *block;
xfs_buf_t *bp;
-#ifdef DEBUG
int error;
-#endif
int ptr;
xfs_bmbt_rec_t *rp;
block = xfs_bmbt_get_block(cur, 0, &bp);
ptr = cur->bc_ptrs[0];
-#ifdef DEBUG
if ((error = xfs_btree_check_lblock(cur, block, 0, bp)))
return error;
-#endif
if (ptr > be16_to_cpu(block->bb_numrecs) || ptr <= 0) {
*stat = 0;
return 0;
@@ -1913,7 +1919,7 @@ xfs_bmbt_get_all(
* Get the block pointer for the given level of the cursor.
* Fill in the buffer pointer, if applicable.
*/
-xfs_bmbt_block_t *
+STATIC xfs_bmbt_block_t *
xfs_bmbt_get_block(
xfs_btree_cur_t *cur,
int level,
@@ -2015,10 +2021,11 @@ xfs_bmbt_disk_get_blockcount(
return (xfs_filblks_t)(INT_GET(r->l1, ARCH_CONVERT) & XFS_MASK64LO(21));
}
+#ifdef DEBUG
/*
* Extract the startblock field from an on disk bmap extent record.
*/
-xfs_fsblock_t
+STATIC xfs_fsblock_t
xfs_bmbt_disk_get_startblock(
xfs_bmbt_rec_t *r)
{
@@ -2026,19 +2033,27 @@ xfs_bmbt_disk_get_startblock(
return (((xfs_fsblock_t)INT_GET(r->l0, ARCH_CONVERT) & XFS_MASK64LO(9)) << 43) |
(((xfs_fsblock_t)INT_GET(r->l1, ARCH_CONVERT)) >> 21);
#else
-#ifdef DEBUG
xfs_dfsbno_t b;
b = (((xfs_dfsbno_t)INT_GET(r->l0, ARCH_CONVERT) & XFS_MASK64LO(9)) << 43) |
(((xfs_dfsbno_t)INT_GET(r->l1, ARCH_CONVERT)) >> 21);
ASSERT((b >> 32) == 0 || ISNULLDSTARTBLOCK(b));
return (xfs_fsblock_t)b;
-#else /* !DEBUG */
- return (xfs_fsblock_t)(((xfs_dfsbno_t)INT_GET(r->l1, ARCH_CONVERT)) >> 21);
-#endif /* DEBUG */
#endif /* XFS_BIG_BLKNOS */
}
+STATIC xfs_exntst_t
+xfs_bmbt_disk_get_state(
+ xfs_bmbt_rec_t *r)
+{
+ int ext_flag;
+
+ ext_flag = (int)((INT_GET(r->l0, ARCH_CONVERT)) >> (64 - BMBT_EXNTFLAG_BITLEN));
+ return xfs_extent_state(xfs_bmbt_disk_get_blockcount(r),
+ ext_flag);
+}
+#endif /* DEBUG */
+
/*
* Extract the startoff field from a disk format bmap extent record.
*/
@@ -2049,17 +2064,6 @@ xfs_bmbt_disk_get_startoff(
return ((xfs_fileoff_t)INT_GET(r->l0, ARCH_CONVERT) &
XFS_MASK64LO(64 - BMBT_EXNTFLAG_BITLEN)) >> 9;
}
-
-xfs_exntst_t
-xfs_bmbt_disk_get_state(
- xfs_bmbt_rec_t *r)
-{
- int ext_flag;
-
- ext_flag = (int)((INT_GET(r->l0, ARCH_CONVERT)) >> (64 - BMBT_EXNTFLAG_BITLEN));
- return xfs_extent_state(xfs_bmbt_disk_get_blockcount(r),
- ext_flag);
-}
#endif /* XFS_NATIVE_HOST */
@@ -2506,7 +2510,7 @@ xfs_bmbt_set_allf(
/*
* Set all the fields in a bmap extent record from the uncompressed form.
*/
-void
+STATIC void
xfs_bmbt_disk_set_all(
xfs_bmbt_rec_t *r,
xfs_bmbt_irec_t *s)
@@ -2548,7 +2552,7 @@ xfs_bmbt_disk_set_all(
/*
* Set all the fields in a disk format bmap extent record from the arguments.
*/
-void
+STATIC void
xfs_bmbt_disk_set_allf(
xfs_bmbt_rec_t *r,
xfs_fileoff_t o,
Index: xfs-linux/xfs_bmap_btree.h
===================================================================
--- xfs-linux.orig/xfs_bmap_btree.h
+++ xfs-linux/xfs_bmap_btree.h
@@ -306,8 +306,6 @@ extern void xfs_bmdr_to_bmbt(xfs_bmdr_bl
extern int xfs_bmbt_decrement(struct xfs_btree_cur *, int, int *);
extern int xfs_bmbt_delete(struct xfs_btree_cur *, int *);
extern void xfs_bmbt_get_all(xfs_bmbt_rec_t *r, xfs_bmbt_irec_t *s);
-extern xfs_bmbt_block_t *xfs_bmbt_get_block(struct xfs_btree_cur *cur,
- int, struct xfs_buf **bpp);
extern xfs_filblks_t xfs_bmbt_get_blockcount(xfs_bmbt_rec_t *r);
extern xfs_fsblock_t xfs_bmbt_get_startblock(xfs_bmbt_rec_t *r);
extern xfs_fileoff_t xfs_bmbt_get_startoff(xfs_bmbt_rec_t *r);
@@ -315,9 +313,7 @@ extern xfs_exntst_t xfs_bmbt_get_state(x
#ifndef XFS_NATIVE_HOST
extern void xfs_bmbt_disk_get_all(xfs_bmbt_rec_t *r, xfs_bmbt_irec_t *s);
-extern xfs_exntst_t xfs_bmbt_disk_get_state(xfs_bmbt_rec_t *r);
extern xfs_filblks_t xfs_bmbt_disk_get_blockcount(xfs_bmbt_rec_t *r);
-extern xfs_fsblock_t xfs_bmbt_disk_get_startblock(xfs_bmbt_rec_t *r);
extern xfs_fileoff_t xfs_bmbt_disk_get_startoff(xfs_bmbt_rec_t *r);
#else
#define xfs_bmbt_disk_get_all(r, s) xfs_bmbt_get_all(r, s)
@@ -351,11 +347,7 @@ extern void xfs_bmbt_set_startblock(xfs_
extern void xfs_bmbt_set_startoff(xfs_bmbt_rec_t *r, xfs_fileoff_t v);
extern void xfs_bmbt_set_state(xfs_bmbt_rec_t *r, xfs_exntst_t v);
-#ifndef XFS_NATIVE_HOST
-extern void xfs_bmbt_disk_set_all(xfs_bmbt_rec_t *r, xfs_bmbt_irec_t *s);
-extern void xfs_bmbt_disk_set_allf(xfs_bmbt_rec_t *r, xfs_fileoff_t o,
- xfs_fsblock_t b, xfs_filblks_t c, xfs_exntst_t v);
-#else
+#ifdef XFS_NATIVE_HOST
#define xfs_bmbt_disk_set_all(r, s) xfs_bmbt_set_all(r, s)
#define xfs_bmbt_disk_set_allf(r, o, b, c, v) xfs_bmbt_set_allf(r, o, b, c, v)
#endif /* XFS_NATIVE_HOST */
Index: xfs-linux/xfs_dir2.h
===================================================================
--- xfs-linux.orig/xfs_dir2.h
+++ xfs-linux/xfs_dir2.h
@@ -107,10 +107,6 @@ extern int xfs_dir_ino_validate(struct x
*/
extern int xfs_dir2_grow_inode(struct xfs_da_args *args, int space,
xfs_dir2_db_t *dbp);
-extern int xfs_dir2_isblock(struct xfs_trans *tp, struct xfs_inode *dp,
- int *vp);
-extern int xfs_dir2_isleaf(struct xfs_trans *tp, struct xfs_inode *dp,
- int *vp);
extern int xfs_dir2_shrink_inode(struct xfs_da_args *args, xfs_dir2_db_t db,
struct xfs_dabuf *bp);
Index: xfs-linux/xfs_dir2_data.h
===================================================================
--- xfs-linux.orig/xfs_dir2_data.h
+++ xfs-linux/xfs_dir2_data.h
@@ -161,8 +161,6 @@ extern void xfs_dir2_data_check(struct x
#else
#define xfs_dir2_data_check(dp,bp)
#endif
-extern xfs_dir2_data_free_t *xfs_dir2_data_freefind(xfs_dir2_data_t *d,
- xfs_dir2_data_unused_t *dup);
extern xfs_dir2_data_free_t *xfs_dir2_data_freeinsert(xfs_dir2_data_t *d,
xfs_dir2_data_unused_t *dup, int *loghead);
extern void xfs_dir2_data_freescan(struct xfs_mount *mp, xfs_dir2_data_t *d,
Index: xfs-linux/xfs_dir2_node.h
===================================================================
--- xfs-linux.orig/xfs_dir2_node.h
+++ xfs-linux/xfs_dir2_node.h
@@ -77,8 +77,6 @@ xfs_dir2_db_to_fdindex(struct xfs_mount
return ((db) % XFS_DIR2_MAX_FREE_BESTS(mp));
}
-extern void xfs_dir2_free_log_bests(struct xfs_trans *tp, struct xfs_dabuf *bp,
- int first, int last);
extern int xfs_dir2_leaf_to_node(struct xfs_da_args *args,
struct xfs_dabuf *lbp);
extern xfs_dahash_t xfs_dir2_leafn_lasthash(struct xfs_dabuf *bp, int *count);
Index: xfs-linux/xfs_log_priv.h
===================================================================
--- xfs-linux.orig/xfs_log_priv.h
+++ xfs-linux/xfs_log_priv.h
@@ -484,18 +484,11 @@ typedef struct log {
/* common routines */
extern xfs_lsn_t xlog_assign_tail_lsn(struct xfs_mount *mp);
-extern int xlog_find_tail(xlog_t *log,
- xfs_daddr_t *head_blk,
- xfs_daddr_t *tail_blk);
extern int xlog_recover(xlog_t *log);
extern int xlog_recover_finish(xlog_t *log, int mfsi_flags);
extern void xlog_pack_data(xlog_t *log, xlog_in_core_t *iclog, int);
extern void xlog_recover_process_iunlinks(xlog_t *log);
-extern struct xfs_buf *xlog_get_bp(xlog_t *, int);
-extern void xlog_put_bp(struct xfs_buf *);
-extern int xlog_bread(xlog_t *, xfs_daddr_t, int, struct xfs_buf *);
-
/* iclog tracing */
#define XLOG_TRACE_GRAB_FLUSH 1
#define XLOG_TRACE_REL_FLUSH 2
Index: xfs-linux/xfs_log_recover.c
===================================================================
--- xfs-linux.orig/xfs_log_recover.c
+++ xfs-linux/xfs_log_recover.c
@@ -69,7 +69,7 @@ STATIC void xlog_recover_check_ail(xfs_m
((bbs + (log)->l_sectbb_mask + 1) & ~(log)->l_sectbb_mask) : (bbs) )
#define XLOG_SECTOR_ROUNDDOWN_BLKNO(log, bno) ((bno) & ~(log)->l_sectbb_mask)
-xfs_buf_t *
+STATIC xfs_buf_t *
xlog_get_bp(
xlog_t *log,
int num_bblks)
@@ -84,7 +84,7 @@ xlog_get_bp(
return xfs_buf_get_noaddr(BBTOB(num_bblks), log->l_mp->m_logdev_targp);
}
-void
+STATIC void
xlog_put_bp(
xfs_buf_t *bp)
{
@@ -95,7 +95,7 @@ xlog_put_bp(
/*
* nbblks should be uint, but oh well. Just want to catch that 32-bit length.
*/
-int
+STATIC int
xlog_bread(
xlog_t *log,
xfs_daddr_t blk_no,
@@ -293,7 +293,7 @@ xlog_recover_iodone(
* Note that the algorithm can not be perfect because the disk will not
* necessarily be perfect.
*/
-int
+STATIC int
xlog_find_cycle_start(
xlog_t *log,
xfs_buf_t *bp,
@@ -777,7 +777,7 @@ xlog_find_head(
* We could speed up search by using current head_blk buffer, but it is not
* available.
*/
-int
+STATIC int
xlog_find_tail(
xlog_t *log,
xfs_daddr_t *head_blk,
Index: xfs-linux/xfs_mount.h
===================================================================
--- xfs-linux.orig/xfs_mount.h
+++ xfs-linux/xfs_mount.h
@@ -586,8 +586,6 @@ extern void xfs_unmountfs_close(xfs_moun
extern int xfs_unmountfs_writesb(xfs_mount_t *);
extern int xfs_unmount_flush(xfs_mount_t *, int);
extern int xfs_mod_incore_sb(xfs_mount_t *, xfs_sb_field_t, int, int);
-extern int xfs_mod_incore_sb_unlocked(xfs_mount_t *, xfs_sb_field_t,
- int, int);
extern int xfs_mod_incore_sb_batch(xfs_mount_t *, xfs_mod_sb_t *,
uint, int);
extern struct xfs_buf *xfs_getsb(xfs_mount_t *, int);
Index: xfs-linux/quota/xfs_qm_bhv.c
===================================================================
--- xfs-linux.orig/quota/xfs_qm_bhv.c
+++ xfs-linux/quota/xfs_qm_bhv.c
@@ -401,7 +401,7 @@ STATIC struct xfs_qmops xfs_qmcore_xfs =
.xfs_dqtrxops = &xfs_trans_dquot_ops,
};
-struct bhv_module_vfsops xfs_qmops = { {
+static struct bhv_module_vfsops xfs_qmops = { {
BHV_IDENTITY_INIT(VFS_BHV_QM, VFS_POSITION_QM),
.vfs_parseargs = xfs_qm_parseargs,
.vfs_showargs = xfs_qm_showargs,
Index: xfs-linux/xfs_attr.c
===================================================================
--- xfs-linux.orig/xfs_attr.c
+++ xfs-linux/xfs_attr.c
@@ -59,6 +59,7 @@
#define ATTR_SYSCOUNT 2
STATIC struct attrnames posix_acl_access;
STATIC struct attrnames posix_acl_default;
+STATIC struct attrnames attr_system;
STATIC struct attrnames *attr_system_names[ATTR_SYSCOUNT];
/*========================================================================
@@ -2690,7 +2691,7 @@ attr_system_remove(
return namesp->attr_remove(vp, name, xflags);
}
-struct attrnames attr_system = {
+STATIC struct attrnames attr_system = {
.attr_name = "system.",
.attr_namelen = sizeof("system.") - 1,
.attr_flag = ATTR_SYSTEM,
Index: xfs-linux/xfs_quota.h
===================================================================
--- xfs-linux.orig/xfs_quota.h
+++ xfs-linux/xfs_quota.h
@@ -363,8 +363,6 @@ typedef struct xfs_dqtrxops {
extern int xfs_qm_dqcheck(xfs_disk_dquot_t *, xfs_dqid_t, uint, uint, char *);
extern int xfs_mount_reset_sbqflags(struct xfs_mount *);
-extern struct bhv_module_vfsops xfs_qmops;
-
#endif /* __KERNEL__ */
#endif /* __XFS_QUOTA_H__ */
Index: xfs-linux/linux-2.4/xfs_vfs.c
===================================================================
--- xfs-linux.orig/linux-2.4/xfs_vfs.c
+++ xfs-linux/linux-2.4/xfs_vfs.c
@@ -275,7 +275,7 @@ vfs_deallocate(
kmem_free(vfsp, sizeof(bhv_vfs_t));
}
-void
+static void
vfs_insertbhv(
struct bhv_vfs *vfsp,
struct bhv_desc *bdp,
Index: xfs-linux/linux-2.4/xfs_vfs.h
===================================================================
--- xfs-linux.orig/linux-2.4/xfs_vfs.h
+++ xfs-linux/linux-2.4/xfs_vfs.h
@@ -230,7 +230,6 @@ typedef struct bhv_module {
extern bhv_vfs_t *vfs_allocate(struct super_block *);
extern bhv_vfs_t *vfs_from_sb(struct super_block *);
extern void vfs_deallocate(bhv_vfs_t *);
-extern void vfs_insertbhv(bhv_vfs_t *, bhv_desc_t *, bhv_vfsops_t *, void *);
#define bhv_lookup_module(n,m) ( (m) ? \
inter_module_get_request(n, m) : \
Index: xfs-linux/linux-2.6/xfs_vfs.c
===================================================================
--- xfs-linux.orig/linux-2.6/xfs_vfs.c
+++ xfs-linux/linux-2.6/xfs_vfs.c
@@ -274,7 +274,7 @@ vfs_deallocate(
kmem_free(vfsp, sizeof(bhv_vfs_t));
}
-void
+static void
vfs_insertbhv(
struct bhv_vfs *vfsp,
struct bhv_desc *bdp,
Index: xfs-linux/linux-2.6/xfs_vfs.h
===================================================================
--- xfs-linux.orig/linux-2.6/xfs_vfs.h
+++ xfs-linux/linux-2.6/xfs_vfs.h
@@ -224,7 +224,6 @@ typedef struct bhv_module {
extern bhv_vfs_t *vfs_allocate(struct super_block *);
extern bhv_vfs_t *vfs_from_sb(struct super_block *);
extern void vfs_deallocate(bhv_vfs_t *);
-extern void vfs_insertbhv(bhv_vfs_t *, bhv_desc_t *, bhv_vfsops_t *, void *);
extern void bhv_module_init(const char *, struct module *, const void *);
extern void bhv_module_exit(const char *);
Index: xfs-linux/quota/xfs_dquot.c
===================================================================
--- xfs-linux.orig/quota/xfs_dquot.c
+++ xfs-linux/quota/xfs_dquot.c
@@ -72,7 +72,7 @@ STATIC void xfs_qm_dqflush_done(xfs_buf
xfs_buftarg_t *xfs_dqerror_target;
int xfs_do_dqerror;
int xfs_dqreq_num;
-int xfs_dqerror_mod = 33;
+STATIC int xfs_dqerror_mod = 33;
#endif
/*
Index: xfs-linux/xfs_bmap.c
===================================================================
--- xfs-linux.orig/xfs_bmap.c
+++ xfs-linux/xfs_bmap.c
@@ -6072,8 +6072,7 @@ xfs_bmap_check_extents(
}
}
-STATIC
-xfs_buf_t *
+STATIC xfs_buf_t *
xfs_bmap_get_bp(
xfs_btree_cur_t *cur,
xfs_fsblock_t bno)
@@ -6134,7 +6133,7 @@ xfs_bmap_get_bp(
return(bp);
}
-void
+STATIC void
xfs_check_block(
xfs_bmbt_block_t *block,
xfs_mount_t *mp,
Index: xfs-linux/xfs_btree.c
===================================================================
--- xfs-linux.orig/xfs_btree.c
+++ xfs-linux/xfs_btree.c
@@ -110,7 +110,7 @@ xfs_btree_maxrecs(
/*
* Debug routine: check that block header is ok.
*/
-void
+STATIC void
xfs_btree_check_block(
xfs_btree_cur_t *cur, /* btree cursor */
xfs_btree_block_t *block, /* generic btree block pointer */
@@ -337,6 +337,7 @@ xfs_btree_check_sblock(
return 0;
}
+#ifdef DEBUG
/*
* Checking routine: check that (short) pointer is ok.
*/
@@ -357,6 +358,7 @@ xfs_btree_check_sptr(
ptr < be32_to_cpu(agf->agf_length));
return 0;
}
+#endif
/*
* Delete the btree cursor.
Index: xfs-linux/xfs_btree.h
===================================================================
--- xfs-linux.orig/xfs_btree.h
+++ xfs-linux/xfs_btree.h
@@ -192,16 +192,6 @@ typedef struct xfs_btree_cur
#ifdef DEBUG
/*
- * Debug routine: check that block header is ok.
- */
-void
-xfs_btree_check_block(
- xfs_btree_cur_t *cur, /* btree cursor */
- xfs_btree_block_t *block, /* generic btree block pointer */
- int level, /* level of the btree block */
- struct xfs_buf *bp); /* buffer containing block, if any */
-
-/*
* Debug routine: check that keys are in the right order.
*/
void
@@ -256,6 +246,7 @@ xfs_btree_check_sblock(
int level, /* level of the btree block */
struct xfs_buf *bp); /* buffer containing block */
+#ifdef DEBUG
/*
* Checking routine: check that (short) pointer is ok.
*/
@@ -264,6 +255,7 @@ xfs_btree_check_sptr(
xfs_btree_cur_t *cur, /* btree cursor */
xfs_agblock_t ptr, /* btree block disk address */
int level); /* btree block level */
+#endif
/*
* Delete the btree cursor.
Index: xfs-linux/xfs_trans_buf.c
===================================================================
--- xfs-linux.orig/xfs_trans_buf.c
+++ xfs-linux/xfs_trans_buf.c
@@ -260,7 +260,7 @@ xfs_trans_getsb(xfs_trans_t *tp,
xfs_buftarg_t *xfs_error_target;
int xfs_do_error;
int xfs_req_num;
-int xfs_error_mod = 33;
+STATIC int xfs_error_mod = 33;
#endif
/*
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 1/2] Make stuff static
2006-09-29 3:28 [PATCH 1/2] Make stuff static sandeen
@ 2006-10-14 4:31 ` Eric Sandeen
2006-10-16 9:12 ` Timothy Shimmin
1 sibling, 0 replies; 24+ messages in thread
From: Eric Sandeen @ 2006-10-14 4:31 UTC (permalink / raw)
To: xfs
sandeen@sandeen.net wrote:
> Make things static which can be.
>
> linux-2.4/xfs_vfs.c | 2 -
> linux-2.4/xfs_vfs.h | 1
> linux-2.4/xfs_vnode.c | 2 -
> linux-2.6/xfs_vfs.c | 2 -
> linux-2.6/xfs_vfs.h | 1
> linux-2.6/xfs_vnode.c | 2 -
> quota/xfs_dquot.c | 2 -
> quota/xfs_qm_bhv.c | 2 -
> xfs_attr.c | 3 +-
....
Ugh, forgot a quilt refresh, there are a couple files missing here.
Is there any interest in this change? if so I'll send the update.
Thanks,
-Eric
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] Make stuff static
2006-09-29 3:28 [PATCH 1/2] Make stuff static sandeen
2006-10-14 4:31 ` Eric Sandeen
@ 2006-10-16 9:12 ` Timothy Shimmin
2006-10-16 13:49 ` Eric Sandeen
1 sibling, 1 reply; 24+ messages in thread
From: Timothy Shimmin @ 2006-10-16 9:12 UTC (permalink / raw)
To: sandeen, xfs
Okay, started looking :-)
Some comments below...
--Tim
--On 28 September 2006 10:28:56 PM -0500 sandeen@sandeen.net wrote:
> Make things static which can be.
>
> linux-2.4/xfs_vfs.c | 2 -
> linux-2.4/xfs_vfs.h | 1
> linux-2.4/xfs_vnode.c | 2 -
> linux-2.6/xfs_vfs.c | 2 -
> linux-2.6/xfs_vfs.h | 1
> linux-2.6/xfs_vnode.c | 2 -
> quota/xfs_dquot.c | 2 -
> quota/xfs_qm_bhv.c | 2 -
> xfs_attr.c | 3 +-
> xfs_attr.h | 4 ---
> xfs_bmap.c | 5 +---
> xfs_bmap_btree.c | 52 ++++++++++++++++++++++++++------------------------
> xfs_bmap_btree.h | 10 ---------
> xfs_btree.c | 4 ++-
> xfs_btree.h | 12 +----------
> xfs_dir2.h | 4 ---
> xfs_dir2_data.h | 2 -
> xfs_dir2_node.h | 2 -
> xfs_inode.c | 49 ++++++++++++++++++++++++++++++-----------------
> xfs_inode.h | 29 +--------------------------
> xfs_log_priv.h | 7 ------
> xfs_log_recover.c | 10 ++++-----
> xfs_mount.h | 2 -
> xfs_quota.h | 2 -
> xfs_trans_buf.c | 2 -
> 25 files changed, 84 insertions(+), 129 deletions(-)
>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
>
> Index: xfs-linux/linux-2.4/xfs_vnode.c
> ===================================================================
> --- xfs-linux.orig/linux-2.4/xfs_vnode.c
> +++ xfs-linux/linux-2.4/xfs_vnode.c
> @@ -17,7 +17,7 @@
> #include "xfs.h"
>
> uint64_t vn_generation; /* vnode generation number */
> -spinlock_t vnumber_lock = SPIN_LOCK_UNLOCKED;
> +static spinlock_t vnumber_lock = SPIN_LOCK_UNLOCKED;
>
Not STATIC? Curious.
> /*
> * Dedicated vnode inactive/reclaim sync semaphores.
> Index: xfs-linux/linux-2.6/xfs_vnode.c
> ===================================================================
> --- xfs-linux.orig/linux-2.6/xfs_vnode.c
> +++ xfs-linux/linux-2.6/xfs_vnode.c
> @@ -18,7 +18,7 @@
> #include "xfs.h"
>
> uint64_t vn_generation; /* vnode generation number */
> -DEFINE_SPINLOCK(vnumber_lock);
> +static DEFINE_SPINLOCK(vnumber_lock);
>
Not STATIC?
> /*
> * Dedicated vnode inactive/reclaim sync semaphores.
> Index: xfs-linux/xfs_inode.c
> ===================================================================
> --- xfs-linux.orig/xfs_inode.c
> +++ xfs-linux/xfs_inode.c
> @@ -65,7 +65,22 @@ STATIC int xfs_iflush_int(xfs_inode_t *,
> STATIC int xfs_iformat_local(xfs_inode_t *, xfs_dinode_t *, int, int);
> STATIC int xfs_iformat_extents(xfs_inode_t *, xfs_dinode_t *, int);
> STATIC int xfs_iformat_btree(xfs_inode_t *, xfs_dinode_t *, int);
> -
> +STATIC void xfs_iext_add_indirect_multi(xfs_ifork_t *, int, xfs_extnum_t, int);
> +STATIC void xfs_iext_remove_inline(xfs_ifork_t *, xfs_extnum_t, int);
> +STATIC void xfs_iext_remove_direct(xfs_ifork_t *, xfs_extnum_t, int);
> +STATIC void xfs_iext_remove_indirect(xfs_ifork_t *, xfs_extnum_t, int);
> +STATIC void xfs_iext_inline_to_direct(xfs_ifork_t *, int);
> +STATIC void xfs_iext_realloc_direct(xfs_ifork_t *, int);
> +STATIC void xfs_iext_realloc_indirect(xfs_ifork_t *, int);
> +STATIC void xfs_iext_direct_to_inline(xfs_ifork_t *, xfs_extnum_t);
> +STATIC void xfs_iext_irec_init(xfs_ifork_t *);
> +STATIC void xfs_iext_irec_remove(xfs_ifork_t *, int);
> +STATIC void xfs_iext_irec_compact(xfs_ifork_t *);
> +STATIC void xfs_iext_irec_compact_pages(xfs_ifork_t *);
> +STATIC void xfs_iext_irec_compact_full(xfs_ifork_t *);
> +STATIC void xfs_iext_irec_update_extoffs(xfs_ifork_t *, int, int);
> +STATIC xfs_ext_irec_t *xfs_iext_bno_to_irec(xfs_ifork_t *, xfs_fileoff_t, int *);
> +STATIC xfs_ext_irec_t *xfs_iext_irec_new(xfs_ifork_t *, int);
>
Yeah, they all the xfs_iext_* seem to be local. see below...
> #ifdef DEBUG
> /*
> @@ -105,7 +120,7 @@ xfs_validate_extents(
> * unlinked field of 0.
> */
> #if defined(DEBUG)
> -void
> +STATIC void
> xfs_inobp_check(
Yep.
> xfs_mount_t *mp,
> xfs_buf_t *bp)
> @@ -1268,7 +1283,7 @@ xfs_ialloc(
> * at least do it for regular files.
> */
> #ifdef DEBUG
> -void
> +STATIC void
> xfs_isize_check(
Yep.
> xfs_mount_t *mp,
> xfs_inode_t *ip,
> @@ -3621,7 +3636,7 @@ xfs_iaccess(
> /*
> * xfs_iroundup: round up argument to next power of two
> */
> -uint
> +STATIC uint
> xfs_iroundup(
Yep.
> uint v)
> {
> @@ -3834,7 +3849,7 @@ xfs_iext_add(
> * | count | | nex2 | nex2 - number of extents after idx + count
> * |-------| |-------|
> */
> -void
> +STATIC void
> xfs_iext_add_indirect_multi(
Yep for all xfs_iext_ funcs.
Well actually it looks like xfs_bmap.c uses these:
--------------------
xfs_iext_add
xfs_iext_bno_to_ext
xfs_iext_get_ext
xfs_iext_insert
xfs_iext_remove
---------------------
So I'm not sure why some are still here...
xfs_iext_destroy
xfs_iext_idx_to_irec
xfs_iext_realloc (only exists in header???)
--Tim
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 1/2] Make stuff static
2006-10-16 9:12 ` Timothy Shimmin
@ 2006-10-16 13:49 ` Eric Sandeen
2006-10-16 21:34 ` Eric Sandeen
0 siblings, 1 reply; 24+ messages in thread
From: Eric Sandeen @ 2006-10-16 13:49 UTC (permalink / raw)
To: Timothy Shimmin; +Cc: xfs
Timothy Shimmin wrote:
> Okay, started looking :-)
>
> Some comments below...
>
> --Tim
Thanks Tim! :)
>> --- xfs-linux.orig/linux-2.4/xfs_vnode.c
>> +++ xfs-linux/linux-2.4/xfs_vnode.c
>> @@ -17,7 +17,7 @@
>> #include "xfs.h"
>>
>> uint64_t vn_generation; /* vnode generation number */
>> -spinlock_t vnumber_lock = SPIN_LOCK_UNLOCKED;
>> +static spinlock_t vnumber_lock = SPIN_LOCK_UNLOCKED;
>>
>
> Not STATIC? Curious.
Yeah, probably should be; IIRC there were other "statics" in that file, was just
following the convention. I'll make sure it builds OK w/ STATIC and send an
updated patch (need to do that anyway).
> Yep for all xfs_iext_ funcs.
>
> Well actually it looks like xfs_bmap.c uses these:
> --------------------
> xfs_iext_add
> xfs_iext_bno_to_ext
> xfs_iext_get_ext
> xfs_iext_insert
> xfs_iext_remove
> ---------------------
>
> So I'm not sure why some are still here...
> xfs_iext_destroy
Exported... maybe you can tell me why? :)
> xfs_iext_idx_to_irec
Same deal.
> xfs_iext_realloc (only exists in header???)
Odd, missed that one, guess it can go too? Maybe others lurk as well.
Thanks!
-Eric
> --Tim
>
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] Make stuff static
2006-10-16 13:49 ` Eric Sandeen
@ 2006-10-16 21:34 ` Eric Sandeen
2006-10-16 23:22 ` David Chinner
0 siblings, 1 reply; 24+ messages in thread
From: Eric Sandeen @ 2006-10-16 21:34 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Timothy Shimmin, xfs
Eric Sandeen wrote:
> Timothy Shimmin wrote:
>> Okay, started looking :-)
One other thing, based on the bug on osdl today, some of these larger
newly-static functions should probably be marked noinline to keep gcc
from doing things we don't want it to...
-Eric
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] Make stuff static
2006-10-16 21:34 ` Eric Sandeen
@ 2006-10-16 23:22 ` David Chinner
2006-10-16 23:55 ` Russell Cattelan
0 siblings, 1 reply; 24+ messages in thread
From: David Chinner @ 2006-10-16 23:22 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Timothy Shimmin, xfs
On Mon, Oct 16, 2006 at 04:34:34PM -0500, Eric Sandeen wrote:
> Eric Sandeen wrote:
> > Timothy Shimmin wrote:
> >> Okay, started looking :-)
>
> One other thing, based on the bug on osdl today, some of these larger
> newly-static functions should probably be marked noinline to keep gcc
> from doing things we don't want it to...
<grumble>
This is not an obvious compiler hint (compared to, say, likely()) as
the functions gcc automatically inlines changes according to
compiler version, optimisation level and platform. Hence adding
noinline notation will be like playing whack-a-mole and I doubt it
will be consistently used or maintained moving forward. It's the
wrong solution, IMO.
I think we should change the definition of STATIC so we don't have
to poison the code to work around some stupid compiler behaviour.
That is, unless we specifically say "inline" for static functions,
we really mean "noinline".
This will also make debugging easier because we won't get stack
traces that are apparently missing functions and all the associated
pain that this can cause.
Thoughts?
Cheers,
Dave.
--
Dave Chinner
Principal Engineer
SGI Australian Software Group
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] Make stuff static
2006-10-16 23:22 ` David Chinner
@ 2006-10-16 23:55 ` Russell Cattelan
2006-10-17 0:50 ` David Chinner
0 siblings, 1 reply; 24+ messages in thread
From: Russell Cattelan @ 2006-10-16 23:55 UTC (permalink / raw)
To: David Chinner; +Cc: Eric Sandeen, Timothy Shimmin, xfs
[-- Attachment #1: Type: text/plain, Size: 1660 bytes --]
On Tue, 2006-10-17 at 09:22 +1000, David Chinner wrote:
> On Mon, Oct 16, 2006 at 04:34:34PM -0500, Eric Sandeen wrote:
> > Eric Sandeen wrote:
> > > Timothy Shimmin wrote:
> > >> Okay, started looking :-)
> >
> > One other thing, based on the bug on osdl today, some of these larger
> > newly-static functions should probably be marked noinline to keep gcc
> > from doing things we don't want it to...
>
> <grumble>
>
> This is not an obvious compiler hint (compared to, say, likely()) as
> the functions gcc automatically inlines changes according to
> compiler version, optimisation level and platform. Hence adding
> noinline notation will be like playing whack-a-mole and I doubt it
> will be consistently used or maintained moving forward. It's the
> wrong solution, IMO.
>
> I think we should change the definition of STATIC so we don't have
> to poison the code to work around some stupid compiler behaviour.
> That is, unless we specifically say "inline" for static functions,
> we really mean "noinline".
So you are proposing?
/* non-debug */
#define STATIC static inline
/* debug */
#define STATIC noinline
That doesn't sound right that would inline everything that is static
which probably not a good idea in terms of stack usage.
#define STATIC static noinline
?
That doesn't work either ...
STATIC inline
becomes
static noline inline?
>
> This will also make debugging easier because we won't get stack
> traces that are apparently missing functions and all the associated
> pain that this can cause.
>
> Thoughts?
>
> Cheers,
>
> Dave.
--
Russell Cattelan <cattelan@thebarn.com>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] Make stuff static
2006-10-16 23:55 ` Russell Cattelan
@ 2006-10-17 0:50 ` David Chinner
2006-10-17 1:03 ` Eric Sandeen
2006-10-17 7:13 ` Tim Shimmin
0 siblings, 2 replies; 24+ messages in thread
From: David Chinner @ 2006-10-17 0:50 UTC (permalink / raw)
To: Russell Cattelan; +Cc: David Chinner, Eric Sandeen, Timothy Shimmin, xfs
On Mon, Oct 16, 2006 at 06:55:43PM -0500, Russell Cattelan wrote:
> On Tue, 2006-10-17 at 09:22 +1000, David Chinner wrote:
> > On Mon, Oct 16, 2006 at 04:34:34PM -0500, Eric Sandeen wrote:
> > > Eric Sandeen wrote:
> > > > Timothy Shimmin wrote:
> > > >> Okay, started looking :-)
> > >
> > > One other thing, based on the bug on osdl today, some of these larger
> > > newly-static functions should probably be marked noinline to keep gcc
> > > from doing things we don't want it to...
> >
> > <grumble>
> >
> > This is not an obvious compiler hint (compared to, say, likely()) as
> > the functions gcc automatically inlines changes according to
> > compiler version, optimisation level and platform. Hence adding
> > noinline notation will be like playing whack-a-mole and I doubt it
> > will be consistently used or maintained moving forward. It's the
> > wrong solution, IMO.
> >
> > I think we should change the definition of STATIC so we don't have
> > to poison the code to work around some stupid compiler behaviour.
> > That is, unless we specifically say "inline" for static functions,
> > we really mean "noinline".
> So you are proposing?
> /* non-debug */
> #define STATIC static inline
> /* debug */
> #define STATIC noinline
No.
Currently STATIC is defined in fs/xfs/support/debug.h as:
#ifndef STATIC
#define STATIC static
#endif
I'm proposing that gets changed to:
> #define STATIC static noinline
This.
> That doesn't work either ...
> STATIC inline
> becomes
> static noline inline?
Fix them - inline functions in header files should always be "static
inline". Inline functions in .c files should always be static as
well - if they need to be accessed from different source files then
they need to be in header files. Hence "STATIC inline" is broken
code and should be fixed anyway. Luckily, there are very few of
these to fix and they are all in .c files:
chook 137% grep -rIw "STATIC inline" fs/xfs | wc -l
21
Cheers,
Dave.
--
Dave Chinner
Principal Engineer
SGI Australian Software Group
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] Make stuff static
2006-10-17 0:50 ` David Chinner
@ 2006-10-17 1:03 ` Eric Sandeen
2006-10-17 3:09 ` David Chinner
2006-10-18 0:56 ` David Chinner
2006-10-17 7:13 ` Tim Shimmin
1 sibling, 2 replies; 24+ messages in thread
From: Eric Sandeen @ 2006-10-17 1:03 UTC (permalink / raw)
To: David Chinner; +Cc: Russell Cattelan, Timothy Shimmin, xfs
David Chinner wrote:
>> So you are proposing?
>> /* non-debug */
>> #define STATIC static inline
>> /* debug */
>> #define STATIC noinline
>
> No.
>
> Currently STATIC is defined in fs/xfs/support/debug.h as:
>
> #ifndef STATIC
> #define STATIC static
> #endif
>
> I'm proposing that gets changed to:
>
>> #define STATIC static noinline
>
> This.
This sounds reasonable to me, David.
-Eric
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] Make stuff static
2006-10-17 1:03 ` Eric Sandeen
@ 2006-10-17 3:09 ` David Chinner
2006-10-17 3:18 ` Nathan Scott
2006-10-18 0:56 ` David Chinner
1 sibling, 1 reply; 24+ messages in thread
From: David Chinner @ 2006-10-17 3:09 UTC (permalink / raw)
To: Eric Sandeen; +Cc: David Chinner, Russell Cattelan, Timothy Shimmin, xfs
On Mon, Oct 16, 2006 at 08:03:47PM -0500, Eric Sandeen wrote:
> David Chinner wrote:
>
> >>So you are proposing?
> >>/* non-debug */
> >>#define STATIC static inline
> >>/* debug */
> >>#define STATIC noinline
> >
> >No.
> >
> >Currently STATIC is defined in fs/xfs/support/debug.h as:
> >
> >#ifndef STATIC
> >#define STATIC static
> >#endif
> >
> >I'm proposing that gets changed to:
> >
> >>#define STATIC static noinline
> >
> >This.
>
> This sounds reasonable to me, David.
Of course, no plan ever survives contact with the enemy. :/
STATIC is also used for defining various structure tables.
And variables. They'll all get changed to "static"....
Then there's __inline and __inline__ still used in places.....
The patch that follows does the noinline change, de-inlines
xfs_cleanup_inode() and cleans up all the inline cruft hanging around.
Comments?
Cheers,
Dave.
--
Dave Chinner
Principal Engineer
SGI Australian Software Group
---
fs/xfs/dmapi/xfs_dm.c | 6 +++---
fs/xfs/linux-2.4/mrlock.c | 2 +-
fs/xfs/linux-2.4/xfs_buf.c | 30 +++++++++++++++---------------
fs/xfs/linux-2.4/xfs_file.c | 8 ++++----
fs/xfs/linux-2.4/xfs_super.c | 14 +++++++-------
fs/xfs/linux-2.4/xfs_vnode.h | 4 ++--
fs/xfs/linux-2.6/xfs_aops.c | 2 +-
fs/xfs/linux-2.6/xfs_buf.c | 24 ++++++++++++------------
fs/xfs/linux-2.6/xfs_export.c | 2 +-
fs/xfs/linux-2.6/xfs_file.c | 8 ++++----
fs/xfs/linux-2.6/xfs_iops.c | 4 ++--
fs/xfs/linux-2.6/xfs_super.c | 16 ++++++++--------
fs/xfs/linux-2.6/xfs_sysctl.c | 6 +++---
fs/xfs/linux-2.6/xfs_vfs.c | 5 +++--
fs/xfs/linux-2.6/xfs_vnode.c | 2 +-
fs/xfs/linux-2.6/xfs_vnode.h | 4 ++--
fs/xfs/quota/xfs_dquot_item.c | 6 +++---
fs/xfs/quota/xfs_qm.c | 6 +++---
fs/xfs/quota/xfs_qm_bhv.c | 2 +-
fs/xfs/support/debug.h | 2 +-
fs/xfs/xfs_attr.c | 12 ++++++------
fs/xfs/xfs_attr_leaf.c | 6 +++---
fs/xfs/xfs_bit.c | 2 +-
fs/xfs/xfs_bmap_btree.c | 2 +-
fs/xfs/xfs_buf_item.c | 2 +-
fs/xfs/xfs_extfree_item.c | 4 ++--
fs/xfs/xfs_ialloc.c | 2 +-
fs/xfs/xfs_inode.c | 2 +-
fs/xfs/xfs_inode_item.c | 2 +-
fs/xfs/xfs_mount.c | 8 ++++----
fs/xfs/xfs_refcache.c | 10 +++++-----
31 files changed, 103 insertions(+), 102 deletions(-)
Index: 2.6.x-xfs-new/fs/xfs/linux-2.4/xfs_file.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.4/xfs_file.c 2006-08-31 16:17:46.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/linux-2.4/xfs_file.c 2006-10-17 11:09:44.450307346 +1000
@@ -55,7 +55,7 @@ static struct vm_operations_struct xfs_d
#define do_up_read(x)
#endif
-STATIC inline ssize_t
+static inline ssize_t
__xfs_file_read(
struct file *file,
char *buf,
@@ -99,7 +99,7 @@ xfs_file_read_invis(
}
-STATIC inline ssize_t
+static inline ssize_t
__xfs_file_write(
struct file *file,
const char *buf,
@@ -146,7 +146,7 @@ __xfs_file_write(
return rval;
}
-STATIC inline ssize_t
+static inline ssize_t
xfs_file_write(
struct file *file,
const char *buf,
@@ -156,7 +156,7 @@ xfs_file_write(
return __xfs_file_write(file, buf, 0, count, ppos);
}
-STATIC inline ssize_t
+static inline ssize_t
xfs_file_write_invis(
struct file *file,
const char *buf,
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_aops.c 2006-09-14 10:32:27.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_aops.c 2006-10-17 11:09:59.700324081 +1000
@@ -246,7 +246,7 @@ xfs_map_blocks(
return -error;
}
-STATIC inline int
+static inline int
xfs_iomap_valid(
xfs_iomap_t *iomapp,
loff_t offset)
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_buf.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_buf.c 2006-09-21 02:03:52.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_buf.c 2006-10-17 11:37:54.138236617 +1000
@@ -32,13 +32,13 @@
#include <linux/migrate.h>
#include "xfs_linux.h"
-STATIC kmem_zone_t *xfs_buf_zone;
-STATIC kmem_shaker_t xfs_buf_shake;
+static kmem_zone_t *xfs_buf_zone;
+static kmem_shaker_t xfs_buf_shake;
STATIC int xfsbufd(void *);
STATIC int xfsbufd_wakeup(int, gfp_t);
STATIC void xfs_buf_delwri_queue(xfs_buf_t *, int);
-STATIC struct workqueue_struct *xfslogd_workqueue;
+static struct workqueue_struct *xfslogd_workqueue;
struct workqueue_struct *xfsdatad_workqueue;
#ifdef XFS_BUF_TRACE
@@ -137,7 +137,7 @@ page_region_mask(
return mask;
}
-STATIC inline void
+static inline void
set_page_region(
struct page *page,
size_t offset,
@@ -149,7 +149,7 @@ set_page_region(
SetPageUptodate(page);
}
-STATIC inline int
+static inline int
test_page_region(
struct page *page,
size_t offset,
@@ -169,9 +169,9 @@ typedef struct a_list {
struct a_list *next;
} a_list_t;
-STATIC a_list_t *as_free_head;
-STATIC int as_list_len;
-STATIC DEFINE_SPINLOCK(as_lock);
+static a_list_t *as_free_head;
+static int as_list_len;
+static DEFINE_SPINLOCK(as_lock);
/*
* Try to batch vunmaps because they are costly.
@@ -1082,7 +1082,7 @@ xfs_buf_iostart(
return status;
}
-STATIC __inline__ int
+static inline int
_xfs_buf_iolocked(
xfs_buf_t *bp)
{
@@ -1092,7 +1092,7 @@ _xfs_buf_iolocked(
return 0;
}
-STATIC __inline__ void
+static inline void
_xfs_buf_ioend(
xfs_buf_t *bp,
int schedule)
@@ -1423,8 +1423,8 @@ xfs_free_bufhash(
/*
* buftarg list for delwrite queue processing
*/
-STATIC LIST_HEAD(xfs_buftarg_list);
-STATIC DEFINE_SPINLOCK(xfs_buftarg_lock);
+static LIST_HEAD(xfs_buftarg_list);
+static DEFINE_SPINLOCK(xfs_buftarg_lock);
STATIC void
xfs_register_buftarg(
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_file.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_file.c 2006-08-31 16:17:47.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_file.c 2006-10-17 11:10:59.100593910 +1000
@@ -46,7 +46,7 @@ static struct vm_operations_struct xfs_f
static struct vm_operations_struct xfs_dmapi_file_vm_ops;
#endif
-STATIC inline ssize_t
+static inline ssize_t
__xfs_file_read(
struct kiocb *iocb,
char __user *buf,
@@ -84,7 +84,7 @@ xfs_file_aio_read_invis(
return __xfs_file_read(iocb, buf, IO_ISAIO|IO_INVIS, count, pos);
}
-STATIC inline ssize_t
+static inline ssize_t
__xfs_file_write(
struct kiocb *iocb,
const char __user *buf,
@@ -123,7 +123,7 @@ xfs_file_aio_write_invis(
return __xfs_file_write(iocb, buf, IO_ISAIO|IO_INVIS, count, pos);
}
-STATIC inline ssize_t
+static inline ssize_t
__xfs_file_readv(
struct file *file,
const struct iovec *iov,
@@ -168,7 +168,7 @@ xfs_file_readv_invis(
return __xfs_file_readv(file, iov, IO_INVIS, nr_segs, ppos);
}
-STATIC inline ssize_t
+static inline ssize_t
__xfs_file_writev(
struct file *file,
const struct iovec *iov,
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_iops.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_iops.c 2006-08-31 16:17:47.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_iops.c 2006-10-17 11:10:36.575526218 +1000
@@ -250,13 +250,13 @@ xfs_init_security(
*
* XXX(hch): nfsd is broken, better fix it instead.
*/
-STATIC inline int
+static inline int
xfs_has_fs_struct(struct task_struct *task)
{
return (task->fs != init_task.fs);
}
-STATIC inline void
+STATIC void
xfs_cleanup_inode(
bhv_vnode_t *dvp,
bhv_vnode_t *vp,
Index: 2.6.x-xfs-new/fs/xfs/support/debug.h
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/support/debug.h 2006-08-31 16:17:48.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/support/debug.h 2006-10-17 11:08:58.820238266 +1000
@@ -44,7 +44,7 @@ extern unsigned long random(void);
#endif
#ifndef STATIC
-# define STATIC static
+# define STATIC static noinline
#endif
#endif /* __XFS_SUPPORT_DEBUG_H__ */
Index: 2.6.x-xfs-new/fs/xfs/xfs_attr_leaf.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_attr_leaf.c 2006-08-31 16:17:49.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/xfs_attr_leaf.c 2006-10-17 11:11:12.330871050 +1000
@@ -94,7 +94,7 @@ STATIC int xfs_attr_leaf_entsize(xfs_att
* Namespace helper routines
*========================================================================*/
-STATIC inline attrnames_t *
+static inline attrnames_t *
xfs_attr_flags_namesp(int flags)
{
return ((flags & XFS_ATTR_SECURE) ? &attr_secure:
@@ -105,7 +105,7 @@ xfs_attr_flags_namesp(int flags)
* If namespace bits don't match return 0.
* If all match then return 1.
*/
-STATIC inline int
+static inline int
xfs_attr_namesp_match(int arg_flags, int ondisk_flags)
{
return XFS_ATTR_NSP_ONDISK(ondisk_flags) == XFS_ATTR_NSP_ARGS_TO_ONDISK(arg_flags);
@@ -116,7 +116,7 @@ xfs_attr_namesp_match(int arg_flags, int
* then return 0.
* If all match or are overridable then return 1.
*/
-STATIC inline int
+static inline int
xfs_attr_namesp_match_overrides(int arg_flags, int ondisk_flags)
{
if (((arg_flags & ATTR_SECURE) == 0) !=
Index: 2.6.x-xfs-new/fs/xfs/xfs_mount.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_mount.c 2006-10-16 15:55:41.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/xfs_mount.c 2006-10-17 11:11:24.729256155 +1000
@@ -1790,7 +1790,7 @@ xfs_icsb_destroy_counters(
}
}
-STATIC inline void
+static inline void
xfs_icsb_lock_cntr(
xfs_icsb_cnts_t *icsbp)
{
@@ -1799,7 +1799,7 @@ xfs_icsb_lock_cntr(
}
}
-STATIC inline void
+static inline void
xfs_icsb_unlock_cntr(
xfs_icsb_cnts_t *icsbp)
{
@@ -1807,7 +1807,7 @@ xfs_icsb_unlock_cntr(
}
-STATIC inline void
+static inline void
xfs_icsb_lock_all_counters(
xfs_mount_t *mp)
{
@@ -1820,7 +1820,7 @@ xfs_icsb_lock_all_counters(
}
}
-STATIC inline void
+static inline void
xfs_icsb_unlock_all_counters(
xfs_mount_t *mp)
{
Index: 2.6.x-xfs-new/fs/xfs/linux-2.4/xfs_buf.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.4/xfs_buf.c 2006-08-31 16:17:46.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/linux-2.4/xfs_buf.c 2006-10-17 11:51:06.258966560 +1000
@@ -67,17 +67,17 @@
#define VM_MAP VM_ALLOC
#endif
-STATIC kmem_zone_t *xfs_buf_zone;
-STATIC kmem_shaker_t xfs_buf_shake;
+static kmem_zone_t *xfs_buf_zone;
+static kmem_shaker_t xfs_buf_shake;
#define MAX_IO_DAEMONS NR_CPUS
#define CPU_TO_DAEMON(cpu) (cpu)
-STATIC int xb_logio_daemons[MAX_IO_DAEMONS];
-STATIC struct list_head xfs_buf_logiodone_tq[MAX_IO_DAEMONS];
-STATIC wait_queue_head_t xfs_buf_logiodone_wait[MAX_IO_DAEMONS];
-STATIC int xb_dataio_daemons[MAX_IO_DAEMONS];
-STATIC struct list_head xfs_buf_dataiodone_tq[MAX_IO_DAEMONS];
-STATIC wait_queue_head_t xfs_buf_dataiodone_wait[MAX_IO_DAEMONS];
+static int xb_logio_daemons[MAX_IO_DAEMONS];
+static struct list_head xfs_buf_logiodone_tq[MAX_IO_DAEMONS];
+static wait_queue_head_t xfs_buf_logiodone_wait[MAX_IO_DAEMONS];
+static int xb_dataio_daemons[MAX_IO_DAEMONS];
+static struct list_head xfs_buf_dataiodone_tq[MAX_IO_DAEMONS];
+static wait_queue_head_t xfs_buf_dataiodone_wait[MAX_IO_DAEMONS];
/*
* For pre-allocated buffer head pool
@@ -154,9 +154,9 @@ typedef struct a_list {
struct a_list *next;
} a_list_t;
-STATIC a_list_t *as_free_head;
-STATIC int as_list_len;
-STATIC DEFINE_SPINLOCK(as_lock);
+static a_list_t *as_free_head;
+static int as_list_len;
+static DEFINE_SPINLOCK(as_lock);
/*
* Try to batch vunmaps because they are costly.
@@ -515,7 +515,7 @@ _xfs_buf_get_prealloc_bh(void)
* Otherwise, put it back in the pool, and wake up anybody
* waiting for one.
*/
-STATIC inline void
+static inline void
_xfs_buf_free_bh(
struct buffer_head *bh)
{
@@ -1204,7 +1204,7 @@ xfs_buf_iostart(
return status;
}
-STATIC __inline__ int
+static inline int
_xfs_buf_iolocked(
xfs_buf_t *bp)
{
@@ -1366,8 +1366,8 @@ xfs_free_bufhash(
/*
* buftarg list for delwrite queue processing
*/
-STATIC LIST_HEAD(xfs_buftarg_list);
-STATIC DEFINE_SPINLOCK(xfs_buftarg_lock);
+static LIST_HEAD(xfs_buftarg_list);
+static DEFINE_SPINLOCK(xfs_buftarg_lock);
STATIC void
xfs_register_buftarg(
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_sysctl.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_sysctl.c 2006-08-31 16:17:47.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_sysctl.c 2006-10-17 11:18:27.375821188 +1000
@@ -54,7 +54,7 @@ xfs_stats_clear_proc_handler(
}
#endif /* CONFIG_PROC_FS */
-STATIC ctl_table xfs_table[] = {
+static ctl_table xfs_table[] = {
{XFS_RESTRICT_CHOWN, "restrict_chown", &xfs_params.restrict_chown.val,
sizeof(int), 0644, NULL, &proc_dointvec_minmax,
&sysctl_intvec, NULL,
@@ -151,12 +151,12 @@ STATIC ctl_table xfs_table[] = {
{0}
};
-STATIC ctl_table xfs_dir_table[] = {
+static ctl_table xfs_dir_table[] = {
{FS_XFS, "xfs", NULL, 0, 0555, xfs_table},
{0}
};
-STATIC ctl_table xfs_root_table[] = {
+static ctl_table xfs_root_table[] = {
{CTL_FS, "fs", NULL, 0, 0555, xfs_dir_table},
{0}
};
Index: 2.6.x-xfs-new/fs/xfs/xfs_attr.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_attr.c 2006-08-31 16:17:49.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/xfs_attr.c 2006-10-17 11:20:13.991172718 +1000
@@ -57,9 +57,9 @@
*/
#define ATTR_SYSCOUNT 2
-STATIC struct attrnames posix_acl_access;
-STATIC struct attrnames posix_acl_default;
-STATIC struct attrnames *attr_system_names[ATTR_SYSCOUNT];
+static struct attrnames posix_acl_access;
+static struct attrnames posix_acl_default;
+static struct attrnames *attr_system_names[ATTR_SYSCOUNT];
/*========================================================================
* Function prototypes for the kernel.
@@ -2477,7 +2477,7 @@ posix_acl_default_exists(
return xfs_acl_vhasacl_default(vp);
}
-STATIC struct attrnames posix_acl_access = {
+static struct attrnames posix_acl_access = {
.attr_name = "posix_acl_access",
.attr_namelen = sizeof("posix_acl_access") - 1,
.attr_get = posix_acl_access_get,
@@ -2486,7 +2486,7 @@ STATIC struct attrnames posix_acl_access
.attr_exists = posix_acl_access_exists,
};
-STATIC struct attrnames posix_acl_default = {
+static struct attrnames posix_acl_default = {
.attr_name = "posix_acl_default",
.attr_namelen = sizeof("posix_acl_default") - 1,
.attr_get = posix_acl_default_get,
@@ -2495,7 +2495,7 @@ STATIC struct attrnames posix_acl_defaul
.attr_exists = posix_acl_default_exists,
};
-STATIC struct attrnames *attr_system_names[] =
+static struct attrnames *attr_system_names[] =
{ &posix_acl_access, &posix_acl_default };
Index: 2.6.x-xfs-new/fs/xfs/xfs_bit.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_bit.c 2006-08-31 16:17:49.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/xfs_bit.c 2006-10-17 11:18:34.850938070 +1000
@@ -29,7 +29,7 @@
/*
* Index of high bit number in byte, -1 for none set, 0..7 otherwise.
*/
-STATIC const char xfs_highbit[256] = {
+static const char xfs_highbit[256] = {
-1, 0, 1, 1, 2, 2, 2, 2, /* 00 .. 07 */
3, 3, 3, 3, 3, 3, 3, 3, /* 08 .. 0f */
4, 4, 4, 4, 4, 4, 4, 4, /* 10 .. 17 */
Index: 2.6.x-xfs-new/fs/xfs/linux-2.4/mrlock.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.4/mrlock.c 2006-08-31 16:17:46.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/linux-2.4/mrlock.c 2006-10-17 11:24:43.086771964 +1000
@@ -195,7 +195,7 @@ mrtryupdate(mrlock_t *mrp)
return 1;
}
-static __inline__ void mrwake(mrlock_t *mrp)
+static inline void mrwake(mrlock_t *mrp)
{
/*
* First, if the count is now 0, we need to wake-up anyone waiting.
Index: 2.6.x-xfs-new/fs/xfs/linux-2.4/xfs_super.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.4/xfs_super.c 2006-08-31 16:17:46.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/linux-2.4/xfs_super.c 2006-10-17 11:47:23.279919586 +1000
@@ -54,9 +54,9 @@
#include <linux/init.h>
-STATIC struct quotactl_ops xfs_quotactl_operations;
-STATIC struct super_operations xfs_super_operations;
-STATIC kmem_zone_t *xfs_vnode_zone;
+static struct quotactl_ops xfs_quotactl_operations;
+static struct super_operations xfs_super_operations;
+static kmem_zone_t *xfs_vnode_zone;
STATIC struct xfs_mount_args *
xfs_args_allocate(
@@ -113,7 +113,7 @@ xfs_max_file_offset(
return (((__uint64_t)pagefactor) << bitshift) - 1;
}
-STATIC __inline__ void
+static inline void
xfs_set_inodeops(
struct inode *inode)
{
@@ -140,7 +140,7 @@ xfs_set_inodeops(
}
}
-STATIC __inline__ void
+static inline void
xfs_revalidate_inode(
xfs_mount_t *mp,
bhv_vnode_t *vp,
@@ -974,7 +974,7 @@ fail_vfsop:
}
-STATIC struct super_operations xfs_super_operations = {
+static struct super_operations xfs_super_operations = {
.alloc_inode = xfs_fs_alloc_inode,
.destroy_inode = xfs_fs_destroy_inode,
.write_inode = xfs_fs_write_inode,
@@ -991,7 +991,7 @@ STATIC struct super_operations xfs_super
.show_options = xfs_fs_show_options,
};
-STATIC struct quotactl_ops xfs_quotactl_operations = {
+static struct quotactl_ops xfs_quotactl_operations = {
.quota_sync = xfs_fs_quotasync,
.get_xstate = xfs_fs_getxstate,
.set_xstate = xfs_fs_setxstate,
Index: 2.6.x-xfs-new/fs/xfs/linux-2.4/xfs_vnode.h
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.4/xfs_vnode.h 2006-08-31 16:17:47.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/linux-2.4/xfs_vnode.h 2006-10-17 11:24:22.593262148 +1000
@@ -468,14 +468,14 @@ static inline struct bhv_vnode *vn_grab(
#define VN_LOCK(vp) mutex_spinlock(&(vp)->v_lock)
#define VN_UNLOCK(vp, s) mutex_spinunlock(&(vp)->v_lock, s)
-static __inline__ void vn_flagset(struct bhv_vnode *vp, uint flag)
+static inline void vn_flagset(struct bhv_vnode *vp, uint flag)
{
spin_lock(&vp->v_lock);
vp->v_flag |= flag;
spin_unlock(&vp->v_lock);
}
-static __inline__ uint vn_flagclr(struct bhv_vnode *vp, uint flag)
+static inline uint vn_flagclr(struct bhv_vnode *vp, uint flag)
{
uint cleared;
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_super.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_super.c 2006-10-17 09:51:46.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_super.c 2006-10-17 11:39:54.900738643 +1000
@@ -57,10 +57,10 @@
#include <linux/writeback.h>
#include <linux/kthread.h>
-STATIC struct quotactl_ops xfs_quotactl_operations;
-STATIC struct super_operations xfs_super_operations;
-STATIC kmem_zone_t *xfs_vnode_zone;
-STATIC kmem_zone_t *xfs_ioend_zone;
+static struct quotactl_ops xfs_quotactl_operations;
+static struct super_operations xfs_super_operations;
+static kmem_zone_t *xfs_vnode_zone;
+static kmem_zone_t *xfs_ioend_zone;
mempool_t *xfs_ioend_pool;
STATIC struct xfs_mount_args *
@@ -120,7 +120,7 @@ xfs_max_file_offset(
return (((__uint64_t)pagefactor) << bitshift) - 1;
}
-STATIC __inline__ void
+static inline void
xfs_set_inodeops(
struct inode *inode)
{
@@ -146,7 +146,7 @@ xfs_set_inodeops(
}
}
-STATIC __inline__ void
+static inline void
xfs_revalidate_inode(
xfs_mount_t *mp,
bhv_vnode_t *vp,
@@ -879,7 +879,7 @@ xfs_fs_get_sb(
return get_sb_bdev(fs_type, flags, dev_name, data, xfs_fs_fill_super);
}
-STATIC struct super_operations xfs_super_operations = {
+static struct super_operations xfs_super_operations = {
.alloc_inode = xfs_fs_alloc_inode,
.destroy_inode = xfs_fs_destroy_inode,
.write_inode = xfs_fs_write_inode,
@@ -893,7 +893,7 @@ STATIC struct super_operations xfs_super
.show_options = xfs_fs_show_options,
};
-STATIC struct quotactl_ops xfs_quotactl_operations = {
+static struct quotactl_ops xfs_quotactl_operations = {
.quota_sync = xfs_fs_quotasync,
.get_xstate = xfs_fs_getxstate,
.set_xstate = xfs_fs_setxstate,
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_vnode.h
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_vnode.h 2006-08-31 16:17:48.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_vnode.h 2006-10-17 11:26:09.196480962 +1000
@@ -492,14 +492,14 @@ static inline struct bhv_vnode *vn_grab(
#define VN_LOCK(vp) mutex_spinlock(&(vp)->v_lock)
#define VN_UNLOCK(vp, s) mutex_spinunlock(&(vp)->v_lock, s)
-static __inline__ void vn_flagset(struct bhv_vnode *vp, uint flag)
+static inline void vn_flagset(struct bhv_vnode *vp, uint flag)
{
spin_lock(&vp->v_lock);
vp->v_flag |= flag;
spin_unlock(&vp->v_lock);
}
-static __inline__ uint vn_flagclr(struct bhv_vnode *vp, uint flag)
+static inline uint vn_flagclr(struct bhv_vnode *vp, uint flag)
{
uint cleared;
Index: 2.6.x-xfs-new/fs/xfs/xfs_bmap_btree.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_bmap_btree.c 2006-08-31 16:17:49.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/xfs_bmap_btree.c 2006-10-17 11:21:05.161062724 +1000
@@ -1862,7 +1862,7 @@ xfs_bmbt_delete(
* xfs_bmbt_get_startblock, xfs_bmbt_get_blockcount and xfs_bmbt_get_state.
*/
-STATIC __inline__ void
+static inline void
__xfs_bmbt_get_all(
__uint64_t l0,
__uint64_t l1,
Index: 2.6.x-xfs-new/fs/xfs/xfs_extfree_item.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_extfree_item.c 2006-08-31 16:17:51.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/xfs_extfree_item.c 2006-10-17 11:21:26.802471146 +1000
@@ -227,7 +227,7 @@ xfs_efi_item_committing(xfs_efi_log_item
/*
* This is the ops vector shared by all efi log items.
*/
-STATIC struct xfs_item_ops xfs_efi_item_ops = {
+static struct xfs_item_ops xfs_efi_item_ops = {
.iop_size = (uint(*)(xfs_log_item_t*))xfs_efi_item_size,
.iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*))
xfs_efi_item_format,
@@ -525,7 +525,7 @@ xfs_efd_item_committing(xfs_efd_log_item
/*
* This is the ops vector shared by all efd log items.
*/
-STATIC struct xfs_item_ops xfs_efd_item_ops = {
+static struct xfs_item_ops xfs_efd_item_ops = {
.iop_size = (uint(*)(xfs_log_item_t*))xfs_efd_item_size,
.iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*))
xfs_efd_item_format,
Index: 2.6.x-xfs-new/fs/xfs/xfs_inode.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_inode.c 2006-10-17 10:03:19.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/xfs_inode.c 2006-10-17 11:27:02.810045286 +1000
@@ -2125,7 +2125,7 @@ xfs_iunlink_remove(
return 0;
}
-static __inline__ int xfs_inode_clean(xfs_inode_t *ip)
+static inline int xfs_inode_clean(xfs_inode_t *ip)
{
return (((ip->i_itemp == NULL) ||
!(ip->i_itemp->ili_format.ilf_fields & XFS_ILOG_ALL)) &&
Index: 2.6.x-xfs-new/fs/xfs/xfs_buf_item.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_buf_item.c 2006-08-31 16:17:50.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/xfs_buf_item.c 2006-10-17 11:30:07.555675393 +1000
@@ -660,7 +660,7 @@ xfs_buf_item_committing(xfs_buf_log_item
/*
* This is the ops vector shared by all buf log items.
*/
-STATIC struct xfs_item_ops xfs_buf_item_ops = {
+static struct xfs_item_ops xfs_buf_item_ops = {
.iop_size = (uint(*)(xfs_log_item_t*))xfs_buf_item_size,
.iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*))
xfs_buf_item_format,
Index: 2.6.x-xfs-new/fs/xfs/xfs_ialloc.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_ialloc.c 2006-10-13 12:14:02.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/xfs_ialloc.c 2006-10-17 11:30:42.475414928 +1000
@@ -342,7 +342,7 @@ xfs_ialloc_ag_alloc(
return 0;
}
-STATIC __inline xfs_agnumber_t
+static inline xfs_agnumber_t
xfs_ialloc_next_ag(
xfs_mount_t *mp)
{
Index: 2.6.x-xfs-new/fs/xfs/xfs_inode_item.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_inode_item.c 2006-08-31 16:17:51.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/xfs_inode_item.c 2006-10-17 11:31:03.768812145 +1000
@@ -887,7 +887,7 @@ xfs_inode_item_committing(
/*
* This is the ops vector shared by all buf log items.
*/
-STATIC struct xfs_item_ops xfs_inode_item_ops = {
+static struct xfs_item_ops xfs_inode_item_ops = {
.iop_size = (uint(*)(xfs_log_item_t*))xfs_inode_item_size,
.iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*))
xfs_inode_item_format,
Index: 2.6.x-xfs-new/fs/xfs/dmapi/xfs_dm.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/dmapi/xfs_dm.c 2006-10-11 13:36:22.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/dmapi/xfs_dm.c 2006-10-17 11:41:38.844845462 +1000
@@ -133,9 +133,9 @@ typedef struct {
changed!
*/
-STATIC const char dmattr_prefix[DMATTR_PREFIXLEN + 1] = DMATTR_PREFIXSTRING;
+static const char dmattr_prefix[DMATTR_PREFIXLEN + 1] = DMATTR_PREFIXSTRING;
-STATIC dm_size_t dm_min_dio_xfer = 0; /* direct I/O disabled for now */
+static dm_size_t dm_min_dio_xfer = 0; /* direct I/O disabled for now */
/* See xfs_dm_get_dmattr() for a description of why this is needed. */
@@ -3123,7 +3123,7 @@ xfs_dm_obj_ref_hold(
}
-STATIC fsys_function_vector_t xfs_fsys_vector[DM_FSYS_MAX];
+static fsys_function_vector_t xfs_fsys_vector[DM_FSYS_MAX];
int
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_export.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_export.c 2006-08-31 16:17:47.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_export.c 2006-10-17 11:33:53.324543389 +1000
@@ -24,7 +24,7 @@
#include "xfs_mount.h"
#include "xfs_export.h"
-STATIC struct dentry dotdot = { .d_name.name = "..", .d_name.len = 2, };
+static struct dentry dotdot = { .d_name.name = "..", .d_name.len = 2, };
/*
* XFS encodes and decodes the fileid portion of NFS filehandles
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_vfs.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_vfs.c 2006-08-31 16:17:47.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_vfs.c 2006-10-17 11:40:38.795745616 +1000
@@ -295,8 +295,9 @@ typedef struct bhv_module_list {
const char * bm_name;
void * bm_ops;
} bhv_module_list_t;
-STATIC DEFINE_SPINLOCK(bhv_lock);
-STATIC struct list_head bhv_list = LIST_HEAD_INIT(bhv_list);
+
+static DEFINE_SPINLOCK(bhv_lock);
+static struct list_head bhv_list = LIST_HEAD_INIT(bhv_list);
void
bhv_module_init(
Index: 2.6.x-xfs-new/fs/xfs/quota/xfs_dquot_item.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/quota/xfs_dquot_item.c 2006-08-31 16:17:48.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/quota/xfs_dquot_item.c 2006-10-17 11:41:59.366469646 +1000
@@ -399,7 +399,7 @@ xfs_qm_dquot_logitem_committing(
/*
* This is the ops vector for dquots
*/
-STATIC struct xfs_item_ops xfs_dquot_item_ops = {
+static struct xfs_item_ops xfs_dquot_item_ops = {
.iop_size = (uint(*)(xfs_log_item_t*))xfs_qm_dquot_logitem_size,
.iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*))
xfs_qm_dquot_logitem_format,
@@ -606,7 +606,7 @@ xfs_qm_qoffend_logitem_committing(xfs_qo
return;
}
-STATIC struct xfs_item_ops xfs_qm_qoffend_logitem_ops = {
+static struct xfs_item_ops xfs_qm_qoffend_logitem_ops = {
.iop_size = (uint(*)(xfs_log_item_t*))xfs_qm_qoff_logitem_size,
.iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*))
xfs_qm_qoff_logitem_format,
@@ -628,7 +628,7 @@ STATIC struct xfs_item_ops xfs_qm_qoffen
/*
* This is the ops vector shared by all quotaoff-start log items.
*/
-STATIC struct xfs_item_ops xfs_qm_qoff_logitem_ops = {
+static struct xfs_item_ops xfs_qm_qoff_logitem_ops = {
.iop_size = (uint(*)(xfs_log_item_t*))xfs_qm_qoff_logitem_size,
.iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*))
xfs_qm_qoff_logitem_format,
Index: 2.6.x-xfs-new/fs/xfs/quota/xfs_qm.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/quota/xfs_qm.c 2006-09-14 10:32:27.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/quota/xfs_qm.c 2006-10-17 11:39:02.158678202 +1000
@@ -64,10 +64,10 @@ uint ndquot;
kmem_zone_t *qm_dqzone;
kmem_zone_t *qm_dqtrxzone;
-STATIC kmem_shaker_t xfs_qm_shaker;
+static kmem_shaker_t xfs_qm_shaker;
-STATIC cred_t xfs_zerocr;
-STATIC xfs_inode_t xfs_zeroino;
+static cred_t xfs_zerocr;
+static xfs_inode_t xfs_zeroino;
STATIC void xfs_qm_list_init(xfs_dqlist_t *, char *, int);
STATIC void xfs_qm_list_destroy(xfs_dqlist_t *);
Index: 2.6.x-xfs-new/fs/xfs/quota/xfs_qm_bhv.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/quota/xfs_qm_bhv.c 2006-09-14 10:32:27.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/quota/xfs_qm_bhv.c 2006-10-17 11:36:09.341855439 +1000
@@ -384,7 +384,7 @@ xfs_qm_dqrele_null(
}
-STATIC struct xfs_qmops xfs_qmcore_xfs = {
+static struct xfs_qmops xfs_qmcore_xfs = {
.xfs_qminit = xfs_qm_newmount,
.xfs_qmdone = xfs_qm_unmount_quotadestroy,
.xfs_qmmount = xfs_qm_endmount,
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_vnode.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_vnode.c 2006-08-31 16:17:48.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_vnode.c 2006-10-17 11:51:13.890110076 +1000
@@ -26,7 +26,7 @@ DEFINE_SPINLOCK(vnumber_lock);
*/
#define NVSYNC 37
#define vptosync(v) (&vsync[((unsigned long)v) % NVSYNC])
-STATIC wait_queue_head_t vsync[NVSYNC];
+static wait_queue_head_t vsync[NVSYNC];
void
vn_init(void)
Index: 2.6.x-xfs-new/fs/xfs/xfs_refcache.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_refcache.c 2006-08-31 16:17:52.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/xfs_refcache.c 2006-10-17 11:50:41.145766729 +1000
@@ -45,11 +45,11 @@
#include "xfs_buf_item.h"
#include "xfs_refcache.h"
-STATIC spinlock_t xfs_refcache_lock = SPIN_LOCK_UNLOCKED;
-STATIC xfs_inode_t **xfs_refcache;
-STATIC int xfs_refcache_index;
-STATIC int xfs_refcache_busy;
-STATIC int xfs_refcache_count;
+static spinlock_t xfs_refcache_lock = SPIN_LOCK_UNLOCKED;
+static xfs_inode_t **xfs_refcache;
+static int xfs_refcache_index;
+static int xfs_refcache_busy;
+static int xfs_refcache_count;
/*
* Insert the given inode into the reference cache.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 1/2] Make stuff static
2006-10-17 3:09 ` David Chinner
@ 2006-10-17 3:18 ` Nathan Scott
0 siblings, 0 replies; 24+ messages in thread
From: Nathan Scott @ 2006-10-17 3:18 UTC (permalink / raw)
To: David Chinner; +Cc: Eric Sandeen, Russell Cattelan, Timothy Shimmin, xfs
On Tue, 2006-10-17 at 13:09 +1000, David Chinner wrote:
> On Mon, Oct 16, 2006 at 08:03:47PM -0500, Eric Sandeen wrote:
> > David Chinner wrote:
> >
> Of course, no plan ever survives contact with the enemy. :/
>
> STATIC is also used for defining various structure tables.
> And variables. They'll all get changed to "static"....
>
> Then there's __inline and __inline__ still used in places.....
>
> The patch that follows does the noinline change, de-inlines
> xfs_cleanup_inode() and cleans up all the inline cruft hanging around.
>
> Comments?
>
Be careful of userspace, where much of this code is also compiled in
libxfs, and STATIC is always #defined away to nothingness there (as
there's need for some routines to be accessed outside of one source
file there, differently to the kernel, IIRC).
cheers.
--
Nathan
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] Make stuff static
2006-10-17 1:03 ` Eric Sandeen
2006-10-17 3:09 ` David Chinner
@ 2006-10-18 0:56 ` David Chinner
1 sibling, 0 replies; 24+ messages in thread
From: David Chinner @ 2006-10-18 0:56 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Russell Cattelan, Timothy Shimmin, xfs
On Mon, Oct 16, 2006 at 08:03:47PM -0500, Eric Sandeen wrote:
> David Chinner wrote:
>
> >>So you are proposing?
> >>/* non-debug */
> >>#define STATIC static inline
> >>/* debug */
> >>#define STATIC noinline
> >
> >No.
> >
> >Currently STATIC is defined in fs/xfs/support/debug.h as:
> >
> >#ifndef STATIC
> >#define STATIC static
> >#endif
> >
> >I'm proposing that gets changed to:
> >
> >>#define STATIC static noinline
This has a pretty big effect on stack usage. These numbers are from
an x86_64 build. A negative diff means the function uses more stack
space than before (probably a function that was getting inlined):
Function OLD NEW DIFF
------------------------ --- --- ----
xfs_vn_mknod 616 200 416
xfs_ioctl 440 <100 340
xfs_inobt_insert 328 <100 228
xfs_bmbt_insert 312 <100 212
xfs_vn_symlink 376 168 208
xfs_dialloc 360 152 208
xfs_bmapi 552 344 208
xfs_symlink 280 <100 180
xfs_change_file_space 376 200 176
xfs_write 264 <100 164
xfs_bulkstat 264 <100 164
xfs_page_state_convert 344 184 160
xfs_bunmapi 376 216 160
xfs_ifree 248 <100 148
xfs_bmap_add_extent 280 136 144
xfs_getbmap 232 <100 132
xfs_attr_remove 232 <100 132
xfs_iomap_write_direct 216 <100 116
xfs_fs_cmn_err 216 <100 116
xfs_bmbt_delete 216 <100 116
xfs_bmap_add_attrfork 216 <100 116
xfs_attr_set 216 <100 116
xfs_cmn_err 208 <100 108
xfs_iomap_write_allocate 200 <100 100
xfs_inactive 200 <100 100
xfs_bmap_extents_to_btree 200 <100 100
xfs_alloc_insert 200 <100 100
xfs_setattr 184 <100 84
xfs_mkdir 184 <100 84
xfs_inobt_delete 184 <100 84
xfs_dir2_grow_inode 184 <100 84
xfs_create 184 <100 84
xfs_bmap_add_extent_unwritten_real 184 <100 84
xfs_alloc_file_space 184 <100 84
xfs_alloc_delete 184 <100 84
xfs_qm_dqtobp 248 168 80
xfs_vn_unlink 168 <100 68
xfs_vn_setattr 168 <100 68
xfs_vn_rmdir 168 <100 68
xfs_vn_rename 168 <100 68
xfs_rtallocate_extent 168 <100 68
xfs_probe_cluster 168 <100 68
xfs_da_do_buf 168 <100 68
xfs_attr_shortform_to_leaf 168 <100 68
xfs_alloc_fix_freelist 168 <100 68
xfs_acl_allow_set 168 <100 68
xfs_attrmulti_by_handle 168 104 64
xfs_vn_link 160 <100 60
xfs_acl_vtoacl 160 <100 60
xfs_acl_vget 160 <100 60
xfs_acl_setmode 160 <100 60
xfs_readlink 152 <100 52
xfs_link 152 <100 52
xfs_iomap_write_unwritten 152 <100 52
xfs_growfs_rt_alloc 152 <100 52
xfs_dir_replace 152 <100 52
xfs_dir_removename 152 <100 52
xfs_dir_lookup 152 <100 52
xfs_dir_createname 152 <100 52
xfs_dir2_leaf_getdents 152 <100 52
xfs_da_shrink_inode 152 <100 52
xfs_da_grow_inode 152 <100 52
xfs_corruption_error 152 <100 52
xfs_zero_eof 168 120 48
xfs_growfs_rt 168 120 48
xfs_trans_init 144 <100 44
xfs_dir_canenter 144 <100 44
xfs_remove 136 <100 36
xfs_growfs_data 136 <100 36
xfs_fs_fill_super 136 <100 36
xfs_dir_init 136 <100 36
xfs_dir2_node_addname 136 <100 36
xfs_bmbt_newroot 136 <100 36
xfs_bmap_local_to_extents 136 <100 36
xfs_attr_rmtval_set 136 <100 36
xfs_attr_rmtval_get 136 <100 36
xfs_attr_leaf_to_shortform 136 <100 36
xfs_attr_leaf_inactive 136 <100 36
xfs_iomap_write_delay 296 264 32
xfs_zero_remaining_bytes 120 <100 20
xfs_trans_unreserve_and_mod_sb 120 <100 20
xfs_rmdir 120 <100 20
xfs_log_unmount_write 120 <100 20
xfs_free_extent 120 <100 20
xfs_file_readdir 120 <100 20
xfs_dir_ialloc 120 <100 20
xfs_dir2_sf_to_block 120 <100 20
xfs_dir2_block_getdents 120 <100 20
xfs_difree 120 <100 20
xfs_bmap_add_extent_hole_real 120 <100 20
xfs_alloc_rshift 120 <100 20
_xfs_trans_commit 328 312 16
xfs_rename 248 232 16
xfs_qm_dqiterate 136 120 16
xfs_mountfs 136 120 16
xfs_iomap 152 136 16
xfs_attr_fetch 168 152 16
init_xfs_fs 112 <100 12
xfs_vm_direct_IO 104 <100 4
xfs_itruncate_finish 104 <100 4
xfs_inumbers 104 <100 4
xfs_inobt_rshift 104 <100 4
xfs_iflush 104 <100 4
xfs_dir2_node_removename 104 <100 4
xfs_dir2_leafn_lookup_int 104 <100 4
xfs_dir2_leaf_addname 104 <100 4
xfs_dilocate 104 <100 4
xfs_bmap_last_before 104 <100 4
xfs_attr_leaf_split 104 <100 4
xfs_rtallocate_extent_size <100 104 -4
xfs_fssetdm_by_handle <100 104 -4
xfs_bmap_add_extent_hole_delay <100 104 -4
xfs_attr_leaf_freextent <100 104 -4
xfs_zero_last_block <100 120 -20
xfs_rtallocate_extent_near <100 120 -20
xfs_ioc_fsgeometry_v1 <100 120 -20
xfs_ioc_fsgeometry <100 120 -20
xfs_attrlist_by_handle <100 120 -20
xfs_alloc_insrec <100 120 -20
xfs_qm_dqalloc <100 136 -36
xfs_inobt_insrec <100 136 -36
xfs_hex_dump <100 136 -36
xfs_bmap_add_attrfork_local <100 136 -36
xfs_readlink_by_handle <100 152 -52
xfs_bmbt_insrec <100 152 -52
xfs_inactive_symlink_rmt <100 168 -68
xfs_cluster_write <100 168 -68
xfs_alloc_delrec <100 168 -68
xfs_ialloc_ag_alloc <100 184 -84
xfs_attr_remove_int <100 184 -84
xfs_inobt_split <100 200 -100
xfs_inobt_newroot <100 200 -100
xfs_inobt_delrec <100 200 -100
xfs_free_file_space <100 200 -100
xfs_bmbt_split <100 200 -100
xfs_bmbt_delrec <100 200 -100
xfs_bmap_add_extent_delay_real <100 200 -100
xfs_attr_set_int <100 200 -100
xfs_cleanup_inode <100 208 -108
xfs_bmap_del_extent <100 216 -116
xfs_bmap_btalloc <100 216 -116
xfs_find_handle <100 248 -148
So this may help a lot with stack usage, but what it does to performance
will be an interesting question....
Cheers,
Dave.
--
Dave Chinner
Principal Engineer
SGI Australian Software Group
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] Make stuff static
2006-10-17 0:50 ` David Chinner
2006-10-17 1:03 ` Eric Sandeen
@ 2006-10-17 7:13 ` Tim Shimmin
2006-10-17 21:57 ` David Chinner
1 sibling, 1 reply; 24+ messages in thread
From: Tim Shimmin @ 2006-10-17 7:13 UTC (permalink / raw)
To: David Chinner, Russell Cattelan; +Cc: Eric Sandeen, xfs
--On 17 October 2006 10:50:38 AM +1000 David Chinner <dgc@sgi.com> wrote:
> On Mon, Oct 16, 2006 at 06:55:43PM -0500, Russell Cattelan wrote:
>> On Tue, 2006-10-17 at 09:22 +1000, David Chinner wrote:
>> > On Mon, Oct 16, 2006 at 04:34:34PM -0500, Eric Sandeen wrote:
>> > > Eric Sandeen wrote:
>> > > > Timothy Shimmin wrote:
>> > > >> Okay, started looking :-)
>> > >
>> > > One other thing, based on the bug on osdl today, some of these larger
>> > > newly-static functions should probably be marked noinline to keep gcc
>> > > from doing things we don't want it to...
>> >
>> > <grumble>
>> >
>> > This is not an obvious compiler hint (compared to, say, likely()) as
>> > the functions gcc automatically inlines changes according to
>> > compiler version, optimisation level and platform. Hence adding
>> > noinline notation will be like playing whack-a-mole and I doubt it
>> > will be consistently used or maintained moving forward. It's the
>> > wrong solution, IMO.
>> >
>> > I think we should change the definition of STATIC so we don't have
>> > to poison the code to work around some stupid compiler behaviour.
>> > That is, unless we specifically say "inline" for static functions,
>> > we really mean "noinline".
>
>> So you are proposing?
>> /* non-debug */
>> # define STATIC static inline
>> /* debug */
>> # define STATIC noinline
>
> No.
>
> Currently STATIC is defined in fs/xfs/support/debug.h as:
>
># ifndef STATIC
># define STATIC static
># endif
>
> I'm proposing that gets changed to:
>
>> # define STATIC static noinline
>
> This.
>
>> That doesn't work either ...
>> STATIC inline
>> becomes
>> static noline inline?
>
> Fix them - inline functions in header files should always be "static
> inline". Inline functions in .c files should always be static as
> well - if they need to be accessed from different source files then
> they need to be in header files. Hence "STATIC inline" is broken
> code and should be fixed anyway. Luckily, there are very few of
> these to fix and they are all in .c files:
>
> chook 137% grep -rIw "STATIC inline" fs/xfs | wc -l
> 21
>
So you are saying that "static inline"s should always be.
So for CONFIG_XFS_DEBUG where we define STATIC and so make static
disappear for uses of STATIC, we will no longer touch these
"static inline" functions.
I thought that for debug, we could stop them from being inline
for easier debugging. We could have a STATIC_INLINE :-)
--Tim
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] Make stuff static
2006-10-17 7:13 ` Tim Shimmin
@ 2006-10-17 21:57 ` David Chinner
2006-10-17 22:45 ` Russell Cattelan
2006-10-18 4:06 ` Timothy Shimmin
0 siblings, 2 replies; 24+ messages in thread
From: David Chinner @ 2006-10-17 21:57 UTC (permalink / raw)
To: Tim Shimmin; +Cc: David Chinner, Russell Cattelan, Eric Sandeen, xfs
On Tue, Oct 17, 2006 at 05:13:01PM +1000, Tim Shimmin wrote:
> --On 17 October 2006 10:50:38 AM +1000 David Chinner <dgc@sgi.com> wrote:
> >Fix them - inline functions in header files should always be "static
> >inline". Inline functions in .c files should always be static as
> >well - if they need to be accessed from different source files then
> >they need to be in header files. Hence "STATIC inline" is broken
> >code and should be fixed anyway. Luckily, there are very few of
> >these to fix and they are all in .c files:
> >
> >chook 137% grep -rIw "STATIC inline" fs/xfs | wc -l
> >21
>
> So you are saying that "static inline"s should always be.
Yup.
> So for CONFIG_XFS_DEBUG where we define STATIC and so make static
> disappear for uses of STATIC, we will no longer touch these
> "static inline" functions.
Yup. The function will still get inlined, so changing it's scope on
debug builds doesn't provide any benefit IMO. FWIW, for debug
builds we probably want noinline....
> I thought that for debug, we could stop them from being inline
> for easier debugging. We could have a STATIC_INLINE :-)
We could, but I don't think it gains us anything.
Cheers,
Dave.
--
Dave Chinner
Principal Engineer
SGI Australian Software Group
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] Make stuff static
2006-10-17 21:57 ` David Chinner
@ 2006-10-17 22:45 ` Russell Cattelan
2006-11-22 0:42 ` David Chinner
2006-10-18 4:06 ` Timothy Shimmin
1 sibling, 1 reply; 24+ messages in thread
From: Russell Cattelan @ 2006-10-17 22:45 UTC (permalink / raw)
To: David Chinner; +Cc: Tim Shimmin, Eric Sandeen, xfs
[-- Attachment #1: Type: text/plain, Size: 1690 bytes --]
On Wed, 2006-10-18 at 07:57 +1000, David Chinner wrote:
> On Tue, Oct 17, 2006 at 05:13:01PM +1000, Tim Shimmin wrote:
> > --On 17 October 2006 10:50:38 AM +1000 David Chinner <dgc@sgi.com> wrote:
> > >Fix them - inline functions in header files should always be "static
> > >inline". Inline functions in .c files should always be static as
> > >well - if they need to be accessed from different source files then
> > >they need to be in header files. Hence "STATIC inline" is broken
> > >code and should be fixed anyway. Luckily, there are very few of
> > >these to fix and they are all in .c files:
> > >
> > >chook 137% grep -rIw "STATIC inline" fs/xfs | wc -l
> > >21
> >
> > So you are saying that "static inline"s should always be.
>
> Yup.
>
> > So for CONFIG_XFS_DEBUG where we define STATIC and so make static
> > disappear for uses of STATIC, we will no longer touch these
> > "static inline" functions.
>
> Yup. The function will still get inlined, so changing it's scope on
> debug builds doesn't provide any benefit IMO. FWIW, for debug
> builds we probably want noinline....
>
> > I thought that for debug, we could stop them from being inline
> > for easier debugging. We could have a STATIC_INLINE :-)
>
> We could, but I don't think it gains us anything.
I agree with Tim on this.
when I see STATIC in the code it's generally assumed to
be a way to toggle of static on/off. Adding static inline
to the #define STATIC starts to overload the the macro
and creates an obfuscation that isn't immediately obvious.
STATIC_INLINE should be fairly obvious.
>
> Cheers,
>
> Dave.
--
Russell Cattelan <cattelan@thebarn.com>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] Make stuff static
2006-10-17 22:45 ` Russell Cattelan
@ 2006-11-22 0:42 ` David Chinner
2006-11-22 1:09 ` Russell Cattelan
0 siblings, 1 reply; 24+ messages in thread
From: David Chinner @ 2006-11-22 0:42 UTC (permalink / raw)
To: Russell Cattelan; +Cc: David Chinner, Tim Shimmin, Eric Sandeen, xfs
On Tue, Oct 17, 2006 at 05:45:31PM -0500, Russell Cattelan wrote:
> On Wed, 2006-10-18 at 07:57 +1000, David Chinner wrote:
> > On Tue, Oct 17, 2006 at 05:13:01PM +1000, Tim Shimmin wrote:
> > > I thought that for debug, we could stop them from being inline
> > > for easier debugging. We could have a STATIC_INLINE :-)
> >
> > We could, but I don't think it gains us anything.
> I agree with Tim on this.
> when I see STATIC in the code it's generally assumed to
> be a way to toggle of static on/off. Adding static inline
> to the #define STATIC starts to overload the the macro
> and creates an obfuscation that isn't immediately obvious.
> STATIC_INLINE should be fairly obvious.
Ok, so I've had time to look at this again. Here's the definitions
of STATIC and STATIC_INLINE for debug and nondebug from the
patch (whitespace damaged):
Index: 2.6.x-xfs-new/fs/xfs/support/debug.h
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/support/debug.h 2006-11-22 10:54:37.089984780 +1100
+++ 2.6.x-xfs-new/fs/xfs/support/debug.h 2006-11-22 11:30:20.433326839 +1100
@@ -38,13 +38,37 @@ extern void assfail(char *expr, char *f,
#ifndef DEBUG
# define ASSERT(expr) ((void)0)
-#else
+
+#ifndef STATIC
+# define STATIC static noinline
+#endif
+
+#ifndef STATIC_INLINE
+# define STATIC_INLINE static inline
+#endif
+
+#else /* DEBUG */
+
# define ASSERT(expr) ASSERT_ALWAYS(expr)
extern unsigned long random(void);
-#endif
#ifndef STATIC
-# define STATIC static
+# define STATIC noinline
+#endif
+
+/*
+ * We stop inlining of inline functions in debug mode.
+ * Unfortunately, this means static inline in header files
+ * get multiple definitions, so they need to remain static.
+ * This then gives tonnes of warnings about unused but defined
+ * functions, so we need to add the unused attribute to prevent
+ * these spurious warnings.
+ */
+#ifndef STATIC_INLINE
+# define STATIC_INLINE static __attribute__ ((unused)) noinline
#endif
+#endif /* DEBUG */
+
+
#endif /* __XFS_SUPPORT_DEBUG_H__ */
------
Is this acceptible to everyone?
FWIW, there is one other thing that this conversion causes
problems with, and that's variable definitions. i.e. we can't
use STATIC on them any more because of the "noinline" attribute
it has. Do we care about this and if so, any suggestions on
how to keep this functionality (a different STATIC_xxx define
for structures)?
Cheers,
Dave.
--
Dave Chinner
Principal Engineer
SGI Australian Software Group
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] Make stuff static
2006-11-22 0:42 ` David Chinner
@ 2006-11-22 1:09 ` Russell Cattelan
2006-11-22 2:16 ` David Chatterton
2006-11-22 4:24 ` David Chinner
0 siblings, 2 replies; 24+ messages in thread
From: Russell Cattelan @ 2006-11-22 1:09 UTC (permalink / raw)
To: David Chinner; +Cc: Tim Shimmin, Eric Sandeen, xfs
[-- Attachment #1: Type: text/plain, Size: 3048 bytes --]
On Wed, 2006-11-22 at 11:42 +1100, David Chinner wrote:
> On Tue, Oct 17, 2006 at 05:45:31PM -0500, Russell Cattelan wrote:
> > On Wed, 2006-10-18 at 07:57 +1000, David Chinner wrote:
> > > On Tue, Oct 17, 2006 at 05:13:01PM +1000, Tim Shimmin wrote:
> > > > I thought that for debug, we could stop them from being inline
> > > > for easier debugging. We could have a STATIC_INLINE :-)
> > >
> > > We could, but I don't think it gains us anything.
> > I agree with Tim on this.
> > when I see STATIC in the code it's generally assumed to
> > be a way to toggle of static on/off. Adding static inline
> > to the #define STATIC starts to overload the the macro
> > and creates an obfuscation that isn't immediately obvious.
> > STATIC_INLINE should be fairly obvious.
>
> Ok, so I've had time to look at this again. Here's the definitions
> of STATIC and STATIC_INLINE for debug and nondebug from the
> patch (whitespace damaged):
>
> Index: 2.6.x-xfs-new/fs/xfs/support/debug.h
> ===================================================================
> --- 2.6.x-xfs-new.orig/fs/xfs/support/debug.h 2006-11-22 10:54:37.089984780 +1100
> +++ 2.6.x-xfs-new/fs/xfs/support/debug.h 2006-11-22 11:30:20.433326839 +1100
> @@ -38,13 +38,37 @@ extern void assfail(char *expr, char *f,
>
> #ifndef DEBUG
> # define ASSERT(expr) ((void)0)
> -#else
> +
> +#ifndef STATIC
> +# define STATIC static noinline
> +#endif
> +
> +#ifndef STATIC_INLINE
> +# define STATIC_INLINE static inline
> +#endif
> +
> +#else /* DEBUG */
> +
> # define ASSERT(expr) ASSERT_ALWAYS(expr)
> extern unsigned long random(void);
> -#endif
>
> #ifndef STATIC
> -# define STATIC static
> +# define STATIC noinline
> +#endif
> +
> +/*
> + * We stop inlining of inline functions in debug mode.
> + * Unfortunately, this means static inline in header files
> + * get multiple definitions, so they need to remain static.
> + * This then gives tonnes of warnings about unused but defined
> + * functions, so we need to add the unused attribute to prevent
> + * these spurious warnings.
> + */
> +#ifndef STATIC_INLINE
> +# define STATIC_INLINE static __attribute__ ((unused)) noinline
> #endif
>
> +#endif /* DEBUG */
> +
> +
> #endif /* __XFS_SUPPORT_DEBUG_H__ */
>
> ------
>
> Is this acceptible to everyone?
Yup.
>
> FWIW, there is one other thing that this conversion causes
> problems with, and that's variable definitions. i.e. we can't
> use STATIC on them any more because of the "noinline" attribute
> it has. Do we care about this and if so, any suggestions on
> how to keep this functionality (a different STATIC_xxx define
> for structures)?
So I know things like systemtap kgdb oprofile all work better when
functions are not static, but what about variables/structures?
do things really get that confused?
Maybe we shouldn't worry about conditioning them and just make them
static
>
> Cheers,
>
> Dave.
--
Russell Cattelan <cattelan@thebarn.com>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] Make stuff static
2006-11-22 1:09 ` Russell Cattelan
@ 2006-11-22 2:16 ` David Chatterton
2006-11-22 4:24 ` David Chinner
1 sibling, 0 replies; 24+ messages in thread
From: David Chatterton @ 2006-11-22 2:16 UTC (permalink / raw)
To: Russell Cattelan; +Cc: David Chinner, Tim Shimmin, Eric Sandeen, xfs
Russell Cattelan wrote:
> On Wed, 2006-11-22 at 11:42 +1100, David Chinner wrote:
>> On Tue, Oct 17, 2006 at 05:45:31PM -0500, Russell Cattelan wrote:
>>> On Wed, 2006-10-18 at 07:57 +1000, David Chinner wrote:
>>>> On Tue, Oct 17, 2006 at 05:13:01PM +1000, Tim Shimmin wrote:
>>>>> I thought that for debug, we could stop them from being inline
>>>>> for easier debugging. We could have a STATIC_INLINE :-)
>>>> We could, but I don't think it gains us anything.
>>> I agree with Tim on this.
>>> when I see STATIC in the code it's generally assumed to
>>> be a way to toggle of static on/off. Adding static inline
>>> to the #define STATIC starts to overload the the macro
>>> and creates an obfuscation that isn't immediately obvious.
>>> STATIC_INLINE should be fairly obvious.
>> Ok, so I've had time to look at this again. Here's the definitions
>> of STATIC and STATIC_INLINE for debug and nondebug from the
>> patch (whitespace damaged):
>>
>> Index: 2.6.x-xfs-new/fs/xfs/support/debug.h
>> ===================================================================
>> --- 2.6.x-xfs-new.orig/fs/xfs/support/debug.h 2006-11-22 10:54:37.089984780 +1100
>> +++ 2.6.x-xfs-new/fs/xfs/support/debug.h 2006-11-22 11:30:20.433326839 +1100
>> @@ -38,13 +38,37 @@ extern void assfail(char *expr, char *f,
>>
>> #ifndef DEBUG
>> # define ASSERT(expr) ((void)0)
>> -#else
>> +
>> +#ifndef STATIC
>> +# define STATIC static noinline
>> +#endif
>> +
>> +#ifndef STATIC_INLINE
>> +# define STATIC_INLINE static inline
>> +#endif
>> +
>> +#else /* DEBUG */
>> +
>> # define ASSERT(expr) ASSERT_ALWAYS(expr)
>> extern unsigned long random(void);
>> -#endif
>>
>> #ifndef STATIC
>> -# define STATIC static
>> +# define STATIC noinline
>> +#endif
>> +
>> +/*
>> + * We stop inlining of inline functions in debug mode.
>> + * Unfortunately, this means static inline in header files
>> + * get multiple definitions, so they need to remain static.
>> + * This then gives tonnes of warnings about unused but defined
>> + * functions, so we need to add the unused attribute to prevent
>> + * these spurious warnings.
>> + */
>> +#ifndef STATIC_INLINE
>> +# define STATIC_INLINE static __attribute__ ((unused)) noinline
>> #endif
>>
>> +#endif /* DEBUG */
>> +
>> +
>> #endif /* __XFS_SUPPORT_DEBUG_H__ */
>>
>> ------
>>
>> Is this acceptible to everyone?
> Yup.
>
>> FWIW, there is one other thing that this conversion causes
>> problems with, and that's variable definitions. i.e. we can't
>> use STATIC on them any more because of the "noinline" attribute
>> it has. Do we care about this and if so, any suggestions on
>> how to keep this functionality (a different STATIC_xxx define
>> for structures)?
> So I know things like systemtap kgdb oprofile all work better when
> functions are not static, but what about variables/structures?
> do things really get that confused?
> Maybe we shouldn't worry about conditioning them and just make them
> static
>
I agree with Russell, is there a case for not defining a structure static?
I can't think of one, unless it kdb/lcrash is going to work better if they are
not static in a debug build. Otherwise, we should just use "static" and not
"STATIC". Some for static file variables.
David
--
David Chatterton
XFS Engineering Manager
SGI Australia
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] Make stuff static
2006-11-22 1:09 ` Russell Cattelan
2006-11-22 2:16 ` David Chatterton
@ 2006-11-22 4:24 ` David Chinner
2006-11-22 4:53 ` David Chatterton
2006-11-26 14:05 ` Eric Sandeen
1 sibling, 2 replies; 24+ messages in thread
From: David Chinner @ 2006-11-22 4:24 UTC (permalink / raw)
To: Russell Cattelan; +Cc: David Chinner, Tim Shimmin, Eric Sandeen, xfs
On Tue, Nov 21, 2006 at 07:09:43PM -0600, Russell Cattelan wrote:
> On Wed, 2006-11-22 at 11:42 +1100, David Chinner wrote:
> > Ok, so I've had time to look at this again. Here's the definitions
> > of STATIC and STATIC_INLINE for debug and nondebug from the
> > patch (whitespace damaged):
.....
> > Is this acceptible to everyone?
> Yup.
>
> >
> > FWIW, there is one other thing that this conversion causes
> > problems with, and that's variable definitions. i.e. we can't
> > use STATIC on them any more because of the "noinline" attribute
> > it has. Do we care about this and if so, any suggestions on
> > how to keep this functionality (a different STATIC_xxx define
> > for structures)?
> So I know things like systemtap kgdb oprofile all work better when
> functions are not static, but what about variables/structures?
> do things really get that confused?
> Maybe we shouldn't worry about conditioning them and just make them
> static
Ok, so I'd already converted them to static where necessary.
Attached is thecomplete patch. On ia64, the size of the xfs.ko
and xfs_quota.ko modules decreases with this patch:
Orig:
-rw-rw-r-- 1 dgc ptg 1662416 2006-11-22 14:41 fs/xfs/quota/xfs_quota.ko
-rw-rw-r-- 1 dgc ptg 856748 2006-11-22 14:41 fs/xfs/xfsidbg.ko
-rw-rw-r-- 1 dgc ptg 13614719 2006-11-22 14:41 fs/xfs/xfs.ko
With patch:
-rw-rw-r-- 1 dgc ptg 1657814 2006-11-22 14:42 fs/xfs/quota/xfs_quota.ko
-rw-rw-r-- 1 dgc ptg 856748 2006-11-22 14:42 fs/xfs/xfsidbg.ko
-rw-rw-r-- 1 dgc ptg 13557579 2006-11-22 14:42 fs/xfs/xfs.ko
The original top 10 stack users:
0x000e10c6 xfs_vn_mknod [xfs]: 576
0x000ddfe6 xfs_ioctl [xfs]: 368
0x000e1f46 xfs_vn_symlink [xfs]: 368
0x000345a6 xfs_bmapi [xfs]: 320
0x000b1146 _xfs_trans_commit [xfs]: 272
0x000c59c6 xfs_change_file_space [xfs]: 272
0x0003a6a6 xfs_bunmapi [xfs]: 240
0x000afa06 xfs_trans_unreserve_and_mod_sb [xfs]: 224
0x00040626 xfs_bmbt_insert [xfs]: 192
0x0008be26 xfs_iomap_write_delay [xfs]: 192
[64 functions with stack usage larger than 100 bytes]
With patch:
0x000b7c46 _xfs_trans_commit [xfs]: 272
0x000b5426 xfs_trans_unreserve_and_mod_sb [xfs]: 224
0x000e4106 xfs_find_handle [xfs]: 224
0x000396c6 xfs_bmapi [xfs]: 208
0x00090066 xfs_iomap_write_delay [xfs]: 208
0x000e9046 xfs_cleanup_inode [xfs]: 208
0x000058c6 xfs_acl_setmode [xfs]: 160
0x00005f46 xfs_acl_allow_set [xfs]: 160
0x000067c6 xfs_acl_vtoacl [xfs]: 160
0x00007366 xfs_acl_vget [xfs]: 160
[69 functions with stack usage larger than 100 bytes]
Performance appears to be slight faster with the noinline
patch, but the variation is within the error margins of
my measurements so I'd say it's neutral.
Comments?
Cheers,
Dave.
--
Dave Chinner
Principal Engineer
SGI Australian Software Group
---
fs/xfs/dmapi/xfs_dm.c | 6 +++---
fs/xfs/linux-2.4/mrlock.c | 2 +-
fs/xfs/linux-2.4/xfs_buf.c | 30 +++++++++++++++---------------
fs/xfs/linux-2.4/xfs_file.c | 8 ++++----
fs/xfs/linux-2.4/xfs_super.c | 14 +++++++-------
fs/xfs/linux-2.4/xfs_vnode.h | 4 ++--
fs/xfs/linux-2.6/xfs_aops.c | 2 +-
fs/xfs/linux-2.6/xfs_buf.c | 24 ++++++++++++------------
fs/xfs/linux-2.6/xfs_export.c | 2 +-
fs/xfs/linux-2.6/xfs_file.c | 4 ++--
fs/xfs/linux-2.6/xfs_iops.c | 4 ++--
fs/xfs/linux-2.6/xfs_super.c | 16 ++++++++--------
fs/xfs/linux-2.6/xfs_sysctl.c | 6 +++---
fs/xfs/linux-2.6/xfs_vfs.c | 5 +++--
fs/xfs/linux-2.6/xfs_vnode.c | 2 +-
fs/xfs/linux-2.6/xfs_vnode.h | 4 ++--
fs/xfs/quota/xfs_dquot_item.c | 6 +++---
fs/xfs/quota/xfs_qm.c | 6 +++---
fs/xfs/quota/xfs_qm_bhv.c | 2 +-
fs/xfs/support/debug.h | 30 +++++++++++++++++++++++++++---
fs/xfs/xfs_attr.c | 12 ++++++------
fs/xfs/xfs_attr_leaf.c | 6 +++---
fs/xfs/xfs_bit.c | 2 +-
fs/xfs/xfs_bmap_btree.c | 2 +-
fs/xfs/xfs_buf_item.c | 2 +-
fs/xfs/xfs_extfree_item.c | 4 ++--
fs/xfs/xfs_ialloc.c | 2 +-
fs/xfs/xfs_inode.c | 2 +-
fs/xfs/xfs_inode_item.c | 2 +-
fs/xfs/xfs_mount.c | 8 ++++----
fs/xfs/xfs_refcache.c | 10 +++++-----
31 files changed, 127 insertions(+), 102 deletions(-)
Index: 2.6.x-xfs-new/fs/xfs/linux-2.4/xfs_file.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.4/xfs_file.c 2006-11-22 14:41:06.076805511 +1100
+++ 2.6.x-xfs-new/fs/xfs/linux-2.4/xfs_file.c 2006-11-22 14:42:21.558973682 +1100
@@ -55,7 +55,7 @@ static struct vm_operations_struct xfs_d
#define do_up_read(x)
#endif
-STATIC inline ssize_t
+STATIC_INLINE ssize_t
__xfs_file_read(
struct file *file,
char *buf,
@@ -99,7 +99,7 @@ xfs_file_read_invis(
}
-STATIC inline ssize_t
+STATIC_INLINE ssize_t
__xfs_file_write(
struct file *file,
const char *buf,
@@ -146,7 +146,7 @@ __xfs_file_write(
return rval;
}
-STATIC inline ssize_t
+STATIC_INLINE ssize_t
xfs_file_write(
struct file *file,
const char *buf,
@@ -156,7 +156,7 @@ xfs_file_write(
return __xfs_file_write(file, buf, 0, count, ppos);
}
-STATIC inline ssize_t
+STATIC_INLINE ssize_t
xfs_file_write_invis(
struct file *file,
const char *buf,
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_aops.c 2006-11-22 14:41:06.112800823 +1100
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_aops.c 2006-11-22 14:42:21.558973682 +1100
@@ -246,7 +246,7 @@ xfs_map_blocks(
return -error;
}
-STATIC inline int
+STATIC_INLINE int
xfs_iomap_valid(
xfs_iomap_t *iomapp,
loff_t offset)
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_buf.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_buf.c 2006-11-22 14:41:06.112800823 +1100
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_buf.c 2006-11-22 14:42:21.562973161 +1100
@@ -34,13 +34,13 @@
#include <linux/backing-dev.h>
#include "xfs_linux.h"
-STATIC kmem_zone_t *xfs_buf_zone;
-STATIC kmem_shaker_t xfs_buf_shake;
+static kmem_zone_t *xfs_buf_zone;
+static kmem_shaker_t xfs_buf_shake;
STATIC int xfsbufd(void *);
STATIC int xfsbufd_wakeup(int, gfp_t);
STATIC void xfs_buf_delwri_queue(xfs_buf_t *, int);
-STATIC struct workqueue_struct *xfslogd_workqueue;
+static struct workqueue_struct *xfslogd_workqueue;
struct workqueue_struct *xfsdatad_workqueue;
#ifdef XFS_BUF_TRACE
@@ -139,7 +139,7 @@ page_region_mask(
return mask;
}
-STATIC inline void
+STATIC_INLINE void
set_page_region(
struct page *page,
size_t offset,
@@ -151,7 +151,7 @@ set_page_region(
SetPageUptodate(page);
}
-STATIC inline int
+STATIC_INLINE int
test_page_region(
struct page *page,
size_t offset,
@@ -171,9 +171,9 @@ typedef struct a_list {
struct a_list *next;
} a_list_t;
-STATIC a_list_t *as_free_head;
-STATIC int as_list_len;
-STATIC DEFINE_SPINLOCK(as_lock);
+static a_list_t *as_free_head;
+static int as_list_len;
+static DEFINE_SPINLOCK(as_lock);
/*
* Try to batch vunmaps because they are costly.
@@ -1084,7 +1084,7 @@ xfs_buf_iostart(
return status;
}
-STATIC __inline__ int
+STATIC_INLINE int
_xfs_buf_iolocked(
xfs_buf_t *bp)
{
@@ -1094,7 +1094,7 @@ _xfs_buf_iolocked(
return 0;
}
-STATIC __inline__ void
+STATIC_INLINE void
_xfs_buf_ioend(
xfs_buf_t *bp,
int schedule)
@@ -1425,8 +1425,8 @@ xfs_free_bufhash(
/*
* buftarg list for delwrite queue processing
*/
-STATIC LIST_HEAD(xfs_buftarg_list);
-STATIC DEFINE_SPINLOCK(xfs_buftarg_lock);
+LIST_HEAD(xfs_buftarg_list);
+static DEFINE_SPINLOCK(xfs_buftarg_lock);
STATIC void
xfs_register_buftarg(
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_file.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_file.c 2006-11-22 14:41:06.112800823 +1100
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_file.c 2006-11-22 14:42:21.562973161 +1100
@@ -46,7 +46,7 @@ static struct vm_operations_struct xfs_f
static struct vm_operations_struct xfs_dmapi_file_vm_ops;
#endif
-STATIC inline ssize_t
+STATIC_INLINE ssize_t
__xfs_file_read(
struct kiocb *iocb,
const struct iovec *iov,
@@ -84,7 +84,7 @@ xfs_file_aio_read_invis(
return __xfs_file_read(iocb, iov, nr_segs, IO_ISAIO|IO_INVIS, pos);
}
-STATIC inline ssize_t
+STATIC_INLINE ssize_t
__xfs_file_write(
struct kiocb *iocb,
const struct iovec *iov,
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_iops.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_iops.c 2006-11-22 14:41:06.112800823 +1100
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_iops.c 2006-11-22 14:42:21.562973161 +1100
@@ -250,13 +250,13 @@ xfs_init_security(
*
* XXX(hch): nfsd is broken, better fix it instead.
*/
-STATIC inline int
+STATIC_INLINE int
xfs_has_fs_struct(struct task_struct *task)
{
return (task->fs != init_task.fs);
}
-STATIC inline void
+STATIC void
xfs_cleanup_inode(
bhv_vnode_t *dvp,
bhv_vnode_t *vp,
Index: 2.6.x-xfs-new/fs/xfs/support/debug.h
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/support/debug.h 2006-11-22 14:41:06.144796655 +1100
+++ 2.6.x-xfs-new/fs/xfs/support/debug.h 2006-11-22 14:42:21.566972640 +1100
@@ -38,13 +38,37 @@ extern void assfail(char *expr, char *f,
#ifndef DEBUG
# define ASSERT(expr) ((void)0)
-#else
+
+#ifndef STATIC
+# define STATIC static noinline
+#endif
+
+#ifndef STATIC_INLINE
+# define STATIC_INLINE static inline
+#endif
+
+#else /* DEBUG */
+
# define ASSERT(expr) ASSERT_ALWAYS(expr)
extern unsigned long random(void);
-#endif
#ifndef STATIC
-# define STATIC static
+# define STATIC noinline
+#endif
+
+/*
+ * We stop inlining of inline functions in debug mode.
+ * Unfortunately, this means static inline in header files
+ * get multiple definitions, so they need to remain static.
+ * This then gives tonnes of warnings about unused but defined
+ * functions, so we need to add the unused attribute to prevent
+ * these spurious warnings.
+ */
+#ifndef STATIC_INLINE
+# define STATIC_INLINE static __attribute__ ((unused)) noinline
#endif
+#endif /* DEBUG */
+
+
#endif /* __XFS_SUPPORT_DEBUG_H__ */
Index: 2.6.x-xfs-new/fs/xfs/xfs_attr_leaf.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_attr_leaf.c 2006-11-22 14:41:06.180791966 +1100
+++ 2.6.x-xfs-new/fs/xfs/xfs_attr_leaf.c 2006-11-22 14:42:21.566972640 +1100
@@ -94,7 +94,7 @@ STATIC int xfs_attr_leaf_entsize(xfs_att
* Namespace helper routines
*========================================================================*/
-STATIC inline attrnames_t *
+STATIC_INLINE attrnames_t *
xfs_attr_flags_namesp(int flags)
{
return ((flags & XFS_ATTR_SECURE) ? &attr_secure:
@@ -105,7 +105,7 @@ xfs_attr_flags_namesp(int flags)
* If namespace bits don't match return 0.
* If all match then return 1.
*/
-STATIC inline int
+STATIC_INLINE int
xfs_attr_namesp_match(int arg_flags, int ondisk_flags)
{
return XFS_ATTR_NSP_ONDISK(ondisk_flags) == XFS_ATTR_NSP_ARGS_TO_ONDISK(arg_flags);
@@ -116,7 +116,7 @@ xfs_attr_namesp_match(int arg_flags, int
* then return 0.
* If all match or are overridable then return 1.
*/
-STATIC inline int
+STATIC_INLINE int
xfs_attr_namesp_match_overrides(int arg_flags, int ondisk_flags)
{
if (((arg_flags & ATTR_SECURE) == 0) !=
Index: 2.6.x-xfs-new/fs/xfs/xfs_mount.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_mount.c 2006-11-22 14:41:06.180791966 +1100
+++ 2.6.x-xfs-new/fs/xfs/xfs_mount.c 2006-11-22 14:42:21.566972640 +1100
@@ -1796,7 +1796,7 @@ xfs_icsb_destroy_counters(
}
}
-STATIC inline void
+STATIC_INLINE void
xfs_icsb_lock_cntr(
xfs_icsb_cnts_t *icsbp)
{
@@ -1805,7 +1805,7 @@ xfs_icsb_lock_cntr(
}
}
-STATIC inline void
+STATIC_INLINE void
xfs_icsb_unlock_cntr(
xfs_icsb_cnts_t *icsbp)
{
@@ -1813,7 +1813,7 @@ xfs_icsb_unlock_cntr(
}
-STATIC inline void
+STATIC_INLINE void
xfs_icsb_lock_all_counters(
xfs_mount_t *mp)
{
@@ -1826,7 +1826,7 @@ xfs_icsb_lock_all_counters(
}
}
-STATIC inline void
+STATIC_INLINE void
xfs_icsb_unlock_all_counters(
xfs_mount_t *mp)
{
Index: 2.6.x-xfs-new/fs/xfs/linux-2.4/xfs_buf.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.4/xfs_buf.c 2006-11-22 14:41:06.076805511 +1100
+++ 2.6.x-xfs-new/fs/xfs/linux-2.4/xfs_buf.c 2006-11-22 14:42:21.578971077 +1100
@@ -67,17 +67,17 @@
#define VM_MAP VM_ALLOC
#endif
-STATIC kmem_zone_t *xfs_buf_zone;
-STATIC kmem_shaker_t xfs_buf_shake;
+static kmem_zone_t *xfs_buf_zone;
+static kmem_shaker_t xfs_buf_shake;
#define MAX_IO_DAEMONS NR_CPUS
#define CPU_TO_DAEMON(cpu) (cpu)
-STATIC int xb_logio_daemons[MAX_IO_DAEMONS];
-STATIC struct list_head xfs_buf_logiodone_tq[MAX_IO_DAEMONS];
-STATIC wait_queue_head_t xfs_buf_logiodone_wait[MAX_IO_DAEMONS];
-STATIC int xb_dataio_daemons[MAX_IO_DAEMONS];
-STATIC struct list_head xfs_buf_dataiodone_tq[MAX_IO_DAEMONS];
-STATIC wait_queue_head_t xfs_buf_dataiodone_wait[MAX_IO_DAEMONS];
+static int xb_logio_daemons[MAX_IO_DAEMONS];
+static struct list_head xfs_buf_logiodone_tq[MAX_IO_DAEMONS];
+static wait_queue_head_t xfs_buf_logiodone_wait[MAX_IO_DAEMONS];
+static int xb_dataio_daemons[MAX_IO_DAEMONS];
+static struct list_head xfs_buf_dataiodone_tq[MAX_IO_DAEMONS];
+static wait_queue_head_t xfs_buf_dataiodone_wait[MAX_IO_DAEMONS];
/*
* For pre-allocated buffer head pool
@@ -154,9 +154,9 @@ typedef struct a_list {
struct a_list *next;
} a_list_t;
-STATIC a_list_t *as_free_head;
-STATIC int as_list_len;
-STATIC DEFINE_SPINLOCK(as_lock);
+static a_list_t *as_free_head;
+static int as_list_len;
+static DEFINE_SPINLOCK(as_lock);
/*
* Try to batch vunmaps because they are costly.
@@ -515,7 +515,7 @@ _xfs_buf_get_prealloc_bh(void)
* Otherwise, put it back in the pool, and wake up anybody
* waiting for one.
*/
-STATIC inline void
+STATIC_INLINE void
_xfs_buf_free_bh(
struct buffer_head *bh)
{
@@ -1204,7 +1204,7 @@ xfs_buf_iostart(
return status;
}
-STATIC __inline__ int
+STATIC_INLINE int
_xfs_buf_iolocked(
xfs_buf_t *bp)
{
@@ -1366,8 +1366,8 @@ xfs_free_bufhash(
/*
* buftarg list for delwrite queue processing
*/
-STATIC LIST_HEAD(xfs_buftarg_list);
-STATIC DEFINE_SPINLOCK(xfs_buftarg_lock);
+static LIST_HEAD(xfs_buftarg_list);
+static DEFINE_SPINLOCK(xfs_buftarg_lock);
STATIC void
xfs_register_buftarg(
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_sysctl.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_sysctl.c 2006-11-22 14:41:06.112800823 +1100
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_sysctl.c 2006-11-22 14:42:21.594968993 +1100
@@ -54,7 +54,7 @@ xfs_stats_clear_proc_handler(
}
#endif /* CONFIG_PROC_FS */
-STATIC ctl_table xfs_table[] = {
+static ctl_table xfs_table[] = {
{XFS_RESTRICT_CHOWN, "restrict_chown", &xfs_params.restrict_chown.val,
sizeof(int), 0644, NULL, &proc_dointvec_minmax,
&sysctl_intvec, NULL,
@@ -151,12 +151,12 @@ STATIC ctl_table xfs_table[] = {
{0}
};
-STATIC ctl_table xfs_dir_table[] = {
+static ctl_table xfs_dir_table[] = {
{FS_XFS, "xfs", NULL, 0, 0555, xfs_table},
{0}
};
-STATIC ctl_table xfs_root_table[] = {
+static ctl_table xfs_root_table[] = {
{CTL_FS, "fs", NULL, 0, 0555, xfs_dir_table},
{0}
};
Index: 2.6.x-xfs-new/fs/xfs/xfs_attr.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_attr.c 2006-11-22 14:41:06.188790924 +1100
+++ 2.6.x-xfs-new/fs/xfs/xfs_attr.c 2006-11-22 14:42:21.598968473 +1100
@@ -57,9 +57,9 @@
*/
#define ATTR_SYSCOUNT 2
-STATIC struct attrnames posix_acl_access;
-STATIC struct attrnames posix_acl_default;
-STATIC struct attrnames *attr_system_names[ATTR_SYSCOUNT];
+static struct attrnames posix_acl_access;
+static struct attrnames posix_acl_default;
+static struct attrnames *attr_system_names[ATTR_SYSCOUNT];
/*========================================================================
* Function prototypes for the kernel.
@@ -2477,7 +2477,7 @@ posix_acl_default_exists(
return xfs_acl_vhasacl_default(vp);
}
-STATIC struct attrnames posix_acl_access = {
+static struct attrnames posix_acl_access = {
.attr_name = "posix_acl_access",
.attr_namelen = sizeof("posix_acl_access") - 1,
.attr_get = posix_acl_access_get,
@@ -2486,7 +2486,7 @@ STATIC struct attrnames posix_acl_access
.attr_exists = posix_acl_access_exists,
};
-STATIC struct attrnames posix_acl_default = {
+static struct attrnames posix_acl_default = {
.attr_name = "posix_acl_default",
.attr_namelen = sizeof("posix_acl_default") - 1,
.attr_get = posix_acl_default_get,
@@ -2495,7 +2495,7 @@ STATIC struct attrnames posix_acl_defaul
.attr_exists = posix_acl_default_exists,
};
-STATIC struct attrnames *attr_system_names[] =
+static struct attrnames *attr_system_names[] =
{ &posix_acl_access, &posix_acl_default };
Index: 2.6.x-xfs-new/fs/xfs/xfs_bit.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_bit.c 2006-11-22 14:41:06.196789882 +1100
+++ 2.6.x-xfs-new/fs/xfs/xfs_bit.c 2006-11-22 14:42:21.598968473 +1100
@@ -29,7 +29,7 @@
/*
* Index of high bit number in byte, -1 for none set, 0..7 otherwise.
*/
-STATIC const char xfs_highbit[256] = {
+static const char xfs_highbit[256] = {
-1, 0, 1, 1, 2, 2, 2, 2, /* 00 .. 07 */
3, 3, 3, 3, 3, 3, 3, 3, /* 08 .. 0f */
4, 4, 4, 4, 4, 4, 4, 4, /* 10 .. 17 */
Index: 2.6.x-xfs-new/fs/xfs/linux-2.4/mrlock.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.4/mrlock.c 2006-11-22 14:41:06.076805511 +1100
+++ 2.6.x-xfs-new/fs/xfs/linux-2.4/mrlock.c 2006-11-22 14:42:21.598968473 +1100
@@ -195,7 +195,7 @@ mrtryupdate(mrlock_t *mrp)
return 1;
}
-static __inline__ void mrwake(mrlock_t *mrp)
+STATIC_INLINE void mrwake(mrlock_t *mrp)
{
/*
* First, if the count is now 0, we need to wake-up anyone waiting.
Index: 2.6.x-xfs-new/fs/xfs/linux-2.4/xfs_super.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.4/xfs_super.c 2006-11-22 14:41:06.076805511 +1100
+++ 2.6.x-xfs-new/fs/xfs/linux-2.4/xfs_super.c 2006-11-22 14:42:21.598968473 +1100
@@ -54,9 +54,9 @@
#include <linux/init.h>
-STATIC struct quotactl_ops xfs_quotactl_operations;
-STATIC struct super_operations xfs_super_operations;
-STATIC kmem_zone_t *xfs_vnode_zone;
+static struct quotactl_ops xfs_quotactl_operations;
+static struct super_operations xfs_super_operations;
+static kmem_zone_t *xfs_vnode_zone;
STATIC struct xfs_mount_args *
xfs_args_allocate(
@@ -113,7 +113,7 @@ xfs_max_file_offset(
return (((__uint64_t)pagefactor) << bitshift) - 1;
}
-STATIC __inline__ void
+STATIC_INLINE void
xfs_set_inodeops(
struct inode *inode)
{
@@ -140,7 +140,7 @@ xfs_set_inodeops(
}
}
-STATIC __inline__ void
+STATIC_INLINE void
xfs_revalidate_inode(
xfs_mount_t *mp,
bhv_vnode_t *vp,
@@ -974,7 +974,7 @@ fail_vfsop:
}
-STATIC struct super_operations xfs_super_operations = {
+static struct super_operations xfs_super_operations = {
.alloc_inode = xfs_fs_alloc_inode,
.destroy_inode = xfs_fs_destroy_inode,
.write_inode = xfs_fs_write_inode,
@@ -991,7 +991,7 @@ STATIC struct super_operations xfs_super
.show_options = xfs_fs_show_options,
};
-STATIC struct quotactl_ops xfs_quotactl_operations = {
+static struct quotactl_ops xfs_quotactl_operations = {
.quota_sync = xfs_fs_quotasync,
.get_xstate = xfs_fs_getxstate,
.set_xstate = xfs_fs_setxstate,
Index: 2.6.x-xfs-new/fs/xfs/linux-2.4/xfs_vnode.h
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.4/xfs_vnode.h 2006-11-22 14:41:06.092803428 +1100
+++ 2.6.x-xfs-new/fs/xfs/linux-2.4/xfs_vnode.h 2006-11-22 14:42:21.598968473 +1100
@@ -468,14 +468,14 @@ static inline struct bhv_vnode *vn_grab(
#define VN_LOCK(vp) mutex_spinlock(&(vp)->v_lock)
#define VN_UNLOCK(vp, s) mutex_spinunlock(&(vp)->v_lock, s)
-static __inline__ void vn_flagset(struct bhv_vnode *vp, uint flag)
+STATIC_INLINE void vn_flagset(struct bhv_vnode *vp, uint flag)
{
spin_lock(&vp->v_lock);
vp->v_flag |= flag;
spin_unlock(&vp->v_lock);
}
-static __inline__ uint vn_flagclr(struct bhv_vnode *vp, uint flag)
+STATIC_INLINE uint vn_flagclr(struct bhv_vnode *vp, uint flag)
{
uint cleared;
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_super.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_super.c 2006-11-22 14:41:06.112800823 +1100
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_super.c 2006-11-22 14:42:21.602967952 +1100
@@ -57,10 +57,10 @@
#include <linux/writeback.h>
#include <linux/kthread.h>
-STATIC struct quotactl_ops xfs_quotactl_operations;
-STATIC struct super_operations xfs_super_operations;
-STATIC kmem_zone_t *xfs_vnode_zone;
-STATIC kmem_zone_t *xfs_ioend_zone;
+static struct quotactl_ops xfs_quotactl_operations;
+static struct super_operations xfs_super_operations;
+static kmem_zone_t *xfs_vnode_zone;
+static kmem_zone_t *xfs_ioend_zone;
mempool_t *xfs_ioend_pool;
STATIC struct xfs_mount_args *
@@ -120,7 +120,7 @@ xfs_max_file_offset(
return (((__uint64_t)pagefactor) << bitshift) - 1;
}
-STATIC __inline__ void
+STATIC_INLINE void
xfs_set_inodeops(
struct inode *inode)
{
@@ -146,7 +146,7 @@ xfs_set_inodeops(
}
}
-STATIC __inline__ void
+STATIC_INLINE void
xfs_revalidate_inode(
xfs_mount_t *mp,
bhv_vnode_t *vp,
@@ -881,7 +881,7 @@ xfs_fs_get_sb(
mnt);
}
-STATIC struct super_operations xfs_super_operations = {
+static struct super_operations xfs_super_operations = {
.alloc_inode = xfs_fs_alloc_inode,
.destroy_inode = xfs_fs_destroy_inode,
.write_inode = xfs_fs_write_inode,
@@ -895,7 +895,7 @@ STATIC struct super_operations xfs_super
.show_options = xfs_fs_show_options,
};
-STATIC struct quotactl_ops xfs_quotactl_operations = {
+static struct quotactl_ops xfs_quotactl_operations = {
.quota_sync = xfs_fs_quotasync,
.get_xstate = xfs_fs_getxstate,
.set_xstate = xfs_fs_setxstate,
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_vnode.h
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_vnode.h 2006-11-22 14:41:06.112800823 +1100
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_vnode.h 2006-11-22 14:42:21.602967952 +1100
@@ -492,14 +492,14 @@ static inline struct bhv_vnode *vn_grab(
#define VN_LOCK(vp) mutex_spinlock(&(vp)->v_lock)
#define VN_UNLOCK(vp, s) mutex_spinunlock(&(vp)->v_lock, s)
-static __inline__ void vn_flagset(struct bhv_vnode *vp, uint flag)
+STATIC_INLINE void vn_flagset(struct bhv_vnode *vp, uint flag)
{
spin_lock(&vp->v_lock);
vp->v_flag |= flag;
spin_unlock(&vp->v_lock);
}
-static __inline__ uint vn_flagclr(struct bhv_vnode *vp, uint flag)
+STATIC_INLINE uint vn_flagclr(struct bhv_vnode *vp, uint flag)
{
uint cleared;
Index: 2.6.x-xfs-new/fs/xfs/xfs_bmap_btree.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_bmap_btree.c 2006-11-22 14:41:06.204788840 +1100
+++ 2.6.x-xfs-new/fs/xfs/xfs_bmap_btree.c 2006-11-22 14:42:21.602967952 +1100
@@ -1862,7 +1862,7 @@ xfs_bmbt_delete(
* xfs_bmbt_get_startblock, xfs_bmbt_get_blockcount and xfs_bmbt_get_state.
*/
-STATIC __inline__ void
+STATIC_INLINE void
__xfs_bmbt_get_all(
__uint64_t l0,
__uint64_t l1,
Index: 2.6.x-xfs-new/fs/xfs/xfs_extfree_item.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_extfree_item.c 2006-11-22 14:41:06.204788840 +1100
+++ 2.6.x-xfs-new/fs/xfs/xfs_extfree_item.c 2006-11-22 14:42:21.606967431 +1100
@@ -227,7 +227,7 @@ xfs_efi_item_committing(xfs_efi_log_item
/*
* This is the ops vector shared by all efi log items.
*/
-STATIC struct xfs_item_ops xfs_efi_item_ops = {
+static struct xfs_item_ops xfs_efi_item_ops = {
.iop_size = (uint(*)(xfs_log_item_t*))xfs_efi_item_size,
.iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*))
xfs_efi_item_format,
@@ -525,7 +525,7 @@ xfs_efd_item_committing(xfs_efd_log_item
/*
* This is the ops vector shared by all efd log items.
*/
-STATIC struct xfs_item_ops xfs_efd_item_ops = {
+static struct xfs_item_ops xfs_efd_item_ops = {
.iop_size = (uint(*)(xfs_log_item_t*))xfs_efd_item_size,
.iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*))
xfs_efd_item_format,
Index: 2.6.x-xfs-new/fs/xfs/xfs_inode.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_inode.c 2006-11-22 14:41:06.204788840 +1100
+++ 2.6.x-xfs-new/fs/xfs/xfs_inode.c 2006-11-22 14:42:21.614966389 +1100
@@ -2125,7 +2125,7 @@ xfs_iunlink_remove(
return 0;
}
-static __inline__ int xfs_inode_clean(xfs_inode_t *ip)
+STATIC_INLINE int xfs_inode_clean(xfs_inode_t *ip)
{
return (((ip->i_itemp == NULL) ||
!(ip->i_itemp->ili_format.ilf_fields & XFS_ILOG_ALL)) &&
Index: 2.6.x-xfs-new/fs/xfs/xfs_buf_item.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_buf_item.c 2006-11-22 14:41:06.204788840 +1100
+++ 2.6.x-xfs-new/fs/xfs/xfs_buf_item.c 2006-11-22 14:42:21.618965868 +1100
@@ -660,7 +660,7 @@ xfs_buf_item_committing(xfs_buf_log_item
/*
* This is the ops vector shared by all buf log items.
*/
-STATIC struct xfs_item_ops xfs_buf_item_ops = {
+static struct xfs_item_ops xfs_buf_item_ops = {
.iop_size = (uint(*)(xfs_log_item_t*))xfs_buf_item_size,
.iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*))
xfs_buf_item_format,
Index: 2.6.x-xfs-new/fs/xfs/xfs_ialloc.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_ialloc.c 2006-11-22 14:41:06.204788840 +1100
+++ 2.6.x-xfs-new/fs/xfs/xfs_ialloc.c 2006-11-22 14:42:21.618965868 +1100
@@ -342,7 +342,7 @@ xfs_ialloc_ag_alloc(
return 0;
}
-STATIC __inline xfs_agnumber_t
+STATIC_INLINE xfs_agnumber_t
xfs_ialloc_next_ag(
xfs_mount_t *mp)
{
Index: 2.6.x-xfs-new/fs/xfs/xfs_inode_item.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_inode_item.c 2006-11-22 14:41:06.204788840 +1100
+++ 2.6.x-xfs-new/fs/xfs/xfs_inode_item.c 2006-11-22 14:42:21.618965868 +1100
@@ -887,7 +887,7 @@ xfs_inode_item_committing(
/*
* This is the ops vector shared by all buf log items.
*/
-STATIC struct xfs_item_ops xfs_inode_item_ops = {
+static struct xfs_item_ops xfs_inode_item_ops = {
.iop_size = (uint(*)(xfs_log_item_t*))xfs_inode_item_size,
.iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*))
xfs_inode_item_format,
Index: 2.6.x-xfs-new/fs/xfs/dmapi/xfs_dm.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/dmapi/xfs_dm.c 2006-11-22 14:41:06.268780504 +1100
+++ 2.6.x-xfs-new/fs/xfs/dmapi/xfs_dm.c 2006-11-22 14:42:21.622965347 +1100
@@ -133,9 +133,9 @@ typedef struct {
changed!
*/
-STATIC const char dmattr_prefix[DMATTR_PREFIXLEN + 1] = DMATTR_PREFIXSTRING;
+static const char dmattr_prefix[DMATTR_PREFIXLEN + 1] = DMATTR_PREFIXSTRING;
-STATIC dm_size_t dm_min_dio_xfer = 0; /* direct I/O disabled for now */
+static dm_size_t dm_min_dio_xfer = 0; /* direct I/O disabled for now */
/* See xfs_dm_get_dmattr() for a description of why this is needed. */
@@ -3124,7 +3124,7 @@ xfs_dm_obj_ref_hold(
}
-STATIC fsys_function_vector_t xfs_fsys_vector[DM_FSYS_MAX];
+static fsys_function_vector_t xfs_fsys_vector[DM_FSYS_MAX];
int
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_export.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_export.c 2006-11-22 14:41:06.112800823 +1100
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_export.c 2006-11-22 14:42:21.622965347 +1100
@@ -24,7 +24,7 @@
#include "xfs_mount.h"
#include "xfs_export.h"
-STATIC struct dentry dotdot = { .d_name.name = "..", .d_name.len = 2, };
+static struct dentry dotdot = { .d_name.name = "..", .d_name.len = 2, };
/*
* XFS encodes and decodes the fileid portion of NFS filehandles
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_vfs.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_vfs.c 2006-11-22 14:41:06.112800823 +1100
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_vfs.c 2006-11-22 14:42:21.622965347 +1100
@@ -295,8 +295,9 @@ typedef struct bhv_module_list {
const char * bm_name;
void * bm_ops;
} bhv_module_list_t;
-STATIC DEFINE_SPINLOCK(bhv_lock);
-STATIC struct list_head bhv_list = LIST_HEAD_INIT(bhv_list);
+
+static DEFINE_SPINLOCK(bhv_lock);
+static struct list_head bhv_list = LIST_HEAD_INIT(bhv_list);
void
bhv_module_init(
Index: 2.6.x-xfs-new/fs/xfs/quota/xfs_dquot_item.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/quota/xfs_dquot_item.c 2006-11-22 14:41:06.300776336 +1100
+++ 2.6.x-xfs-new/fs/xfs/quota/xfs_dquot_item.c 2006-11-22 14:42:21.626964826 +1100
@@ -399,7 +399,7 @@ xfs_qm_dquot_logitem_committing(
/*
* This is the ops vector for dquots
*/
-STATIC struct xfs_item_ops xfs_dquot_item_ops = {
+static struct xfs_item_ops xfs_dquot_item_ops = {
.iop_size = (uint(*)(xfs_log_item_t*))xfs_qm_dquot_logitem_size,
.iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*))
xfs_qm_dquot_logitem_format,
@@ -606,7 +606,7 @@ xfs_qm_qoffend_logitem_committing(xfs_qo
return;
}
-STATIC struct xfs_item_ops xfs_qm_qoffend_logitem_ops = {
+static struct xfs_item_ops xfs_qm_qoffend_logitem_ops = {
.iop_size = (uint(*)(xfs_log_item_t*))xfs_qm_qoff_logitem_size,
.iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*))
xfs_qm_qoff_logitem_format,
@@ -628,7 +628,7 @@ STATIC struct xfs_item_ops xfs_qm_qoffen
/*
* This is the ops vector shared by all quotaoff-start log items.
*/
-STATIC struct xfs_item_ops xfs_qm_qoff_logitem_ops = {
+static struct xfs_item_ops xfs_qm_qoff_logitem_ops = {
.iop_size = (uint(*)(xfs_log_item_t*))xfs_qm_qoff_logitem_size,
.iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*))
xfs_qm_qoff_logitem_format,
Index: 2.6.x-xfs-new/fs/xfs/quota/xfs_qm.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/quota/xfs_qm.c 2006-11-22 14:41:06.316774252 +1100
+++ 2.6.x-xfs-new/fs/xfs/quota/xfs_qm.c 2006-11-22 14:42:21.626964826 +1100
@@ -64,10 +64,10 @@ uint ndquot;
kmem_zone_t *qm_dqzone;
kmem_zone_t *qm_dqtrxzone;
-STATIC kmem_shaker_t xfs_qm_shaker;
+static kmem_shaker_t xfs_qm_shaker;
-STATIC cred_t xfs_zerocr;
-STATIC xfs_inode_t xfs_zeroino;
+static cred_t xfs_zerocr;
+static xfs_inode_t xfs_zeroino;
STATIC void xfs_qm_list_init(xfs_dqlist_t *, char *, int);
STATIC void xfs_qm_list_destroy(xfs_dqlist_t *);
Index: 2.6.x-xfs-new/fs/xfs/quota/xfs_qm_bhv.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/quota/xfs_qm_bhv.c 2006-11-22 14:41:06.316774252 +1100
+++ 2.6.x-xfs-new/fs/xfs/quota/xfs_qm_bhv.c 2006-11-22 14:42:21.630964305 +1100
@@ -384,7 +384,7 @@ xfs_qm_dqrele_null(
}
-STATIC struct xfs_qmops xfs_qmcore_xfs = {
+static struct xfs_qmops xfs_qmcore_xfs = {
.xfs_qminit = xfs_qm_newmount,
.xfs_qmdone = xfs_qm_unmount_quotadestroy,
.xfs_qmmount = xfs_qm_endmount,
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_vnode.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_vnode.c 2006-11-22 14:41:06.120799781 +1100
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_vnode.c 2006-11-22 14:42:21.630964305 +1100
@@ -26,7 +26,7 @@ DEFINE_SPINLOCK(vnumber_lock);
*/
#define NVSYNC 37
#define vptosync(v) (&vsync[((unsigned long)v) % NVSYNC])
-STATIC wait_queue_head_t vsync[NVSYNC];
+static wait_queue_head_t vsync[NVSYNC];
void
vn_init(void)
Index: 2.6.x-xfs-new/fs/xfs/xfs_refcache.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_refcache.c 2006-11-22 14:41:06.324773210 +1100
+++ 2.6.x-xfs-new/fs/xfs/xfs_refcache.c 2006-11-22 14:42:21.630964305 +1100
@@ -45,11 +45,11 @@
#include "xfs_buf_item.h"
#include "xfs_refcache.h"
-STATIC spinlock_t xfs_refcache_lock = SPIN_LOCK_UNLOCKED;
-STATIC xfs_inode_t **xfs_refcache;
-STATIC int xfs_refcache_index;
-STATIC int xfs_refcache_busy;
-STATIC int xfs_refcache_count;
+static spinlock_t xfs_refcache_lock = SPIN_LOCK_UNLOCKED;
+static xfs_inode_t **xfs_refcache;
+static int xfs_refcache_index;
+static int xfs_refcache_busy;
+static int xfs_refcache_count;
/*
* Insert the given inode into the reference cache.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 1/2] Make stuff static
2006-11-22 4:24 ` David Chinner
@ 2006-11-22 4:53 ` David Chatterton
2006-11-22 16:13 ` Eric Sandeen
2006-11-29 7:31 ` David Chinner
2006-11-26 14:05 ` Eric Sandeen
1 sibling, 2 replies; 24+ messages in thread
From: David Chatterton @ 2006-11-22 4:53 UTC (permalink / raw)
To: David Chinner; +Cc: Russell Cattelan, Tim Shimmin, Eric Sandeen, xfs
David Chinner wrote:
> On Tue, Nov 21, 2006 at 07:09:43PM -0600, Russell Cattelan wrote:
>> On Wed, 2006-11-22 at 11:42 +1100, David Chinner wrote:
>>> Ok, so I've had time to look at this again. Here's the definitions
>>> of STATIC and STATIC_INLINE for debug and nondebug from the
>>> patch (whitespace damaged):
> .....
>>> Is this acceptible to everyone?
>> Yup.
>>
>>> FWIW, there is one other thing that this conversion causes
>>> problems with, and that's variable definitions. i.e. we can't
>>> use STATIC on them any more because of the "noinline" attribute
>>> it has. Do we care about this and if so, any suggestions on
>>> how to keep this functionality (a different STATIC_xxx define
>>> for structures)?
>> So I know things like systemtap kgdb oprofile all work better when
>> functions are not static, but what about variables/structures?
>> do things really get that confused?
>> Maybe we shouldn't worry about conditioning them and just make them
>> static
>
> Ok, so I'd already converted them to static where necessary.
> Attached is thecomplete patch. On ia64, the size of the xfs.ko
> and xfs_quota.ko modules decreases with this patch:
>
> Orig:
>
> -rw-rw-r-- 1 dgc ptg 1662416 2006-11-22 14:41 fs/xfs/quota/xfs_quota.ko
> -rw-rw-r-- 1 dgc ptg 856748 2006-11-22 14:41 fs/xfs/xfsidbg.ko
> -rw-rw-r-- 1 dgc ptg 13614719 2006-11-22 14:41 fs/xfs/xfs.ko
>
> With patch:
>
> -rw-rw-r-- 1 dgc ptg 1657814 2006-11-22 14:42 fs/xfs/quota/xfs_quota.ko
> -rw-rw-r-- 1 dgc ptg 856748 2006-11-22 14:42 fs/xfs/xfsidbg.ko
> -rw-rw-r-- 1 dgc ptg 13557579 2006-11-22 14:42 fs/xfs/xfs.ko
>
> The original top 10 stack users:
>
> 0x000e10c6 xfs_vn_mknod [xfs]: 576
> 0x000ddfe6 xfs_ioctl [xfs]: 368
> 0x000e1f46 xfs_vn_symlink [xfs]: 368
> 0x000345a6 xfs_bmapi [xfs]: 320
> 0x000b1146 _xfs_trans_commit [xfs]: 272
> 0x000c59c6 xfs_change_file_space [xfs]: 272
> 0x0003a6a6 xfs_bunmapi [xfs]: 240
> 0x000afa06 xfs_trans_unreserve_and_mod_sb [xfs]: 224
> 0x00040626 xfs_bmbt_insert [xfs]: 192
> 0x0008be26 xfs_iomap_write_delay [xfs]: 192
>
> [64 functions with stack usage larger than 100 bytes]
>
> With patch:
>
> 0x000b7c46 _xfs_trans_commit [xfs]: 272
> 0x000b5426 xfs_trans_unreserve_and_mod_sb [xfs]: 224
> 0x000e4106 xfs_find_handle [xfs]: 224
> 0x000396c6 xfs_bmapi [xfs]: 208
> 0x00090066 xfs_iomap_write_delay [xfs]: 208
> 0x000e9046 xfs_cleanup_inode [xfs]: 208
> 0x000058c6 xfs_acl_setmode [xfs]: 160
> 0x00005f46 xfs_acl_allow_set [xfs]: 160
> 0x000067c6 xfs_acl_vtoacl [xfs]: 160
> 0x00007366 xfs_acl_vget [xfs]: 160
>
> [69 functions with stack usage larger than 100 bytes]
>
> Performance appears to be slight faster with the noinline
> patch, but the variation is within the error margins of
> my measurements so I'd say it's neutral.
>
> Comments?
>
Just reducing xfs_bmapi by 118 bytes makes this worthwhile doesn't it?
Out of interest, what estimated improvement does this have on one of Jesper's
stacks?
Should we be concerned that there are now more functions with 100 or more bytes?
David
--
David Chatterton
XFS Engineering Manager
SGI Australia
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] Make stuff static
2006-11-22 4:53 ` David Chatterton
@ 2006-11-22 16:13 ` Eric Sandeen
2006-11-29 7:31 ` David Chinner
1 sibling, 0 replies; 24+ messages in thread
From: Eric Sandeen @ 2006-11-22 16:13 UTC (permalink / raw)
To: chatz; +Cc: David Chinner, Russell Cattelan, Tim Shimmin, xfs
David Chatterton wrote:
>
> David Chinner wrote:
>> Comments?
>>
>
> Just reducing xfs_bmapi by 118 bytes makes this worthwhile doesn't it?
>
> Out of interest, what estimated improvement does this have on one of Jesper's
> stacks?
>
> Should we be concerned that there are now more functions with 100 or more bytes?
I don't think we need to worry about that, it is probably in the noise.
There will almost certainly be fallout from this change w.r.t. 4k
stacks. It should probably at least be tested on 4k stacks over a fairly
complex volume setup to see.
Also with respect to stack usage, is there extra stack space used, in
addition to the explicit %esp adjustments, to set up each function call?
IOW is the total more than the sum of the parts? :)
I'm glad to hear that there's no apparent performance penalty....
-eric
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] Make stuff static
2006-11-22 4:53 ` David Chatterton
2006-11-22 16:13 ` Eric Sandeen
@ 2006-11-29 7:31 ` David Chinner
1 sibling, 0 replies; 24+ messages in thread
From: David Chinner @ 2006-11-29 7:31 UTC (permalink / raw)
To: David Chatterton
Cc: David Chinner, Russell Cattelan, Tim Shimmin, Eric Sandeen, xfs
On Wed, Nov 22, 2006 at 03:53:49PM +1100, David Chatterton wrote:
> David Chinner wrote:
> > Attached is thecomplete patch. On ia64, the size of the xfs.ko
> > and xfs_quota.ko modules decreases with this patch:
....
> > Performance appears to be slight faster with the noinline
> > patch, but the variation is within the error margins of
> > my measurements so I'd say it's neutral.
> >
> > Comments?
> >
>
> Just reducing xfs_bmapi by 118 bytes makes this worthwhile doesn't it?
Well, this is ia64, so stack usage really isn't the issue there.
A performance degradation was really what I really care about with
this change on ia64.
And the increase in xfs_bmap_btalloc() offsets this saving as well....
> Out of interest, what estimated improvement does this have on one of Jesper's
> stacks?
Depends on the compiler. For gcc 3.3.5, it makes no difference at
all because it doesn't automatically inline static functions. The
problem we're trying to address here is the agressive inlining that
gcc 4.x does of static functions that increases the stack usage
of critical functions.
e.g we've got in the code:
xfs_bmapi()
xfs_bmap_alloc()
xfs_bmap_btalloc()
xfs_bmap_{bt}alloc() are static, single use functions, and so gcc 4.x
inlines them and the stack usage of all three functions is brought
into xfs_bmapi().
Now in some cases this is a slight win in terms in stack usage
if the code always passes through that path, but if the inlined
functions are leaf functions, then we increase stack usage for
no gain.
The clearest example of this on i386 is xfs_page_state_convert(),
which goes from 368 bytes of stack usage to 160 bytes of stack usage
once noinline is used on i386. There's over 200 bytes of extra stack
used by inlining functions that are not in the critical path.
Basically, we can factor the code all we like, but while gcc is
undoing that factoring to "go fast" we are fighting a losing battle.
Hence the noinline change is needed first to make the code stack how
we want it to, not how the compiler thinks it should.
> Should we be concerned that there are now more functions with 100 or more bytes?
Not really - the work that Jesper and Keith have done shows us that
we've got a certain set of functions that we really need to
concentrate on and once we've dealt with them we can start looking
at other leaf functions that are stack hogs....
Cheers,
Dave.
--
Dave Chinner
Principal Engineer
SGI Australian Software Group
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] Make stuff static
2006-11-22 4:24 ` David Chinner
2006-11-22 4:53 ` David Chatterton
@ 2006-11-26 14:05 ` Eric Sandeen
1 sibling, 0 replies; 24+ messages in thread
From: Eric Sandeen @ 2006-11-26 14:05 UTC (permalink / raw)
To: David Chinner; +Cc: Russell Cattelan, Tim Shimmin, xfs
David Chinner wrote:
> Performance appears to be slight faster with the noinline
> patch, but the variation is within the error margins of
> my measurements so I'd say it's neutral.
>
> Comments?
with fewer inlines & more function calls, what about stack frames adding
up? can we measure that?
-Eric
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] Make stuff static
2006-10-17 21:57 ` David Chinner
2006-10-17 22:45 ` Russell Cattelan
@ 2006-10-18 4:06 ` Timothy Shimmin
1 sibling, 0 replies; 24+ messages in thread
From: Timothy Shimmin @ 2006-10-18 4:06 UTC (permalink / raw)
To: David Chinner; +Cc: Russell Cattelan, Eric Sandeen, xfs
Dave,
--On 18 October 2006 7:57:06 AM +1000 David Chinner <dgc@sgi.com> wrote:
> On Tue, Oct 17, 2006 at 05:13:01PM +1000, Tim Shimmin wrote:
>> --On 17 October 2006 10:50:38 AM +1000 David Chinner <dgc@sgi.com> wrote:
>> > Fix them - inline functions in header files should always be "static
>> > inline". Inline functions in .c files should always be static as
>> > well - if they need to be accessed from different source files then
>> > they need to be in header files. Hence "STATIC inline" is broken
>> > code and should be fixed anyway. Luckily, there are very few of
>> > these to fix and they are all in .c files:
>> >
>> > chook 137% grep -rIw "STATIC inline" fs/xfs | wc -l
>> > 21
>>
>> So you are saying that "static inline"s should always be.
>
> Yup.
>
>> So for CONFIG_XFS_DEBUG where we define STATIC and so make static
>> disappear for uses of STATIC, we will no longer touch these
>> "static inline" functions.
>
> Yup. The function will still get inlined, so changing it's scope on
> debug builds doesn't provide any benefit IMO. FWIW, for debug
> builds we probably want noinline....
>
Yep. That's what I was meaning.
On debug I want the STATIC_INLINE's to go away (well, noinline),
so that I can see the funcs in the debug trace.
>> I thought that for debug, we could stop them from being inline
>> for easier debugging. We could have a STATIC_INLINE :-)
>
> We could, but I don't think it gains us anything.
It gives us what we said above, i.e. on debug they become noinline.
And like Russell said, it makes it clearer what is happening.
On non-debug,
STATIC -> static noinline
STATIC_INLINE -> static inline
On debug,
STATIC -> noinline
STATIC_INLINE -> noinline
Although for STATIC_INLINE in headers, it won't work on debug b/c of
multiple definitions. D'oh.
--Tim
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2006-11-29 7:32 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-09-29 3:28 [PATCH 1/2] Make stuff static sandeen
2006-10-14 4:31 ` Eric Sandeen
2006-10-16 9:12 ` Timothy Shimmin
2006-10-16 13:49 ` Eric Sandeen
2006-10-16 21:34 ` Eric Sandeen
2006-10-16 23:22 ` David Chinner
2006-10-16 23:55 ` Russell Cattelan
2006-10-17 0:50 ` David Chinner
2006-10-17 1:03 ` Eric Sandeen
2006-10-17 3:09 ` David Chinner
2006-10-17 3:18 ` Nathan Scott
2006-10-18 0:56 ` David Chinner
2006-10-17 7:13 ` Tim Shimmin
2006-10-17 21:57 ` David Chinner
2006-10-17 22:45 ` Russell Cattelan
2006-11-22 0:42 ` David Chinner
2006-11-22 1:09 ` Russell Cattelan
2006-11-22 2:16 ` David Chatterton
2006-11-22 4:24 ` David Chinner
2006-11-22 4:53 ` David Chatterton
2006-11-22 16:13 ` Eric Sandeen
2006-11-29 7:31 ` David Chinner
2006-11-26 14:05 ` Eric Sandeen
2006-10-18 4:06 ` Timothy Shimmin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox