[PATCH 0/3] xfsprogs: more libxfs/ spring cleaning

[PATCH 0/3] xfsprogs: more libxfs/ spring cleaning

[Eric Sandeen]

It wasn't super clear before - my goal here is to keep reducing
cosmetic differences between kernelspace & userspace libxfs/*
files and functions.

To that end, 3 more patches ... the first one may requires someone
who groks the libxfs_* API namespace picture (looking at you, Dave!)

(this abandons the "make new files" patches I sent before, at least
for now, I'll heed dave's advice to minimize moves...)

Thanks,
-Eric

[PATCH 1/3] libxfs: rename shared kernel functions from libxfs_ to xfs_

[Eric Sandeen]

The libxfs_* function namespace has gotten a bit confused and
muddled; the general idea is that functions called from userspace
utilities should use the libxfs_* namespace.  In many cases
we use #defines to define xfs_* namespace to libxfs_*; in other
cases we have explicitly defined libxfs_* functions which are clear
counterparts or even clones of kernel libxfs/* functions.

For any function definitions within libxfs/* which match kernel
names, give them standard xfs_* names to further reduce differnces
between userspace and kernel libxfs/* code.

Then add #defines to libxfs_* for any functions which are needed
by utilities, as is done with other core functionality.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/include/libxfs.h b/include/libxfs.h
index 230bc3e8..ceebccdc 100644
--- a/include/libxfs.h
+++ b/include/libxfs.h
@@ -151,7 +151,7 @@ extern int	libxfs_log_header(char *, uuid_t *, int, int, int, xfs_lsn_t,
 
 /* Shared utility routines */
 
-extern int	libxfs_alloc_file_space (struct xfs_inode *, xfs_off_t,
+extern int	xfs_alloc_file_space (struct xfs_inode *, xfs_off_t,
 				xfs_off_t, int, int);
 
 /* XXX: this is messy and needs fixing */
diff --git a/include/xfs_inode.h b/include/xfs_inode.h
index 88b58ac3..3e7e80ea 100644
--- a/include/xfs_inode.h
+++ b/include/xfs_inode.h
@@ -139,21 +139,21 @@ typedef struct cred {
 	gid_t	cr_gid;
 } cred_t;
 
-extern int	libxfs_inode_alloc (struct xfs_trans **, struct xfs_inode *,
+extern int	xfs_inode_alloc (struct xfs_trans **, struct xfs_inode *,
 				mode_t, nlink_t, xfs_dev_t, struct cred *,
 				struct fsxattr *, struct xfs_inode **);
-extern void	libxfs_trans_inode_alloc_buf (struct xfs_trans *,
+extern void	xfs_trans_inode_alloc_buf (struct xfs_trans *,
 				struct xfs_buf *);
 
-extern void	libxfs_trans_ichgtime(struct xfs_trans *,
+extern void	xfs_trans_ichgtime(struct xfs_trans *,
 				struct xfs_inode *, int);
-extern int	libxfs_iflush_int (struct xfs_inode *, struct xfs_buf *);
+extern int	xfs_iflush_int (struct xfs_inode *, struct xfs_buf *);
 
 /* Inode Cache Interfaces */
-extern bool	libxfs_inode_verify_forks(struct xfs_inode *ip);
-extern int	libxfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
+extern bool	xfs_inode_verify_forks(struct xfs_inode *ip);
+extern int	xfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
 				uint, struct xfs_inode **,
 				struct xfs_ifork_ops *);
-extern void	libxfs_irele(struct xfs_inode *ip);
+extern void	xfs_irele(struct xfs_inode *ip);
 
 #endif /* __XFS_INODE_H__ */
diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index 10b74538..d32acc9e 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -75,46 +75,46 @@ typedef struct xfs_trans {
 void	xfs_trans_init(struct xfs_mount *);
 int	xfs_trans_roll(struct xfs_trans **);
 
-int	libxfs_trans_alloc(struct xfs_mount *mp, struct xfs_trans_res *resp,
+int	xfs_trans_alloc(struct xfs_mount *mp, struct xfs_trans_res *resp,
 			   uint blocks, uint rtextents, uint flags,
 			   struct xfs_trans **tpp);
 int	libxfs_trans_alloc_rollable(struct xfs_mount *mp, uint blocks,
 				    struct xfs_trans **tpp);
-int	libxfs_trans_alloc_empty(struct xfs_mount *mp, struct xfs_trans **tpp);
-int	libxfs_trans_commit(struct xfs_trans *);
-void	libxfs_trans_cancel(struct xfs_trans *);
+int	xfs_trans_alloc_empty(struct xfs_mount *mp, struct xfs_trans **tpp);
+int	xfs_trans_commit(struct xfs_trans *);
+void	xfs_trans_cancel(struct xfs_trans *);
 
 /* cancel dfops associated with a transaction */
 void xfs_defer_cancel(struct xfs_trans *);
 
-struct xfs_buf *libxfs_trans_getsb(struct xfs_trans *, struct xfs_mount *, int);
+struct xfs_buf *xfs_trans_getsb(struct xfs_trans *, struct xfs_mount *, int);
 
-void	libxfs_trans_ijoin(struct xfs_trans *, struct xfs_inode *, uint);
-void	libxfs_trans_log_inode (struct xfs_trans *, struct xfs_inode *,
+void	xfs_trans_ijoin(struct xfs_trans *, struct xfs_inode *, uint);
+void	xfs_trans_log_inode (struct xfs_trans *, struct xfs_inode *,
 				uint);
-int	libxfs_trans_roll_inode (struct xfs_trans **, struct xfs_inode *);
-
-void	libxfs_trans_brelse(struct xfs_trans *, struct xfs_buf *);
-void	libxfs_trans_binval(struct xfs_trans *, struct xfs_buf *);
-void	libxfs_trans_bjoin(struct xfs_trans *, struct xfs_buf *);
-void	libxfs_trans_bhold(struct xfs_trans *, struct xfs_buf *);
-void	libxfs_trans_dirty_buf(struct xfs_trans *, struct xfs_buf *);
-void	libxfs_trans_log_buf(struct xfs_trans *, struct xfs_buf *,
+int	xfs_trans_roll_inode (struct xfs_trans **, struct xfs_inode *);
+
+void	xfs_trans_brelse(struct xfs_trans *, struct xfs_buf *);
+void	xfs_trans_binval(struct xfs_trans *, struct xfs_buf *);
+void	xfs_trans_bjoin(struct xfs_trans *, struct xfs_buf *);
+void	xfs_trans_bhold(struct xfs_trans *, struct xfs_buf *);
+void	xfs_trans_dirty_buf(struct xfs_trans *, struct xfs_buf *);
+void	xfs_trans_log_buf(struct xfs_trans *, struct xfs_buf *,
 				uint, uint);
-bool	libxfs_trans_ordered_buf(xfs_trans_t *, struct xfs_buf *);
+bool	xfs_trans_ordered_buf(xfs_trans_t *, struct xfs_buf *);
 
-struct xfs_buf	*libxfs_trans_get_buf_map(struct xfs_trans *tp,
+struct xfs_buf	*xfs_trans_get_buf_map(struct xfs_trans *tp,
 					struct xfs_buftarg *btp,
 					struct xfs_buf_map *map, int nmaps,
 					uint flags);
 
-int	libxfs_trans_read_buf_map(struct xfs_mount *mp, struct xfs_trans *tp,
+int	xfs_trans_read_buf_map(struct xfs_mount *mp, struct xfs_trans *tp,
 				  struct xfs_buftarg *btp,
 				  struct xfs_buf_map *map, int nmaps,
 				  uint flags, struct xfs_buf **bpp,
 				  const struct xfs_buf_ops *ops);
 static inline struct xfs_buf *
-libxfs_trans_get_buf(
+xfs_trans_get_buf(
 	struct xfs_trans	*tp,
 	struct xfs_buftarg	*btp,
 	xfs_daddr_t		blkno,
@@ -122,11 +122,11 @@ libxfs_trans_get_buf(
 	uint			flags)
 {
 	DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
-	return libxfs_trans_get_buf_map(tp, btp, &map, 1, flags);
+	return xfs_trans_get_buf_map(tp, btp, &map, 1, flags);
 }
 
 static inline int
-libxfs_trans_read_buf(
+xfs_trans_read_buf(
 	struct xfs_mount	*mp,
 	struct xfs_trans	*tp,
 	struct xfs_buftarg	*btp,
@@ -137,7 +137,7 @@ libxfs_trans_read_buf(
 	const struct xfs_buf_ops *ops)
 {
 	DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
-	return libxfs_trans_read_buf_map(mp, tp, btp, &map, 1, flags, bpp, ops);
+	return xfs_trans_read_buf_map(mp, tp, btp, &map, 1, flags, bpp, ops);
 }
 
 #endif	/* __XFS_TRANS_H__ */
diff --git a/libxfs/init.c b/libxfs/init.c
index 2f6decc8..4b402a6e 100644
--- a/libxfs/init.c
+++ b/libxfs/init.c
@@ -449,7 +449,7 @@ rtmount_init(
 }
 
 static int
-libxfs_initialize_perag(
+xfs_initialize_perag(
 	xfs_mount_t	*mp,
 	xfs_agnumber_t	agcount,
 	xfs_agnumber_t	*maxagi)
@@ -790,7 +790,7 @@ libxfs_mount(
 	}
 
 	/*
-	 * libxfs_initialize_perag will allocate a perag structure for each ag.
+	 * xfs_initialize_perag will allocate a perag structure for each ag.
 	 * If agcount is corrupted and insanely high, this will OOM the box.
 	 * If the agount seems (arbitrarily) high, try to read what would be
 	 * the last AG, and if that fails for a relatively high agcount, just
@@ -812,7 +812,7 @@ libxfs_mount(
 		libxfs_putbuf(bp);
 	}
 
-	error = libxfs_initialize_perag(mp, sbp->sb_agcount, &mp->m_maxagi);
+	error = xfs_initialize_perag(mp, sbp->sb_agcount, &mp->m_maxagi);
 	if (error) {
 		fprintf(stderr, _("%s: perag init failed\n"),
 			progname);
@@ -826,9 +826,9 @@ void
 libxfs_rtmount_destroy(xfs_mount_t *mp)
 {
 	if (mp->m_rsumip)
-		libxfs_irele(mp->m_rsumip);
+		xfs_irele(mp->m_rsumip);
 	if (mp->m_rbmip)
-		libxfs_irele(mp->m_rbmip);
+		xfs_irele(mp->m_rbmip);
 	mp->m_rsumip = mp->m_rbmip = NULL;
 }
 
diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
index 1150ec93..2b8ac5ab 100644
--- a/libxfs/libxfs_api_defs.h
+++ b/libxfs/libxfs_api_defs.h
@@ -17,6 +17,7 @@
 #define xfs_highbit64			libxfs_highbit64
 
 #define xfs_trans_alloc			libxfs_trans_alloc
+#define xfs_trans_alloc_rollable	libxfs_trans_alloc_rollable
 #define xfs_trans_alloc_empty		libxfs_trans_alloc_empty
 #define xfs_trans_add_item		libxfs_trans_add_item
 #define xfs_trans_bhold			libxfs_trans_bhold
@@ -90,6 +91,9 @@
 #define xfs_idestroy_fork		libxfs_idestroy_fork
 #define xfs_inode_validate_extsize	libxfs_inode_validate_extsize
 #define xfs_inode_validate_cowextsize	libxfs_inode_validate_cowextsize
+#define xfs_inode_alloc			libxfs_inode_alloc
+#define xfs_iflush_int			libxfs_iflush_int
+#define xfs_alloc_file_space		libxfs_alloc_file_space
 
 #define xfs_rmap_alloc			libxfs_rmap_alloc
 #define xfs_rmap_query_range		libxfs_rmap_query_range
@@ -144,4 +148,9 @@
 #define xfs_fs_geometry			libxfs_fs_geometry
 #define xfs_init_local_fork		libxfs_init_local_fork
 
+#define xfs_getsb			libxfs_getsb
+#define xfs_irele			libxfs_irele
+#define xfs_iget			libxfs_iget
+#define xfs_inode_verify_forks		libxfs_inode_verify_forks
+
 #endif /* __LIBXFS_API_DEFS_H__ */
diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index 79ffd470..8fa2c2c5 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -177,7 +177,7 @@ extern void	libxfs_putbuf (xfs_buf_t *);
 
 extern void	libxfs_readbuf_verify(struct xfs_buf *bp,
 			const struct xfs_buf_ops *ops);
-extern xfs_buf_t *libxfs_getsb(struct xfs_mount *, int);
+extern xfs_buf_t *xfs_getsb(struct xfs_mount *, int);
 extern void	libxfs_bcache_purge(void);
 extern void	libxfs_bcache_free(void);
 extern void	libxfs_bcache_flush(void);
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index d668a157..157a99d6 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -559,7 +559,7 @@ typedef int (*xfs_rtalloc_query_range_fn)(
 	struct xfs_rtalloc_rec	*rec,
 	void			*priv);
 
-int libxfs_zero_extent(struct xfs_inode *ip, xfs_fsblock_t start_fsb,
+int xfs_zero_extent(struct xfs_inode *ip, xfs_fsblock_t start_fsb,
                         xfs_off_t count_fsb);
 
 
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index e3ff5842..8d6f958a 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -476,7 +476,7 @@ libxfs_trace_putbuf(const char *func, const char *file, int line, xfs_buf_t *bp)
 
 
 xfs_buf_t *
-libxfs_getsb(xfs_mount_t *mp, int flags)
+xfs_getsb(xfs_mount_t *mp, int flags)
 {
 	return libxfs_readbuf(mp->m_ddev_targp, XFS_SB_DADDR,
 				XFS_FSS_TO_BB(mp, 1), flags, &xfs_sb_buf_ops);
@@ -1374,7 +1374,7 @@ extern kmem_zone_t	*xfs_ili_zone;
  * make sure they're not corrupt.
  */
 bool
-libxfs_inode_verify_forks(
+xfs_inode_verify_forks(
 	struct xfs_inode	*ip)
 {
 	struct xfs_ifork	*ifp;
@@ -1403,7 +1403,7 @@ libxfs_inode_verify_forks(
 }
 
 int
-libxfs_iget(
+xfs_iget(
 	struct xfs_mount	*mp,
 	struct xfs_trans	*tp,
 	xfs_ino_t		ino,
@@ -1428,8 +1428,8 @@ libxfs_iget(
 	}
 
 	ip->i_fork_ops = ifork_ops;
-	if (!libxfs_inode_verify_forks(ip)) {
-		libxfs_irele(ip);
+	if (!xfs_inode_verify_forks(ip)) {
+		xfs_irele(ip);
 		return -EFSCORRUPTED;
 	}
 
@@ -1446,26 +1446,26 @@ libxfs_iget(
 }
 
 static void
-libxfs_idestroy(xfs_inode_t *ip)
+xfs_idestroy(xfs_inode_t *ip)
 {
 	switch (VFS_I(ip)->i_mode & S_IFMT) {
 		case S_IFREG:
 		case S_IFDIR:
 		case S_IFLNK:
-			libxfs_idestroy_fork(ip, XFS_DATA_FORK);
+			xfs_idestroy_fork(ip, XFS_DATA_FORK);
 			break;
 	}
 	if (ip->i_afp)
-		libxfs_idestroy_fork(ip, XFS_ATTR_FORK);
+		xfs_idestroy_fork(ip, XFS_ATTR_FORK);
 	if (ip->i_cowfp)
 		xfs_idestroy_fork(ip, XFS_COW_FORK);
 }
 
 void
-libxfs_irele(
+xfs_irele(
 	struct xfs_inode	*ip)
 {
 	ASSERT(ip->i_itemp == NULL);
-	libxfs_idestroy(ip);
+	xfs_idestroy(ip);
 	kmem_zone_free(xfs_inode_zone, ip);
 }
diff --git a/libxfs/trans.c b/libxfs/trans.c
index cb15552c..5ef0c055 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -36,7 +36,7 @@ kmem_zone_t	*xfs_trans_zone;
  * in the mount structure.
  */
 void
-libxfs_trans_init(
+xfs_trans_init(
 	struct xfs_mount	*mp)
 {
 	xfs_trans_resv_calc(mp, &mp->m_resv);
@@ -46,7 +46,7 @@ libxfs_trans_init(
  * Add the given log item to the transaction's list of log items.
  */
 void
-libxfs_trans_add_item(
+xfs_trans_add_item(
 	struct xfs_trans	*tp,
 	struct xfs_log_item	*lip)
 {
@@ -62,7 +62,7 @@ libxfs_trans_add_item(
  * Unlink and free the given descriptor.
  */
 void
-libxfs_trans_del_item(
+xfs_trans_del_item(
 	struct xfs_log_item	*lip)
 {
 	clear_bit(XFS_LI_DIRTY, &lip->li_flags);
@@ -77,7 +77,7 @@ libxfs_trans_del_item(
  * chunk we've been working on and get a new transaction to continue.
  */
 int
-libxfs_trans_roll(
+xfs_trans_roll(
 	struct xfs_trans	**tpp)
 {
 	struct xfs_trans	*trans = *tpp;
@@ -245,7 +245,7 @@ undo_blocks:
 }
 
 int
-libxfs_trans_alloc(
+xfs_trans_alloc(
 	struct xfs_mount	*mp,
 	struct xfs_trans_res	*resp,
 	unsigned int		blocks,
@@ -289,7 +289,7 @@ libxfs_trans_alloc(
  * without any dirty data.
  */
 int
-libxfs_trans_alloc_empty(
+xfs_trans_alloc_empty(
 	struct xfs_mount		*mp,
 	struct xfs_trans		**tpp)
 {
@@ -304,17 +304,17 @@ libxfs_trans_alloc_empty(
  * permanent log reservation flag to avoid blowing asserts.
  */
 int
-libxfs_trans_alloc_rollable(
+xfs_trans_alloc_rollable(
 	struct xfs_mount	*mp,
 	unsigned int		blocks,
 	struct xfs_trans	**tpp)
 {
-	return libxfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, blocks,
+	return xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, blocks,
 			0, 0, tpp);
 }
 
 void
-libxfs_trans_cancel(
+xfs_trans_cancel(
 	struct xfs_trans	*tp)
 {
 #ifdef XACT_DEBUG
@@ -337,7 +337,7 @@ out:
 }
 
 void
-libxfs_trans_ijoin(
+xfs_trans_ijoin(
 	xfs_trans_t		*tp,
 	xfs_inode_t		*ip,
 	uint			lock_flags)
@@ -360,7 +360,7 @@ libxfs_trans_ijoin(
 }
 
 void
-libxfs_trans_inode_alloc_buf(
+xfs_trans_inode_alloc_buf(
 	xfs_trans_t		*tp,
 	xfs_buf_t		*bp)
 {
@@ -407,7 +407,7 @@ xfs_trans_log_inode(
 }
 
 int
-libxfs_trans_roll_inode(
+xfs_trans_roll_inode(
 	struct xfs_trans	**tpp,
 	struct xfs_inode	*ip)
 {
@@ -425,7 +425,7 @@ libxfs_trans_roll_inode(
  * Mark a buffer dirty in the transaction.
  */
 void
-libxfs_trans_dirty_buf(
+xfs_trans_dirty_buf(
 	struct xfs_trans	*tp,
 	struct xfs_buf		*bp)
 {
@@ -451,7 +451,7 @@ libxfs_trans_dirty_buf(
  * value of b_blkno.
  */
 void
-libxfs_trans_log_buf(
+xfs_trans_log_buf(
 	struct xfs_trans	*tp,
 	struct xfs_buf		*bp,
 	uint			first,
@@ -473,7 +473,7 @@ libxfs_trans_log_buf(
  * If the buffer is already dirty, trigger the "already logged" return condition.
  */
 bool
-libxfs_trans_ordered_buf(
+xfs_trans_ordered_buf(
 	struct xfs_trans	*tp,
 	struct xfs_buf		*bp)
 {
@@ -481,7 +481,7 @@ libxfs_trans_ordered_buf(
 	bool			ret;
 
 	ret = test_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags);
-	libxfs_trans_log_buf(tp, bp, 0, bp->b_bcount);
+	xfs_trans_log_buf(tp, bp, 0, bp->b_bcount);
 	return ret;
 }
 
@@ -496,7 +496,7 @@ xfs_buf_item_put(
 }
 
 void
-libxfs_trans_brelse(
+xfs_trans_brelse(
 	xfs_trans_t		*tp,
 	xfs_buf_t		*bp)
 {
@@ -531,7 +531,7 @@ libxfs_trans_brelse(
 }
 
 void
-libxfs_trans_binval(
+xfs_trans_binval(
 	xfs_trans_t		*tp,
 	xfs_buf_t		*bp)
 {
@@ -556,7 +556,7 @@ libxfs_trans_binval(
 }
 
 void
-libxfs_trans_bjoin(
+xfs_trans_bjoin(
 	xfs_trans_t		*tp,
 	xfs_buf_t		*bp)
 {
@@ -574,7 +574,7 @@ libxfs_trans_bjoin(
 }
 
 void
-libxfs_trans_bhold(
+xfs_trans_bhold(
 	xfs_trans_t		*tp,
 	xfs_buf_t		*bp)
 {
@@ -590,7 +590,7 @@ libxfs_trans_bhold(
 }
 
 xfs_buf_t *
-libxfs_trans_get_buf_map(
+xfs_trans_get_buf_map(
 	xfs_trans_t		*tp,
 	struct xfs_buftarg	*btp,
 	struct xfs_buf_map	*map,
@@ -619,14 +619,14 @@ libxfs_trans_get_buf_map(
 	fprintf(stderr, "trans_get_buf buffer %p, transaction %p\n", bp, tp);
 #endif
 
-	libxfs_trans_bjoin(tp, bp);
+	xfs_trans_bjoin(tp, bp);
 	bip = bp->b_log_item;
 	bip->bli_recur = 0;
 	return bp;
 }
 
 xfs_buf_t *
-libxfs_trans_getsb(
+xfs_trans_getsb(
 	xfs_trans_t		*tp,
 	xfs_mount_t		*mp,
 	int			flags)
@@ -637,7 +637,7 @@ libxfs_trans_getsb(
 	DEFINE_SINGLE_BUF_MAP(map, XFS_SB_DADDR, len);
 
 	if (tp == NULL)
-		return libxfs_getsb(mp, flags);
+		return xfs_getsb(mp, flags);
 
 	bp = xfs_trans_buf_item_match(tp, mp->m_dev, &map, 1);
 	if (bp != NULL) {
@@ -648,19 +648,19 @@ libxfs_trans_getsb(
 		return bp;
 	}
 
-	bp = libxfs_getsb(mp, flags);
+	bp = xfs_getsb(mp, flags);
 #ifdef XACT_DEBUG
 	fprintf(stderr, "trans_get_sb buffer %p, transaction %p\n", bp, tp);
 #endif
 
-	libxfs_trans_bjoin(tp, bp);
+	xfs_trans_bjoin(tp, bp);
 	bip = bp->b_log_item;
 	bip->bli_recur = 0;
 	return bp;
 }
 
 int
-libxfs_trans_read_buf_map(
+xfs_trans_read_buf_map(
 	xfs_mount_t		*mp,
 	xfs_trans_t		*tp,
 	struct xfs_buftarg	*btp,
@@ -728,7 +728,7 @@ out_relse:
  * Originally derived from xfs_trans_mod_sb().
  */
 void
-libxfs_trans_mod_sb(
+xfs_trans_mod_sb(
 	xfs_trans_t		*tp,
 	uint			field,
 	long			delta)
@@ -814,7 +814,7 @@ inode_item_done(
 	 * of whether the flush succeed or not. If we fail the flush, make sure
 	 * we still release the buffer reference we currently hold.
 	 */
-	error = libxfs_iflush_int(ip, bp);
+	error = xfs_iflush_int(ip, bp);
 	bp->b_transp = NULL;	/* remove xact ptr */
 
 	if (error) {
@@ -989,7 +989,7 @@ out_unreserve:
 }
 
 int
-libxfs_trans_commit(
+xfs_trans_commit(
 	struct xfs_trans	*tp)
 {
 	return __xfs_trans_commit(tp, false);
diff --git a/libxfs/util.c b/libxfs/util.c
index 8c9954f6..4901123a 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -143,7 +143,7 @@ xfs_log_calc_unit_res(
  * where it's no longer worth the hassle of maintaining common code.
  */
 void
-libxfs_trans_ichgtime(
+xfs_trans_ichgtime(
 	struct xfs_trans	*tp,
 	struct xfs_inode	*ip,
 	int			flags)
@@ -232,7 +232,7 @@ xfs_flags2diflags2(
  * where it's no longer worth the hassle of maintaining common code.
  */
 static int
-libxfs_ialloc(
+xfs_ialloc(
 	xfs_trans_t	*tp,
 	xfs_inode_t	*pip,
 	mode_t		mode,
@@ -262,7 +262,7 @@ libxfs_ialloc(
 	}
 	ASSERT(*ialloc_context == NULL);
 
-	error = libxfs_iget(tp->t_mountp, tp, ino, 0, &ip,
+	error = xfs_iget(tp->t_mountp, tp, ino, 0, &ip,
 			&xfs_default_ifork_ops);
 	if (error != 0)
 		return error;
@@ -388,7 +388,7 @@ libxfs_ialloc(
  * Originally based on xfs_iflush_int() from xfs_inode.c in the kernel.
  */
 int
-libxfs_iflush_int(xfs_inode_t *ip, xfs_buf_t *bp)
+xfs_iflush_int(xfs_inode_t *ip, xfs_buf_t *bp)
 {
 	xfs_inode_log_item_t	*iip;
 	xfs_dinode_t		*dip;
@@ -421,7 +421,7 @@ libxfs_iflush_int(xfs_inode_t *ip, xfs_buf_t *bp)
 		VFS_I(ip)->i_version++;
 
 	/* Check the inline fork data before we write out. */
-	if (!libxfs_inode_verify_forks(ip))
+	if (!xfs_inode_verify_forks(ip))
 		return -EFSCORRUPTED;
 
 	/*
@@ -467,10 +467,9 @@ libxfs_mod_incore_sb(
 
 /*
  * This routine allocates disk space for the given file.
- * Originally derived from xfs_alloc_file_space().
  */
 int
-libxfs_alloc_file_space(
+xfs_alloc_file_space(
 	xfs_inode_t	*ip,
 	xfs_off_t	offset,
 	xfs_off_t	len,
@@ -547,14 +546,14 @@ error0:	/* Cancel bmap, cancel trans */
 }
 
 /*
- * Wrapper around call to libxfs_ialloc. Takes care of committing and
+ * Wrapper around call to xfs_ialloc. Takes care of committing and
  * allocating a new transaction as needed.
  *
  * Originally there were two copies of this code - one in mkfs, the
  * other in repair - now there is just the one.
  */
 int
-libxfs_inode_alloc(
+xfs_inode_alloc(
 	xfs_trans_t	**tp,
 	xfs_inode_t	*pip,
 	mode_t		mode,
@@ -569,7 +568,7 @@ libxfs_inode_alloc(
 	int		error;
 
 	ialloc_context = (xfs_buf_t *)0;
-	error = libxfs_ialloc(*tp, pip, mode, nlink, rdev, cr, fsx,
+	error = xfs_ialloc(*tp, pip, mode, nlink, rdev, cr, fsx,
 			   &ialloc_context, &ip);
 	if (error) {
 		*ipp = NULL;
@@ -591,7 +590,7 @@ libxfs_inode_alloc(
 			exit(1);
 		}
 		xfs_trans_bjoin(*tp, ialloc_context);
-		error = libxfs_ialloc(*tp, pip, mode, nlink, rdev, cr,
+		error = xfs_ialloc(*tp, pip, mode, nlink, rdev, cr,
 				   fsx, &ialloc_context, &ip);
 		if (!ip)
 			error = -ENOSPC;
@@ -712,7 +711,7 @@ xfs_fsb_to_db(struct xfs_inode *ip, xfs_fsblock_t fsb)
 }
 
 int
-libxfs_zero_extent(
+xfs_zero_extent(
 	struct xfs_inode *ip,
 	xfs_fsblock_t	start_fsb,
 	xfs_off_t	count_fsb)

[PATCH 2/3] libxfs: remove libxfs API #defines for unexported xfs functions

[Eric Sandeen]

We define "libxfs_*" functions for anything used by userspace,
called from outside the libxfs/ directory.  However, many of the
current redefinitions are for functions only used within libxfs/*
so remove their API redefinitions.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
index 2b8ac5ab..a53efa68 100644
--- a/libxfs/libxfs_api_defs.h
+++ b/libxfs/libxfs_api_defs.h
@@ -18,37 +18,22 @@
 
 #define xfs_trans_alloc			libxfs_trans_alloc
 #define xfs_trans_alloc_rollable	libxfs_trans_alloc_rollable
-#define xfs_trans_alloc_empty		libxfs_trans_alloc_empty
-#define xfs_trans_add_item		libxfs_trans_add_item
 #define xfs_trans_bhold			libxfs_trans_bhold
 #define xfs_trans_binval		libxfs_trans_binval
 #define xfs_trans_bjoin			libxfs_trans_bjoin
 #define xfs_trans_brelse		libxfs_trans_brelse
 #define xfs_trans_commit		libxfs_trans_commit
 #define xfs_trans_cancel		libxfs_trans_cancel
-#define xfs_trans_del_item		libxfs_trans_del_item
 #define xfs_trans_get_buf		libxfs_trans_get_buf
-#define xfs_trans_getsb			libxfs_trans_getsb
 #define xfs_trans_ichgtime		libxfs_trans_ichgtime
 #define xfs_trans_ijoin			libxfs_trans_ijoin
-#define xfs_trans_init			libxfs_trans_init
-#define xfs_trans_inode_alloc_buf	libxfs_trans_inode_alloc_buf
-#define xfs_trans_dirty_buf		libxfs_trans_dirty_buf
 #define xfs_trans_log_buf		libxfs_trans_log_buf
-#define xfs_trans_ordered_buf		libxfs_trans_ordered_buf
 #define xfs_trans_log_inode		libxfs_trans_log_inode
 #define xfs_trans_roll_inode		libxfs_trans_roll_inode
-#define xfs_trans_mod_sb		libxfs_trans_mod_sb
 #define xfs_trans_read_buf		libxfs_trans_read_buf
-#define xfs_trans_read_buf_map		libxfs_trans_read_buf_map
-#define xfs_trans_roll			libxfs_trans_roll
-#define xfs_trans_get_buf_map		libxfs_trans_get_buf_map
-#define xfs_trans_resv_calc		libxfs_trans_resv_calc
 #define xfs_log_get_max_trans_res	libxfs_log_get_max_trans_res
-#define xfs_attr_get			libxfs_attr_get
 #define xfs_attr_set			libxfs_attr_set
 #define xfs_attr_remove			libxfs_attr_remove
-#define xfs_attr_leaf_newentsize	libxfs_attr_leaf_newentsize
 
 #define xfs_agfl_walk			libxfs_agfl_walk
 #define xfs_alloc_fix_freelist		libxfs_alloc_fix_freelist
@@ -57,15 +42,11 @@
 #define xfs_bmap_last_offset		libxfs_bmap_last_offset
 #define xfs_iext_lookup_extent		libxfs_iext_lookup_extent
 #define xfs_bmapi_write			libxfs_bmapi_write
-#define xfs_bmapi_read			libxfs_bmapi_read
 #define xfs_bunmapi			libxfs_bunmapi
 #define xfs_rtfree_extent		libxfs_rtfree_extent
-#define xfs_verify_rtbno		libxfs_verify_rtbno
 #define xfs_verify_ino			libxfs_verify_ino
-#define xfs_zero_extent			libxfs_zero_extent
 
 #define xfs_defer_finish		libxfs_defer_finish
-#define xfs_defer_cancel		libxfs_defer_cancel
 
 #define xfs_da_hashname			libxfs_da_hashname
 #define xfs_da_shrink_inode		libxfs_da_shrink_inode
@@ -85,14 +66,11 @@
 #define xfs_da_get_buf			libxfs_da_get_buf
 
 #define xfs_inode_from_disk		libxfs_inode_from_disk
-#define xfs_inode_to_disk		libxfs_inode_to_disk
 #define xfs_dinode_calc_crc		libxfs_dinode_calc_crc
 #define xfs_idata_realloc		libxfs_idata_realloc
-#define xfs_idestroy_fork		libxfs_idestroy_fork
 #define xfs_inode_validate_extsize	libxfs_inode_validate_extsize
 #define xfs_inode_validate_cowextsize	libxfs_inode_validate_cowextsize
 #define xfs_inode_alloc			libxfs_inode_alloc
-#define xfs_iflush_int			libxfs_iflush_int
 #define xfs_alloc_file_space		libxfs_alloc_file_space
 
 #define xfs_rmap_alloc			libxfs_rmap_alloc
@@ -107,7 +85,6 @@
 
 #define xfs_log_sb			libxfs_log_sb
 #define xfs_sb_from_disk		libxfs_sb_from_disk
-#define xfs_sb_quota_from_disk		libxfs_sb_quota_from_disk
 #define xfs_sb_to_disk			libxfs_sb_to_disk
 
 #define xfs_calc_dquots_per_chunk	libxfs_calc_dquots_per_chunk
@@ -151,6 +128,5 @@
 #define xfs_getsb			libxfs_getsb
 #define xfs_irele			libxfs_irele
 #define xfs_iget			libxfs_iget
-#define xfs_inode_verify_forks		libxfs_inode_verify_forks
 
 #endif /* __LIBXFS_API_DEFS_H__ */

[PATCH 3/3] xfsprogs: remove unused flags arg from getsb interfaces

[Eric Sandeen]

The flags value is always* passed as 0 so remove the argument.

*mkfs.xfs is the sole exception; it passes a flag to fail on read
error, but we can easily detect the error and exit from main()
manually instead.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

(This one might actually come in via a libxfs merge instead)

diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index d32acc9e..953da5d1 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -87,7 +87,7 @@ void	xfs_trans_cancel(struct xfs_trans *);
 /* cancel dfops associated with a transaction */
 void xfs_defer_cancel(struct xfs_trans *);
 
-struct xfs_buf *xfs_trans_getsb(struct xfs_trans *, struct xfs_mount *, int);
+struct xfs_buf *xfs_trans_getsb(struct xfs_trans *, struct xfs_mount *);
 
 void	xfs_trans_ijoin(struct xfs_trans *, struct xfs_inode *, uint);
 void	xfs_trans_log_inode (struct xfs_trans *, struct xfs_inode *,
diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index 8fa2c2c5..4dc4d5f3 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -177,7 +177,7 @@ extern void	libxfs_putbuf (xfs_buf_t *);
 
 extern void	libxfs_readbuf_verify(struct xfs_buf *bp,
 			const struct xfs_buf_ops *ops);
-extern xfs_buf_t *xfs_getsb(struct xfs_mount *, int);
+extern xfs_buf_t *xfs_getsb(struct xfs_mount *);
 extern void	libxfs_bcache_purge(void);
 extern void	libxfs_bcache_free(void);
 extern void	libxfs_bcache_flush(void);
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 8d6f958a..132ed0a9 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -476,10 +476,10 @@ libxfs_trace_putbuf(const char *func, const char *file, int line, xfs_buf_t *bp)
 
 
 xfs_buf_t *
-xfs_getsb(xfs_mount_t *mp, int flags)
+xfs_getsb(xfs_mount_t *mp)
 {
 	return libxfs_readbuf(mp->m_ddev_targp, XFS_SB_DADDR,
-				XFS_FSS_TO_BB(mp, 1), flags, &xfs_sb_buf_ops);
+				XFS_FSS_TO_BB(mp, 1), 0, &xfs_sb_buf_ops);
 }
 
 kmem_zone_t			*xfs_buf_zone;
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 5ef0c055..9e49def0 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -628,8 +628,7 @@ xfs_trans_get_buf_map(
 xfs_buf_t *
 xfs_trans_getsb(
 	xfs_trans_t		*tp,
-	xfs_mount_t		*mp,
-	int			flags)
+	xfs_mount_t		*mp)
 {
 	xfs_buf_t		*bp;
 	xfs_buf_log_item_t	*bip;
@@ -637,7 +636,7 @@ xfs_trans_getsb(
 	DEFINE_SINGLE_BUF_MAP(map, XFS_SB_DADDR, len);
 
 	if (tp == NULL)
-		return xfs_getsb(mp, flags);
+		return xfs_getsb(mp);
 
 	bp = xfs_trans_buf_item_match(tp, mp->m_dev, &map, 1);
 	if (bp != NULL) {
@@ -648,7 +647,7 @@ xfs_trans_getsb(
 		return bp;
 	}
 
-	bp = xfs_getsb(mp, flags);
+	bp = xfs_getsb(mp);
 #ifdef XACT_DEBUG
 	fprintf(stderr, "trans_get_sb buffer %p, transaction %p\n", bp, tp);
 #endif
diff --git a/libxfs/xfs_sb.c b/libxfs/xfs_sb.c
index f1e2ba57..a1857e10 100644
--- a/libxfs/xfs_sb.c
+++ b/libxfs/xfs_sb.c
@@ -915,7 +915,7 @@ xfs_log_sb(
 	struct xfs_trans	*tp)
 {
 	struct xfs_mount	*mp = tp->t_mountp;
-	struct xfs_buf		*bp = xfs_trans_getsb(tp, mp, 0);
+	struct xfs_buf		*bp = xfs_trans_getsb(tp, mp);
 
 	mp->m_sb.sb_icount = percpu_counter_sum(&mp->m_icount);
 	mp->m_sb.sb_ifree = percpu_counter_sum(&mp->m_ifree);
@@ -1045,7 +1045,7 @@ xfs_sync_sb_buf(
 	if (error)
 		return error;
 
-	bp = xfs_trans_getsb(tp, mp, 0);
+	bp = xfs_trans_getsb(tp, mp);
 	xfs_log_sb(tp);
 	xfs_trans_bhold(tp, bp);
 	xfs_trans_set_sync(tp);
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 09106648..647e2bc2 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -4123,7 +4123,11 @@ main(
 	/*
 	 * Mark the filesystem ok.
 	 */
-	buf = libxfs_getsb(mp, LIBXFS_EXIT_ON_FAILURE);
+	buf = libxfs_getsb(mp);
+	if (!buf) {
+		fprintf(stderr, _("couldn't get superblock\n"));
+		exit(1);
+	}
 	(XFS_BUF_TO_SBP(buf))->sb_inprogress = 0;
 	libxfs_writebuf(buf, LIBXFS_EXIT_ON_FAILURE);
 
diff --git a/repair/phase5.c b/repair/phase5.c
index 0f6a8395..8ad32365 100644
--- a/repair/phase5.c
+++ b/repair/phase5.c
@@ -2177,7 +2177,7 @@ sync_sb(xfs_mount_t *mp)
 {
 	xfs_buf_t	*bp;
 
-	bp = libxfs_getsb(mp, 0);
+	bp = libxfs_getsb(mp);
 	if (!bp)
 		do_error(_("couldn't get superblock\n"));
 
diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
index 9657503f..4000b3e6 100644
--- a/repair/xfs_repair.c
+++ b/repair/xfs_repair.c
@@ -1044,7 +1044,7 @@ _("Warning:  project quota information would be cleared.\n"
 	/*
 	 * Clear the quota flags if they're on.
 	 */
-	sbp = libxfs_getsb(mp, 0);
+	sbp = libxfs_getsb(mp);
 	if (!sbp)
 		do_error(_("couldn't get superblock\n"));
 

[PATCH 4/3] libxfs: Remove XACT_DEBUG #ifdefs

[Eric Sandeen]

Remove XACT_DEBUG #ifdefs to reduce more cosmetic differences
between userspace & kernelspace libxfs.  Add in some corresponding
(stubbed-out) tracepoint calls.

If these are felt to be particularly useful, the tracepoint calls
could be fleshed out to provide similar information.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 include/xfs_trace.h | 13 +++++++
 libxfs/trans.c      | 85 ++++++++++++---------------------------------
 2 files changed, 35 insertions(+), 63 deletions(-)

diff --git a/include/xfs_trace.h b/include/xfs_trace.h
index 793ac56e..43720040 100644
--- a/include/xfs_trace.h
+++ b/include/xfs_trace.h
@@ -161,6 +161,19 @@
 #define trace_xfs_perag_get_tag(a,b,c,d) ((c) = (c))
 #define trace_xfs_perag_put(a,b,c,d)	((c) = (c))
 
+#define trace_xfs_trans_alloc(a,b)		((void) 0)
+#define trace_xfs_trans_cancel(a,b)		((void) 0)
+#define trace_xfs_trans_brelse(a)		((void) 0)
+#define trace_xfs_trans_binval(a)		((void) 0)
+#define trace_xfs_trans_bjoin(a)		((void) 0)
+#define trace_xfs_trans_bhold(a)		((void) 0)
+#define trace_xfs_trans_get_buf(a)		((void) 0)
+#define trace_xfs_trans_getsb_recur(a)		((void) 0)
+#define trace_xfs_trans_getsb(a)		((void) 0)
+#define trace_xfs_trans_read_buf_recur(a)	((void) 0)
+#define trace_xfs_trans_read_buf(a)		((void) 0)
+#define trace_xfs_trans_commit(a,b)		((void) 0)
+
 #define trace_xfs_defer_cancel(a,b)		((void) 0)
 #define trace_xfs_defer_pending_commit(a,b)	((void) 0)
 #define trace_xfs_defer_pending_abort(a,b)	((void) 0)
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 9e49def0..6967a1de 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -18,6 +18,7 @@
 #include "xfs_trans.h"
 #include "xfs_sb.h"
 #include "xfs_defer.h"
+#include "xfs_trace.h"
 
 static void xfs_trans_free_items(struct xfs_trans *tp);
 STATIC struct xfs_trans *xfs_trans_dup(struct xfs_trans *tp);
@@ -269,9 +270,9 @@ xfs_trans_alloc(
 		xfs_trans_cancel(tp);
 		return error;
 	}
-#ifdef XACT_DEBUG
-	fprintf(stderr, "allocated new transaction %p\n", tp);
-#endif
+
+	trace_xfs_trans_alloc(tp, _RET_IP_);
+
 	*tpp = tp;
 	return 0;
 }
@@ -317,23 +318,16 @@ void
 xfs_trans_cancel(
 	struct xfs_trans	*tp)
 {
-#ifdef XACT_DEBUG
-	struct xfs_trans	*otp = tp;
-#endif
+	trace_xfs_trans_cancel(tp, _RET_IP_);
+
 	if (tp == NULL)
-		goto out;
+		return;
 
 	if (tp->t_flags & XFS_TRANS_PERM_LOG_RES)
 		xfs_defer_cancel(tp);
 
 	xfs_trans_free_items(tp);
 	xfs_trans_free(tp);
-
-out:
-#ifdef XACT_DEBUG
-	fprintf(stderr, "## cancelled transaction %p\n", otp);
-#endif
-	return;
 }
 
 void
@@ -353,10 +347,6 @@ xfs_trans_ijoin(
 	iip->ili_lock_flags = lock_flags;
 
 	xfs_trans_add_item(tp, (xfs_log_item_t *)(iip));
-
-#ifdef XACT_DEBUG
-	fprintf(stderr, "ijoin'd inode %llu, transaction %p\n", ip->i_ino, tp);
-#endif
 }
 
 void
@@ -388,9 +378,6 @@ xfs_trans_log_inode(
 	uint			flags)
 {
 	ASSERT(ip->i_itemp != NULL);
-#ifdef XACT_DEBUG
-	fprintf(stderr, "dirtied inode %llu, transaction %p\n", ip->i_ino, tp);
-#endif
 
 	tp->t_flags |= XFS_TRANS_DIRTY;
 	set_bit(XFS_LI_DIRTY, &ip->i_itemp->ili_item.li_flags);
@@ -434,9 +421,6 @@ xfs_trans_dirty_buf(
 	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
 
-#ifdef XACT_DEBUG
-	fprintf(stderr, "dirtied buffer %p, transaction %p\n", bp, tp);
-#endif
 	tp->t_flags |= XFS_TRANS_DIRTY;
 	set_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags);
 }
@@ -501,15 +485,14 @@ xfs_trans_brelse(
 	xfs_buf_t		*bp)
 {
 	xfs_buf_log_item_t	*bip;
-#ifdef XACT_DEBUG
-	fprintf(stderr, "released buffer %p, transaction %p\n", bp, tp);
-#endif
 
 	if (tp == NULL) {
 		ASSERT(bp->b_transp == NULL);
 		libxfs_putbuf(bp);
 		return;
 	}
+
+	trace_xfs_trans_brelse(bip);
 	ASSERT(bp->b_transp == tp);
 	bip = bp->b_log_item;
 	ASSERT(bip->bli_item.li_type == XFS_LI_BUF);
@@ -536,13 +519,12 @@ xfs_trans_binval(
 	xfs_buf_t		*bp)
 {
 	xfs_buf_log_item_t	*bip = bp->b_log_item;
-#ifdef XACT_DEBUG
-	fprintf(stderr, "binval'd buffer %p, transaction %p\n", bp, tp);
-#endif
 
 	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
 
+	trace_xfs_trans_binval(bip);
+
 	if (bip->bli_flags & XFS_BLI_STALE)
 		return;
 	XFS_BUF_UNDELAYWRITE(bp);
@@ -563,14 +545,12 @@ xfs_trans_bjoin(
 	xfs_buf_log_item_t	*bip;
 
 	ASSERT(bp->b_transp == NULL);
-#ifdef XACT_DEBUG
-	fprintf(stderr, "bjoin'd buffer %p, transaction %p\n", bp, tp);
-#endif
 
 	xfs_buf_item_init(bp, tp->t_mountp);
 	bip = bp->b_log_item;
 	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
 	bp->b_transp = tp;
+	trace_xfs_trans_bjoin(bp->b_log_item);
 }
 
 void
@@ -582,11 +562,9 @@ xfs_trans_bhold(
 
 	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
-#ifdef XACT_DEBUG
-	fprintf(stderr, "bhold'd buffer %p, transaction %p\n", bp, tp);
-#endif
 
 	bip->bli_flags |= XFS_BLI_HOLD;
+	trace_xfs_trans_bhold(bip);
 }
 
 xfs_buf_t *
@@ -615,13 +593,11 @@ xfs_trans_get_buf_map(
 	bp = libxfs_getbuf_map(btp, map, nmaps, 0);
 	if (bp == NULL)
 		return NULL;
-#ifdef XACT_DEBUG
-	fprintf(stderr, "trans_get_buf buffer %p, transaction %p\n", bp, tp);
-#endif
 
 	xfs_trans_bjoin(tp, bp);
 	bip = bp->b_log_item;
 	bip->bli_recur = 0;
+	trace_xfs_trans_get_buf(bp->b_log_item);
 	return bp;
 }
 
@@ -644,17 +620,16 @@ xfs_trans_getsb(
 		bip = bp->b_log_item;
 		ASSERT(bip != NULL);
 		bip->bli_recur++;
+		trace_xfs_trans_getsb_recur(bip);
 		return bp;
 	}
 
 	bp = xfs_getsb(mp);
-#ifdef XACT_DEBUG
-	fprintf(stderr, "trans_get_sb buffer %p, transaction %p\n", bp, tp);
-#endif
 
 	xfs_trans_bjoin(tp, bp);
 	bip = bp->b_log_item;
 	bip->bli_recur = 0;
+	trace_xfs_trans_getsb(bp->b_log_item);
 	return bp;
 }
 
@@ -691,6 +666,7 @@ xfs_trans_read_buf_map(
 		ASSERT(bp->b_log_item != NULL);
 		bip = bp->b_log_item;
 		bip->bli_recur++;
+		trace_xfs_trans_read_buf_recur(bip);
 		goto done;
 	}
 
@@ -701,14 +677,11 @@ xfs_trans_read_buf_map(
 	if (bp->b_error)
 		goto out_relse;
 
-#ifdef XACT_DEBUG
-	fprintf(stderr, "trans_read_buf buffer %p, transaction %p\n", bp, tp);
-#endif
-
 	xfs_trans_bjoin(tp, bp);
 	bip = bp->b_log_item;
 	bip->bli_recur = 0;
 done:
+	trace_xfs_trans_read_buf(bp->b_log_item);
 	*bpp = bp;
 	return 0;
 out_relse:
@@ -824,10 +797,6 @@ inode_item_done(
 	}
 
 	libxfs_writebuf(bp, 0);
-#ifdef XACT_DEBUG
-	fprintf(stderr, "flushing dirty inode %llu, buffer %p\n",
-			ip->i_ino, bp);
-#endif
 free:
 	xfs_inode_item_put(iip);
 }
@@ -845,13 +814,8 @@ buf_item_done(
 	bp->b_transp = NULL;			/* remove xact ptr */
 
 	hold = (bip->bli_flags & XFS_BLI_HOLD);
-	if (bip->bli_flags & XFS_BLI_DIRTY) {
-#ifdef XACT_DEBUG
-		fprintf(stderr, "flushing/staling buffer %p (hold=%d)\n",
-			bp, hold);
-#endif
+	if (bip->bli_flags & XFS_BLI_DIRTY)
 		libxfs_writebuf_int(bp, 0);
-	}
 
 	bip->bli_flags &= ~XFS_BLI_HOLD;
 	xfs_buf_item_put(bip);
@@ -937,6 +901,8 @@ __xfs_trans_commit(
 	struct xfs_sb		*sbp;
 	int			error = 0;
 
+	trace_xfs_trans_commit(tp, _RET_IP_);
+
 	if (tp == NULL)
 		return 0;
 
@@ -952,12 +918,8 @@ __xfs_trans_commit(
 			goto out_unreserve;
 	}
 
-	if (!(tp->t_flags & XFS_TRANS_DIRTY)) {
-#ifdef XACT_DEBUG
-		fprintf(stderr, "committed clean transaction %p\n", tp);
-#endif
+	if (!(tp->t_flags & XFS_TRANS_DIRTY))
 		goto out_unreserve;
-	}
 
 	if (tp->t_flags & XFS_TRANS_SB_DIRTY) {
 		sbp = &(tp->t_mountp->m_sb);
@@ -972,9 +934,6 @@ __xfs_trans_commit(
 		xfs_log_sb(tp);
 	}
 
-#ifdef XACT_DEBUG
-	fprintf(stderr, "committing dirty transaction %p\n", tp);
-#endif
 	trans_committed(tp);
 
 	/* That's it for the transaction structure.  Free it. */
-- 
2.17.0

[PATCH 5/3] libxfs: rename bli_format to avoid confusion with bli_formats

[Eric Sandeen]

Rename the bli_format structure to __bli_format to avoid
accidently confusing them with the bli_formats pointer.

(nb: userspace currently has no bli_formats pointer)

Source kernel commit: b94381737e9c4d014a4003e8ece9ba88670a2dd4

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 include/xfs_trans.h | 2 +-
 libxfs/logitem.c    | 6 +++---
 libxfs/trans.c      | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index 953da5d1..fe03ba64 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -39,7 +39,7 @@ typedef struct xfs_buf_log_item {
 	struct xfs_buf		*bli_buf;	/* real buffer pointer */
 	unsigned int		bli_flags;	/* misc flags */
 	unsigned int		bli_recur;	/* recursion count */
-	xfs_buf_log_format_t	bli_format;	/* in-log header */
+	xfs_buf_log_format_t	__bli_format;	/* in-log header */
 } xfs_buf_log_item_t;
 
 #define XFS_BLI_DIRTY			(1<<0)
diff --git a/libxfs/logitem.c b/libxfs/logitem.c
index 4da9bc1b..e862ab4f 100644
--- a/libxfs/logitem.c
+++ b/libxfs/logitem.c
@@ -107,9 +107,9 @@ xfs_buf_item_init(
 	bip->bli_item.li_mountp = mp;
 	INIT_LIST_HEAD(&bip->bli_item.li_trans);
 	bip->bli_buf = bp;
-	bip->bli_format.blf_type = XFS_LI_BUF;
-	bip->bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
-	bip->bli_format.blf_len = (unsigned short)BTOBB(bp->b_bcount);
+	bip->__bli_format.blf_type = XFS_LI_BUF;
+	bip->__bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
+	bip->__bli_format.blf_len = (unsigned short)BTOBB(bp->b_bcount);
 	bp->b_log_item = bip;
 }
 
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 6967a1de..f3c28fa7 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -531,8 +531,8 @@ xfs_trans_binval(
 	xfs_buf_stale(bp);
 	bip->bli_flags |= XFS_BLI_STALE;
 	bip->bli_flags &= ~XFS_BLI_DIRTY;
-	bip->bli_format.blf_flags &= ~XFS_BLF_INODE_BUF;
-	bip->bli_format.blf_flags |= XFS_BLF_CANCEL;
+	bip->__bli_format.blf_flags &= ~XFS_BLF_INODE_BUF;
+	bip->__bli_format.blf_flags |= XFS_BLF_CANCEL;
 	set_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags);
 	tp->t_flags |= XFS_TRANS_DIRTY;
 }
-- 
2.17.0

[PATCH 6/3] libxfs: factor common xfs_trans_bjoin code

[Eric Sandeen]

Most of xfs_trans_bjoin is duplicated in xfs_trans_get_buf_map,
xfs_trans_getsb and xfs_trans_read_buf_map.  Add a new
_xfs_trans_bjoin which can be called by all three functions.

Source kernel commit: d7e84f413726876c0ec66bbf90770f69841f7663

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Alex Elder <aelder@sgi.com>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/trans.c | 53 +++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 39 insertions(+), 14 deletions(-)

diff --git a/libxfs/trans.c b/libxfs/trans.c
index f3c28fa7..f78222fd 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -537,19 +537,50 @@ xfs_trans_binval(
 	tp->t_flags |= XFS_TRANS_DIRTY;
 }
 
-void
-xfs_trans_bjoin(
-	xfs_trans_t		*tp,
-	xfs_buf_t		*bp)
+/*
+ * Add the locked buffer to the transaction.
+ *
+ * The buffer must be locked, and it cannot be associated with any
+ * transaction.
+ *
+ * If the buffer does not yet have a buf log item associated with it,
+ * then allocate one for it.  Then add the buf item to the transaction.
+ */
+STATIC void
+_xfs_trans_bjoin(
+	struct xfs_trans	*tp,
+	struct xfs_buf		*bp,
+	int			reset_recur)
 {
-	xfs_buf_log_item_t	*bip;
+	struct xfs_buf_log_item	*bip;
 
 	ASSERT(bp->b_transp == NULL);
 
+        /*
+	 * The xfs_buf_log_item pointer is stored in b_log_item.  If
+	 * it doesn't have one yet, then allocate one and initialize it.
+	 * The checks to see if one is there are in xfs_buf_item_init().
+	 */
 	xfs_buf_item_init(bp, tp->t_mountp);
 	bip = bp->b_log_item;
+	if (reset_recur)
+		bip->bli_recur = 0;
+
+	/*
+	 * Attach the item to the transaction so we can find it in
+	 * xfs_trans_get_buf() and friends.
+	 */
 	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
 	bp->b_transp = tp;
+
+}
+
+void
+xfs_trans_bjoin(
+	struct xfs_trans	*tp,
+	struct xfs_buf		*bp)
+{
+	_xfs_trans_bjoin(tp, bp, 0);
 	trace_xfs_trans_bjoin(bp->b_log_item);
 }
 
@@ -594,9 +625,7 @@ xfs_trans_get_buf_map(
 	if (bp == NULL)
 		return NULL;
 
-	xfs_trans_bjoin(tp, bp);
-	bip = bp->b_log_item;
-	bip->bli_recur = 0;
+	_xfs_trans_bjoin(tp, bp, 1);
 	trace_xfs_trans_get_buf(bp->b_log_item);
 	return bp;
 }
@@ -626,9 +655,7 @@ xfs_trans_getsb(
 
 	bp = xfs_getsb(mp);
 
-	xfs_trans_bjoin(tp, bp);
-	bip = bp->b_log_item;
-	bip->bli_recur = 0;
+	_xfs_trans_bjoin(tp, bp, 1);
 	trace_xfs_trans_getsb(bp->b_log_item);
 	return bp;
 }
@@ -677,9 +704,7 @@ xfs_trans_read_buf_map(
 	if (bp->b_error)
 		goto out_relse;
 
-	xfs_trans_bjoin(tp, bp);
-	bip = bp->b_log_item;
-	bip->bli_recur = 0;
+	_xfs_trans_bjoin(tp, bp, 1);
 done:
 	trace_xfs_trans_read_buf(bp->b_log_item);
 	*bpp = bp;
-- 
2.17.0

[PATCH 7/3] libxfs: fix argument to xfs_trans_add_item

[Eric Sandeen]

The hack of casting an inode_log_item or buf_log_item to a
xfs_log_item_t is pretty gross; yes it's the first member in the
structure, but yuk.  Pass in the correct structure member.

This was fixed in the kernel with commit e98c414f9
("xfs: simplify log item descriptor tracking")

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/trans.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libxfs/trans.c b/libxfs/trans.c
index f78222fd..6ef4841f 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -346,7 +346,7 @@ xfs_trans_ijoin(
 	ASSERT(iip->ili_lock_flags == 0);
 	iip->ili_lock_flags = lock_flags;
 
-	xfs_trans_add_item(tp, (xfs_log_item_t *)(iip));
+	xfs_trans_add_item(tp, &iip->ili_item);
 }
 
 void
@@ -570,7 +570,7 @@ _xfs_trans_bjoin(
 	 * Attach the item to the transaction so we can find it in
 	 * xfs_trans_get_buf() and friends.
 	 */
-	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
+	xfs_trans_add_item(tp, &bip->bli_item);
 	bp->b_transp = tp;
 
 }
-- 
2.17.0

[PATCH 8/3] xfs: factor log item initialisation

[Eric Sandeen]

Each log item type does manual initialisation of the log item.
Delayed logging introduces new fields that need initialisation, so
factor all the open coded initialisation into a common function
first.

Source kernel commit: 43f5efc5b59db1b66e39fe9fdfc4ba6a27152afa

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/libxfs_priv.h |  1 +
 libxfs/logitem.c     |  8 ++------
 libxfs/util.c        | 12 ++++++++++++
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index 157a99d6..7551ed65 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -564,6 +564,7 @@ int xfs_zero_extent(struct xfs_inode *ip, xfs_fsblock_t start_fsb,
 
 
 bool xfs_log_check_lsn(struct xfs_mount *, xfs_lsn_t);
+void xfs_log_item_init(struct xfs_mount *, struct xfs_log_item *, int);
 #define xfs_log_in_recovery(mp)	(false)
 
 /* xfs_icache.c */
diff --git a/libxfs/logitem.c b/libxfs/logitem.c
index e862ab4f..14c62f67 100644
--- a/libxfs/logitem.c
+++ b/libxfs/logitem.c
@@ -103,9 +103,7 @@ xfs_buf_item_init(
 	fprintf(stderr, "adding buf item %p for not-logged buffer %p\n",
 		bip, bp);
 #endif
-	bip->bli_item.li_type = XFS_LI_BUF;
-	bip->bli_item.li_mountp = mp;
-	INIT_LIST_HEAD(&bip->bli_item.li_trans);
+	xfs_log_item_init(mp, &bip->bli_item, XFS_LI_BUF);
 	bip->bli_buf = bp;
 	bip->__bli_format.blf_type = XFS_LI_BUF;
 	bip->__bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
@@ -149,8 +147,6 @@ xfs_inode_item_init(
 		ip->i_ino, iip);
 #endif
 
-	iip->ili_item.li_type = XFS_LI_INODE;
-	iip->ili_item.li_mountp = mp;
-	INIT_LIST_HEAD(&iip->ili_item.li_trans);
+	xfs_log_item_init(mp, &iip->ili_item, XFS_LI_INODE);
 	iip->ili_inode = ip;
 }
diff --git a/libxfs/util.c b/libxfs/util.c
index 4901123a..aff91080 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -691,6 +691,18 @@ xfs_log_check_lsn(
 	return true;
 }
 
+void
+xfs_log_item_init(
+	struct xfs_mount	*mp,
+	struct xfs_log_item	*item,
+	int			type)
+{
+	item->li_mountp = mp; 
+	item->li_type = type;
+        
+	INIT_LIST_HEAD(&item->li_trans);
+}   
+
 static struct xfs_buftarg *
 xfs_find_bdev_for_inode(
 	struct xfs_inode	*ip)
-- 
2.17.0

[PATCH 9/3] libxfs: create current_time helper and sync xfs_trans_ichgtime

[Eric Sandeen]

Make xfs_trans_ichgtime() almost match kernelspace by creating a
new current_time() helper to match the kernel utility.

This reduces still more cosmetic change.  We may want to sync the
creation flag over to the kernel even though it's not used today.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/include/xfs_inode.h b/include/xfs_inode.h
index 3e7e80ea..289d1774 100644
--- a/include/xfs_inode.h
+++ b/include/xfs_inode.h
@@ -16,6 +16,16 @@ struct xfs_mount;
 struct xfs_inode_log_item;
 struct xfs_dir_ops;
 
+/*
+ * These are not actually used, they are only for userspace build
+ * compatibility in code that looks at i_state
+ */
+#define I_DIRTY_TIME		0
+#define I_DIRTY_TIME_EXPIRED	0
+
+#define IS_I_VERSION(inode)			(0)
+#define inode_maybe_inc_iversion(inode,flags)	(0)
+
 /*
  * Inode interface. This fakes up a "VFS inode" to make the xfs_inode appear
  * similar to the kernel which now is used tohold certain parts of the on-disk
@@ -25,6 +35,7 @@ struct inode {
 	mode_t		i_mode;
 	uint32_t	i_nlink;
 	xfs_dev_t	i_rdev;		/* This actually holds xfs_dev_t */
+	unsigned long	i_state;	/* Not actually used in userspace */
 	uint32_t	i_generation;
 	uint64_t	i_version;
 	struct timespec	i_atime;
@@ -149,6 +160,9 @@ extern void	xfs_trans_ichgtime(struct xfs_trans *,
 				struct xfs_inode *, int);
 extern int	xfs_iflush_int (struct xfs_inode *, struct xfs_buf *);
 
+#define timespec64 timespec
+extern struct timespec64 current_time(struct inode *inode);
+
 /* Inode Cache Interfaces */
 extern bool	xfs_inode_verify_forks(struct xfs_inode *ip);
 extern int	xfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
diff --git a/libxfs/util.c b/libxfs/util.c
index aff91080..1734ae9a 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -136,11 +136,21 @@ xfs_log_calc_unit_res(
 	return unit_bytes;
 }
 
+struct timespec64
+current_time(struct inode *inode)
+{
+	struct timespec64	tv;
+	struct timeval		stv;
+
+	gettimeofday(&stv, (struct timezone *)0);
+	tv.tv_sec = stv.tv_sec;
+	tv.tv_nsec = stv.tv_usec * 1000;
+
+	return tv;
+}
+
 /*
  * Change the requested timestamp in the given inode.
- *
- * This was once shared with the kernel, but has diverged to the point
- * where it's no longer worth the hassle of maintaining common code.
  */
 void
 xfs_trans_ichgtime(
@@ -148,12 +158,14 @@ xfs_trans_ichgtime(
 	struct xfs_inode	*ip,
 	int			flags)
 {
-	struct timespec tv;
-	struct timeval	stv;
+	struct inode		*inode = VFS_I(ip);
+	struct timespec64	tv;
+
+	ASSERT(tp);
+	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+
+	tv = current_time(inode);
 
-	gettimeofday(&stv, (struct timezone *)0);
-	tv.tv_sec = stv.tv_sec;
-	tv.tv_nsec = stv.tv_usec * 1000;
 	if (flags & XFS_ICHGTIME_MOD)
 		VFS_I(ip)->i_mtime = tv;
 	if (flags & XFS_ICHGTIME_CHG)

[PATCH 10/3] libxfs: share kernel's xfs_trans_inode.c

[Eric Sandeen]

Now that the majority of cosmetic changes and compat shims
are in place, we can directly share kernelspace's
xfs_trans_inode.c with just a couple more small tweaks.
In addition to the file move,

* ili_fsync_fields is added to xfs_inode_log_item (but not used)
* test_and_set_bit() helper is created

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

To finish the sync, we probably need to move the file under
libxfs/ in kernelspace, and add XFS_ICHGTIME_CREATE handling
unless that's deemed undesirable, in which case we can probably
just carry the delta in userspace...

diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index fe03ba64..cdaa69d8 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -30,8 +30,9 @@ typedef struct xfs_inode_log_item {
 	xfs_log_item_t		ili_item;		/* common portion */
 	struct xfs_inode	*ili_inode;		/* inode pointer */
 	unsigned short		ili_lock_flags;		/* lock flags */
-	unsigned int		ili_fields;		/* fields to be logged */
 	unsigned int		ili_last_fields;	/* fields when flushed*/
+	unsigned int		ili_fields;		/* fields to be logged */
+	unsigned int		ili_fsync_fields;	/* ignored by userspace */
 } xfs_inode_log_item_t;
 
 typedef struct xfs_buf_log_item {
diff --git a/libxfs/Makefile b/libxfs/Makefile
index 160498d7..8c681e0b 100644
--- a/libxfs/Makefile
+++ b/libxfs/Makefile
@@ -93,6 +93,7 @@ CFILES = cache.c \
 	xfs_rtbitmap.c \
 	xfs_sb.c \
 	xfs_symlink_remote.c \
+	xfs_trans_inode.c \
 	xfs_trans_resv.c \
 	xfs_types.c
 
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index 7551ed65..fc785664 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -608,6 +608,15 @@ static inline int test_bit(int nr, const volatile unsigned long *addr)
 	return *p & mask;
 }
 
+/* Sets and returns original value of the bit */
+static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
+{
+	if (test_bit(nr, addr))
+		return 1;
+	set_bit(nr, addr);
+	return 0;
+}
+
 /* Keep static checkers quiet about nonstatic functions by exporting */
 int xfs_inode_hasattr(struct xfs_inode *ip);
 int xfs_attr_get_ilocked(struct xfs_inode *ip, struct xfs_da_args *args);
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 6ef4841f..29dd10f7 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -330,25 +330,6 @@ xfs_trans_cancel(
 	xfs_trans_free(tp);
 }
 
-void
-xfs_trans_ijoin(
-	xfs_trans_t		*tp,
-	xfs_inode_t		*ip,
-	uint			lock_flags)
-{
-	xfs_inode_log_item_t	*iip;
-
-	if (ip->i_itemp == NULL)
-		xfs_inode_item_init(ip, ip->i_mount);
-	iip = ip->i_itemp;
-	ASSERT(iip->ili_inode != NULL);
-
-	ASSERT(iip->ili_lock_flags == 0);
-	iip->ili_lock_flags = lock_flags;
-
-	xfs_trans_add_item(tp, &iip->ili_item);
-}
-
 void
 xfs_trans_inode_alloc_buf(
 	xfs_trans_t		*tp,
@@ -362,52 +343,6 @@ xfs_trans_inode_alloc_buf(
 	xfs_trans_buf_set_type(tp, bp, XFS_BLFT_DINO_BUF);
 }
 
-/*
- * This is called to mark the fields indicated in fieldmask as needing
- * to be logged when the transaction is committed.  The inode must
- * already be associated with the given transaction.
- *
- * The values for fieldmask are defined in xfs_log_format.h.  We always
- * log all of the core inode if any of it has changed, and we always log
- * all of the inline data/extents/b-tree root if any of them has changed.
- */
-void
-xfs_trans_log_inode(
-	xfs_trans_t		*tp,
-	xfs_inode_t		*ip,
-	uint			flags)
-{
-	ASSERT(ip->i_itemp != NULL);
-
-	tp->t_flags |= XFS_TRANS_DIRTY;
-	set_bit(XFS_LI_DIRTY, &ip->i_itemp->ili_item.li_flags);
-
-	/*
-	 * Always OR in the bits from the ili_last_fields field.
-	 * This is to coordinate with the xfs_iflush() and xfs_iflush_done()
-	 * routines in the eventual clearing of the ilf_fields bits.
-	 * See the big comment in xfs_iflush() for an explanation of
-	 * this coordination mechanism.
-	 */
-	flags |= ip->i_itemp->ili_last_fields;
-	ip->i_itemp->ili_fields |= flags;
-}
-
-int
-xfs_trans_roll_inode(
-	struct xfs_trans	**tpp,
-	struct xfs_inode	*ip)
-{
-	int			error;
-
-	xfs_trans_log_inode(*tpp, ip, XFS_ILOG_CORE);
-	error = xfs_trans_roll(tpp);
-	if (!error)
-		xfs_trans_ijoin(*tpp, ip, 0);
-	return error;
-}
-
-
 /*
  * Mark a buffer dirty in the transaction.
  */
diff --git a/libxfs/util.c b/libxfs/util.c
index 1734ae9a..5a89bd03 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -149,33 +149,6 @@ current_time(struct inode *inode)
 	return tv;
 }
 
-/*
- * Change the requested timestamp in the given inode.
- */
-void
-xfs_trans_ichgtime(
-	struct xfs_trans	*tp,
-	struct xfs_inode	*ip,
-	int			flags)
-{
-	struct inode		*inode = VFS_I(ip);
-	struct timespec64	tv;
-
-	ASSERT(tp);
-	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
-
-	tv = current_time(inode);
-
-	if (flags & XFS_ICHGTIME_MOD)
-		VFS_I(ip)->i_mtime = tv;
-	if (flags & XFS_ICHGTIME_CHG)
-		VFS_I(ip)->i_ctime = tv;
-	if (flags & XFS_ICHGTIME_CREATE) {
-		ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec;
-		ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec;
-	}
-}
-
 STATIC uint16_t
 xfs_flags2diflags(
 	struct xfs_inode	*ip,
diff --git a/libxfs/xfs_trans_inode.c b/libxfs/xfs_trans_inode.c
new file mode 100644
index 00000000..87e6335b
--- /dev/null
+++ b/libxfs/xfs_trans_inode.c
@@ -0,0 +1,155 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2000,2005 Silicon Graphics, Inc.
+ * All Rights Reserved.
+ */
+#include "libxfs_priv.h"
+#include "xfs_fs.h"
+#include "xfs_shared.h"
+#include "xfs_format.h"
+#include "xfs_log_format.h"
+#include "xfs_trans_resv.h"
+#include "xfs_mount.h"
+#include "xfs_inode.h"
+#include "xfs_trans.h"
+#include "xfs_trace.h"
+
+/*
+ * Add a locked inode to the transaction.
+ *
+ * The inode must be locked, and it cannot be associated with any transaction.
+ * If lock_flags is non-zero the inode will be unlocked on transaction commit.
+ */
+void
+xfs_trans_ijoin(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*ip,
+	uint			lock_flags)
+{
+	xfs_inode_log_item_t	*iip;
+
+	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+	if (ip->i_itemp == NULL)
+		xfs_inode_item_init(ip, ip->i_mount);
+	iip = ip->i_itemp;
+
+	ASSERT(iip->ili_lock_flags == 0);
+	iip->ili_lock_flags = lock_flags;
+
+	/*
+	 * Get a log_item_desc to point at the new item.
+	 */
+	xfs_trans_add_item(tp, &iip->ili_item);
+}
+
+/*
+ * Transactional inode timestamp update. Requires the inode to be locked and
+ * joined to the transaction supplied. Relies on the transaction subsystem to
+ * track dirty state and update/writeback the inode accordingly.
+ */
+void
+xfs_trans_ichgtime(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*ip,
+	int			flags)
+{
+	struct inode		*inode = VFS_I(ip);
+	struct timespec64 tv;
+
+	ASSERT(tp);
+	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+
+	tv = current_time(inode);
+
+	if (flags & XFS_ICHGTIME_MOD)
+		inode->i_mtime = tv;
+	if (flags & XFS_ICHGTIME_CHG)
+		inode->i_ctime = tv;
+	if (flags & XFS_ICHGTIME_CREATE) {
+		ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec;
+		ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec;
+	}
+}
+
+/*
+ * This is called to mark the fields indicated in fieldmask as needing
+ * to be logged when the transaction is committed.  The inode must
+ * already be associated with the given transaction.
+ *
+ * The values for fieldmask are defined in xfs_inode_item.h.  We always
+ * log all of the core inode if any of it has changed, and we always log
+ * all of the inline data/extents/b-tree root if any of them has changed.
+ */
+void
+xfs_trans_log_inode(
+	xfs_trans_t	*tp,
+	xfs_inode_t	*ip,
+	uint		flags)
+{
+	struct inode	*inode = VFS_I(ip);
+
+	ASSERT(ip->i_itemp != NULL);
+	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+
+	/*
+	 * Don't bother with i_lock for the I_DIRTY_TIME check here, as races
+	 * don't matter - we either will need an extra transaction in 24 hours
+	 * to log the timestamps, or will clear already cleared fields in the
+	 * worst case.
+	 */
+	if (inode->i_state & (I_DIRTY_TIME | I_DIRTY_TIME_EXPIRED)) {
+		spin_lock(&inode->i_lock);
+		inode->i_state &= ~(I_DIRTY_TIME | I_DIRTY_TIME_EXPIRED);
+		spin_unlock(&inode->i_lock);
+	}
+
+	/*
+	 * Record the specific change for fdatasync optimisation. This
+	 * allows fdatasync to skip log forces for inodes that are only
+	 * timestamp dirty. We do this before the change count so that
+	 * the core being logged in this case does not impact on fdatasync
+	 * behaviour.
+	 */
+	ip->i_itemp->ili_fsync_fields |= flags;
+
+	/*
+	 * First time we log the inode in a transaction, bump the inode change
+	 * counter if it is configured for this to occur. While we have the
+	 * inode locked exclusively for metadata modification, we can usually
+	 * avoid setting XFS_ILOG_CORE if no one has queried the value since
+	 * the last time it was incremented. If we have XFS_ILOG_CORE already
+	 * set however, then go ahead and bump the i_version counter
+	 * unconditionally.
+	 */
+	if (!test_and_set_bit(XFS_LI_DIRTY, &ip->i_itemp->ili_item.li_flags) &&
+	    IS_I_VERSION(VFS_I(ip))) {
+		if (inode_maybe_inc_iversion(VFS_I(ip), flags & XFS_ILOG_CORE))
+			flags |= XFS_ILOG_CORE;
+	}
+
+	tp->t_flags |= XFS_TRANS_DIRTY;
+
+	/*
+	 * Always OR in the bits from the ili_last_fields field.
+	 * This is to coordinate with the xfs_iflush() and xfs_iflush_done()
+	 * routines in the eventual clearing of the ili_fields bits.
+	 * See the big comment in xfs_iflush() for an explanation of
+	 * this coordination mechanism.
+	 */
+	flags |= ip->i_itemp->ili_last_fields;
+	ip->i_itemp->ili_fields |= flags;
+}
+
+int
+xfs_trans_roll_inode(
+	struct xfs_trans	**tpp,
+	struct xfs_inode	*ip)
+{
+	int			error;
+
+	xfs_trans_log_inode(*tpp, ip, XFS_ILOG_CORE);
+	error = xfs_trans_roll(tpp);
+	if (!error)
+		xfs_trans_ijoin(*tpp, ip, 0);
+	return error;
+}

Re: [PATCH 1/3] libxfs: rename shared kernel functions from libxfs_ to xfs_

[Allison Collins]

On 5/16/19 10:43 AM, Eric Sandeen wrote:
> The libxfs_* function namespace has gotten a bit confused and
> muddled; the general idea is that functions called from userspace
> utilities should use the libxfs_* namespace.  In many cases
> we use #defines to define xfs_* namespace to libxfs_*; in other
> cases we have explicitly defined libxfs_* functions which are clear
> counterparts or even clones of kernel libxfs/* functions.
> 
> For any function definitions within libxfs/* which match kernel
> names, give them standard xfs_* names to further reduce differnces
> between userspace and kernel libxfs/* code.
> 
> Then add #defines to libxfs_* for any functions which are needed
> by utilities, as is done with other core functionality.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/include/libxfs.h b/include/libxfs.h
> index 230bc3e8..ceebccdc 100644
> --- a/include/libxfs.h
> +++ b/include/libxfs.h
> @@ -151,7 +151,7 @@ extern int	libxfs_log_header(char *, uuid_t *, int, int, int, xfs_lsn_t,
>   
>   /* Shared utility routines */
>   
> -extern int	libxfs_alloc_file_space (struct xfs_inode *, xfs_off_t,
> +extern int	xfs_alloc_file_space (struct xfs_inode *, xfs_off_t,
>   				xfs_off_t, int, int);
>   
>   /* XXX: this is messy and needs fixing */
> diff --git a/include/xfs_inode.h b/include/xfs_inode.h
> index 88b58ac3..3e7e80ea 100644
> --- a/include/xfs_inode.h
> +++ b/include/xfs_inode.h
> @@ -139,21 +139,21 @@ typedef struct cred {
>   	gid_t	cr_gid;
>   } cred_t;
>   
> -extern int	libxfs_inode_alloc (struct xfs_trans **, struct xfs_inode *,
> +extern int	xfs_inode_alloc (struct xfs_trans **, struct xfs_inode *,
>   				mode_t, nlink_t, xfs_dev_t, struct cred *,
>   				struct fsxattr *, struct xfs_inode **);
> -extern void	libxfs_trans_inode_alloc_buf (struct xfs_trans *,
> +extern void	xfs_trans_inode_alloc_buf (struct xfs_trans *,
>   				struct xfs_buf *);
>   
> -extern void	libxfs_trans_ichgtime(struct xfs_trans *,
> +extern void	xfs_trans_ichgtime(struct xfs_trans *,
>   				struct xfs_inode *, int);
> -extern int	libxfs_iflush_int (struct xfs_inode *, struct xfs_buf *);
> +extern int	xfs_iflush_int (struct xfs_inode *, struct xfs_buf *);
>   
>   /* Inode Cache Interfaces */
> -extern bool	libxfs_inode_verify_forks(struct xfs_inode *ip);
> -extern int	libxfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
> +extern bool	xfs_inode_verify_forks(struct xfs_inode *ip);
> +extern int	xfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
>   				uint, struct xfs_inode **,
>   				struct xfs_ifork_ops *);
> -extern void	libxfs_irele(struct xfs_inode *ip);
> +extern void	xfs_irele(struct xfs_inode *ip);
>   
>   #endif /* __XFS_INODE_H__ */
> diff --git a/include/xfs_trans.h b/include/xfs_trans.h
> index 10b74538..d32acc9e 100644
> --- a/include/xfs_trans.h
> +++ b/include/xfs_trans.h
> @@ -75,46 +75,46 @@ typedef struct xfs_trans {
>   void	xfs_trans_init(struct xfs_mount *);
>   int	xfs_trans_roll(struct xfs_trans **);
>   
> -int	libxfs_trans_alloc(struct xfs_mount *mp, struct xfs_trans_res *resp,
> +int	xfs_trans_alloc(struct xfs_mount *mp, struct xfs_trans_res *resp,
>   			   uint blocks, uint rtextents, uint flags,
>   			   struct xfs_trans **tpp);
>   int	libxfs_trans_alloc_rollable(struct xfs_mount *mp, uint blocks,
>   				    struct xfs_trans **tpp);

Did you mean to rename libxfs_trans_alloc_rollable too?  I notice the 
function name is changed later down in the patch.

I think the rest of it looks pretty straight forward though.

Allison

> -int	libxfs_trans_alloc_empty(struct xfs_mount *mp, struct xfs_trans **tpp);
> -int	libxfs_trans_commit(struct xfs_trans *);
> -void	libxfs_trans_cancel(struct xfs_trans *);
> +int	xfs_trans_alloc_empty(struct xfs_mount *mp, struct xfs_trans **tpp);
> +int	xfs_trans_commit(struct xfs_trans *);
> +void	xfs_trans_cancel(struct xfs_trans *);
>   
>   /* cancel dfops associated with a transaction */
>   void xfs_defer_cancel(struct xfs_trans *);
>   
> -struct xfs_buf *libxfs_trans_getsb(struct xfs_trans *, struct xfs_mount *, int);
> +struct xfs_buf *xfs_trans_getsb(struct xfs_trans *, struct xfs_mount *, int);
>   
> -void	libxfs_trans_ijoin(struct xfs_trans *, struct xfs_inode *, uint);
> -void	libxfs_trans_log_inode (struct xfs_trans *, struct xfs_inode *,
> +void	xfs_trans_ijoin(struct xfs_trans *, struct xfs_inode *, uint);
> +void	xfs_trans_log_inode (struct xfs_trans *, struct xfs_inode *,
>   				uint);
> -int	libxfs_trans_roll_inode (struct xfs_trans **, struct xfs_inode *);
> -
> -void	libxfs_trans_brelse(struct xfs_trans *, struct xfs_buf *);
> -void	libxfs_trans_binval(struct xfs_trans *, struct xfs_buf *);
> -void	libxfs_trans_bjoin(struct xfs_trans *, struct xfs_buf *);
> -void	libxfs_trans_bhold(struct xfs_trans *, struct xfs_buf *);
> -void	libxfs_trans_dirty_buf(struct xfs_trans *, struct xfs_buf *);
> -void	libxfs_trans_log_buf(struct xfs_trans *, struct xfs_buf *,
> +int	xfs_trans_roll_inode (struct xfs_trans **, struct xfs_inode *);
> +
> +void	xfs_trans_brelse(struct xfs_trans *, struct xfs_buf *);
> +void	xfs_trans_binval(struct xfs_trans *, struct xfs_buf *);
> +void	xfs_trans_bjoin(struct xfs_trans *, struct xfs_buf *);
> +void	xfs_trans_bhold(struct xfs_trans *, struct xfs_buf *);
> +void	xfs_trans_dirty_buf(struct xfs_trans *, struct xfs_buf *);
> +void	xfs_trans_log_buf(struct xfs_trans *, struct xfs_buf *,
>   				uint, uint);
> -bool	libxfs_trans_ordered_buf(xfs_trans_t *, struct xfs_buf *);
> +bool	xfs_trans_ordered_buf(xfs_trans_t *, struct xfs_buf *);
>   
> -struct xfs_buf	*libxfs_trans_get_buf_map(struct xfs_trans *tp,
> +struct xfs_buf	*xfs_trans_get_buf_map(struct xfs_trans *tp,
>   					struct xfs_buftarg *btp,
>   					struct xfs_buf_map *map, int nmaps,
>   					uint flags);
>   
> -int	libxfs_trans_read_buf_map(struct xfs_mount *mp, struct xfs_trans *tp,
> +int	xfs_trans_read_buf_map(struct xfs_mount *mp, struct xfs_trans *tp,
>   				  struct xfs_buftarg *btp,
>   				  struct xfs_buf_map *map, int nmaps,
>   				  uint flags, struct xfs_buf **bpp,
>   				  const struct xfs_buf_ops *ops);
>   static inline struct xfs_buf *
> -libxfs_trans_get_buf(
> +xfs_trans_get_buf(
>   	struct xfs_trans	*tp,
>   	struct xfs_buftarg	*btp,
>   	xfs_daddr_t		blkno,
> @@ -122,11 +122,11 @@ libxfs_trans_get_buf(
>   	uint			flags)
>   {
>   	DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
> -	return libxfs_trans_get_buf_map(tp, btp, &map, 1, flags);
> +	return xfs_trans_get_buf_map(tp, btp, &map, 1, flags);
>   }
>   
>   static inline int
> -libxfs_trans_read_buf(
> +xfs_trans_read_buf(
>   	struct xfs_mount	*mp,
>   	struct xfs_trans	*tp,
>   	struct xfs_buftarg	*btp,
> @@ -137,7 +137,7 @@ libxfs_trans_read_buf(
>   	const struct xfs_buf_ops *ops)
>   {
>   	DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
> -	return libxfs_trans_read_buf_map(mp, tp, btp, &map, 1, flags, bpp, ops);
> +	return xfs_trans_read_buf_map(mp, tp, btp, &map, 1, flags, bpp, ops);
>   }
>   
>   #endif	/* __XFS_TRANS_H__ */
> diff --git a/libxfs/init.c b/libxfs/init.c
> index 2f6decc8..4b402a6e 100644
> --- a/libxfs/init.c
> +++ b/libxfs/init.c
> @@ -449,7 +449,7 @@ rtmount_init(
>   }
>   
>   static int
> -libxfs_initialize_perag(
> +xfs_initialize_perag(
>   	xfs_mount_t	*mp,
>   	xfs_agnumber_t	agcount,
>   	xfs_agnumber_t	*maxagi)
> @@ -790,7 +790,7 @@ libxfs_mount(
>   	}
>   
>   	/*
> -	 * libxfs_initialize_perag will allocate a perag structure for each ag.
> +	 * xfs_initialize_perag will allocate a perag structure for each ag.
>   	 * If agcount is corrupted and insanely high, this will OOM the box.
>   	 * If the agount seems (arbitrarily) high, try to read what would be
>   	 * the last AG, and if that fails for a relatively high agcount, just
> @@ -812,7 +812,7 @@ libxfs_mount(
>   		libxfs_putbuf(bp);
>   	}
>   
> -	error = libxfs_initialize_perag(mp, sbp->sb_agcount, &mp->m_maxagi);
> +	error = xfs_initialize_perag(mp, sbp->sb_agcount, &mp->m_maxagi);
>   	if (error) {
>   		fprintf(stderr, _("%s: perag init failed\n"),
>   			progname);
> @@ -826,9 +826,9 @@ void
>   libxfs_rtmount_destroy(xfs_mount_t *mp)
>   {
>   	if (mp->m_rsumip)
> -		libxfs_irele(mp->m_rsumip);
> +		xfs_irele(mp->m_rsumip);
>   	if (mp->m_rbmip)
> -		libxfs_irele(mp->m_rbmip);
> +		xfs_irele(mp->m_rbmip);
>   	mp->m_rsumip = mp->m_rbmip = NULL;
>   }
>   
> diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
> index 1150ec93..2b8ac5ab 100644
> --- a/libxfs/libxfs_api_defs.h
> +++ b/libxfs/libxfs_api_defs.h
> @@ -17,6 +17,7 @@
>   #define xfs_highbit64			libxfs_highbit64
>   
>   #define xfs_trans_alloc			libxfs_trans_alloc
> +#define xfs_trans_alloc_rollable	libxfs_trans_alloc_rollable
>   #define xfs_trans_alloc_empty		libxfs_trans_alloc_empty
>   #define xfs_trans_add_item		libxfs_trans_add_item
>   #define xfs_trans_bhold			libxfs_trans_bhold
> @@ -90,6 +91,9 @@
>   #define xfs_idestroy_fork		libxfs_idestroy_fork
>   #define xfs_inode_validate_extsize	libxfs_inode_validate_extsize
>   #define xfs_inode_validate_cowextsize	libxfs_inode_validate_cowextsize
> +#define xfs_inode_alloc			libxfs_inode_alloc
> +#define xfs_iflush_int			libxfs_iflush_int
> +#define xfs_alloc_file_space		libxfs_alloc_file_space
>   
>   #define xfs_rmap_alloc			libxfs_rmap_alloc
>   #define xfs_rmap_query_range		libxfs_rmap_query_range
> @@ -144,4 +148,9 @@
>   #define xfs_fs_geometry			libxfs_fs_geometry
>   #define xfs_init_local_fork		libxfs_init_local_fork
>   
> +#define xfs_getsb			libxfs_getsb
> +#define xfs_irele			libxfs_irele
> +#define xfs_iget			libxfs_iget
> +#define xfs_inode_verify_forks		libxfs_inode_verify_forks
> +
>   #endif /* __LIBXFS_API_DEFS_H__ */
> diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
> index 79ffd470..8fa2c2c5 100644
> --- a/libxfs/libxfs_io.h
> +++ b/libxfs/libxfs_io.h
> @@ -177,7 +177,7 @@ extern void	libxfs_putbuf (xfs_buf_t *);
>   
>   extern void	libxfs_readbuf_verify(struct xfs_buf *bp,
>   			const struct xfs_buf_ops *ops);
> -extern xfs_buf_t *libxfs_getsb(struct xfs_mount *, int);
> +extern xfs_buf_t *xfs_getsb(struct xfs_mount *, int);
>   extern void	libxfs_bcache_purge(void);
>   extern void	libxfs_bcache_free(void);
>   extern void	libxfs_bcache_flush(void);
> diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
> index d668a157..157a99d6 100644
> --- a/libxfs/libxfs_priv.h
> +++ b/libxfs/libxfs_priv.h
> @@ -559,7 +559,7 @@ typedef int (*xfs_rtalloc_query_range_fn)(
>   	struct xfs_rtalloc_rec	*rec,
>   	void			*priv);
>   
> -int libxfs_zero_extent(struct xfs_inode *ip, xfs_fsblock_t start_fsb,
> +int xfs_zero_extent(struct xfs_inode *ip, xfs_fsblock_t start_fsb,
>                           xfs_off_t count_fsb);
>   
>   
> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> index e3ff5842..8d6f958a 100644
> --- a/libxfs/rdwr.c
> +++ b/libxfs/rdwr.c
> @@ -476,7 +476,7 @@ libxfs_trace_putbuf(const char *func, const char *file, int line, xfs_buf_t *bp)
>   
>   
>   xfs_buf_t *
> -libxfs_getsb(xfs_mount_t *mp, int flags)
> +xfs_getsb(xfs_mount_t *mp, int flags)
>   {
>   	return libxfs_readbuf(mp->m_ddev_targp, XFS_SB_DADDR,
>   				XFS_FSS_TO_BB(mp, 1), flags, &xfs_sb_buf_ops);
> @@ -1374,7 +1374,7 @@ extern kmem_zone_t	*xfs_ili_zone;
>    * make sure they're not corrupt.
>    */
>   bool
> -libxfs_inode_verify_forks(
> +xfs_inode_verify_forks(
>   	struct xfs_inode	*ip)
>   {
>   	struct xfs_ifork	*ifp;
> @@ -1403,7 +1403,7 @@ libxfs_inode_verify_forks(
>   }
>   
>   int
> -libxfs_iget(
> +xfs_iget(
>   	struct xfs_mount	*mp,
>   	struct xfs_trans	*tp,
>   	xfs_ino_t		ino,
> @@ -1428,8 +1428,8 @@ libxfs_iget(
>   	}
>   
>   	ip->i_fork_ops = ifork_ops;
> -	if (!libxfs_inode_verify_forks(ip)) {
> -		libxfs_irele(ip);
> +	if (!xfs_inode_verify_forks(ip)) {
> +		xfs_irele(ip);
>   		return -EFSCORRUPTED;
>   	}
>   
> @@ -1446,26 +1446,26 @@ libxfs_iget(
>   }
>   
>   static void
> -libxfs_idestroy(xfs_inode_t *ip)
> +xfs_idestroy(xfs_inode_t *ip)
>   {
>   	switch (VFS_I(ip)->i_mode & S_IFMT) {
>   		case S_IFREG:
>   		case S_IFDIR:
>   		case S_IFLNK:
> -			libxfs_idestroy_fork(ip, XFS_DATA_FORK);
> +			xfs_idestroy_fork(ip, XFS_DATA_FORK);
>   			break;
>   	}
>   	if (ip->i_afp)
> -		libxfs_idestroy_fork(ip, XFS_ATTR_FORK);
> +		xfs_idestroy_fork(ip, XFS_ATTR_FORK);
>   	if (ip->i_cowfp)
>   		xfs_idestroy_fork(ip, XFS_COW_FORK);
>   }
>   
>   void
> -libxfs_irele(
> +xfs_irele(
>   	struct xfs_inode	*ip)
>   {
>   	ASSERT(ip->i_itemp == NULL);
> -	libxfs_idestroy(ip);
> +	xfs_idestroy(ip);
>   	kmem_zone_free(xfs_inode_zone, ip);
>   }
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index cb15552c..5ef0c055 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -36,7 +36,7 @@ kmem_zone_t	*xfs_trans_zone;
>    * in the mount structure.
>    */
>   void
> -libxfs_trans_init(
> +xfs_trans_init(
>   	struct xfs_mount	*mp)
>   {
>   	xfs_trans_resv_calc(mp, &mp->m_resv);
> @@ -46,7 +46,7 @@ libxfs_trans_init(
>    * Add the given log item to the transaction's list of log items.
>    */
>   void
> -libxfs_trans_add_item(
> +xfs_trans_add_item(
>   	struct xfs_trans	*tp,
>   	struct xfs_log_item	*lip)
>   {
> @@ -62,7 +62,7 @@ libxfs_trans_add_item(
>    * Unlink and free the given descriptor.
>    */
>   void
> -libxfs_trans_del_item(
> +xfs_trans_del_item(
>   	struct xfs_log_item	*lip)
>   {
>   	clear_bit(XFS_LI_DIRTY, &lip->li_flags);
> @@ -77,7 +77,7 @@ libxfs_trans_del_item(
>    * chunk we've been working on and get a new transaction to continue.
>    */
>   int
> -libxfs_trans_roll(
> +xfs_trans_roll(
>   	struct xfs_trans	**tpp)
>   {
>   	struct xfs_trans	*trans = *tpp;
> @@ -245,7 +245,7 @@ undo_blocks:
>   }
>   
>   int
> -libxfs_trans_alloc(
> +xfs_trans_alloc(
>   	struct xfs_mount	*mp,
>   	struct xfs_trans_res	*resp,
>   	unsigned int		blocks,
> @@ -289,7 +289,7 @@ libxfs_trans_alloc(
>    * without any dirty data.
>    */
>   int
> -libxfs_trans_alloc_empty(
> +xfs_trans_alloc_empty(
>   	struct xfs_mount		*mp,
>   	struct xfs_trans		**tpp)
>   {
> @@ -304,17 +304,17 @@ libxfs_trans_alloc_empty(
>    * permanent log reservation flag to avoid blowing asserts.
>    */
>   int
> -libxfs_trans_alloc_rollable(
> +xfs_trans_alloc_rollable(
>   	struct xfs_mount	*mp,
>   	unsigned int		blocks,
>   	struct xfs_trans	**tpp)
>   {
> -	return libxfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, blocks,
> +	return xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, blocks,
>   			0, 0, tpp);
>   }
>   
>   void
> -libxfs_trans_cancel(
> +xfs_trans_cancel(
>   	struct xfs_trans	*tp)
>   {
>   #ifdef XACT_DEBUG
> @@ -337,7 +337,7 @@ out:
>   }
>   
>   void
> -libxfs_trans_ijoin(
> +xfs_trans_ijoin(
>   	xfs_trans_t		*tp,
>   	xfs_inode_t		*ip,
>   	uint			lock_flags)
> @@ -360,7 +360,7 @@ libxfs_trans_ijoin(
>   }
>   
>   void
> -libxfs_trans_inode_alloc_buf(
> +xfs_trans_inode_alloc_buf(
>   	xfs_trans_t		*tp,
>   	xfs_buf_t		*bp)
>   {
> @@ -407,7 +407,7 @@ xfs_trans_log_inode(
>   }
>   
>   int
> -libxfs_trans_roll_inode(
> +xfs_trans_roll_inode(
>   	struct xfs_trans	**tpp,
>   	struct xfs_inode	*ip)
>   {
> @@ -425,7 +425,7 @@ libxfs_trans_roll_inode(
>    * Mark a buffer dirty in the transaction.
>    */
>   void
> -libxfs_trans_dirty_buf(
> +xfs_trans_dirty_buf(
>   	struct xfs_trans	*tp,
>   	struct xfs_buf		*bp)
>   {
> @@ -451,7 +451,7 @@ libxfs_trans_dirty_buf(
>    * value of b_blkno.
>    */
>   void
> -libxfs_trans_log_buf(
> +xfs_trans_log_buf(
>   	struct xfs_trans	*tp,
>   	struct xfs_buf		*bp,
>   	uint			first,
> @@ -473,7 +473,7 @@ libxfs_trans_log_buf(
>    * If the buffer is already dirty, trigger the "already logged" return condition.
>    */
>   bool
> -libxfs_trans_ordered_buf(
> +xfs_trans_ordered_buf(
>   	struct xfs_trans	*tp,
>   	struct xfs_buf		*bp)
>   {
> @@ -481,7 +481,7 @@ libxfs_trans_ordered_buf(
>   	bool			ret;
>   
>   	ret = test_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags);
> -	libxfs_trans_log_buf(tp, bp, 0, bp->b_bcount);
> +	xfs_trans_log_buf(tp, bp, 0, bp->b_bcount);
>   	return ret;
>   }
>   
> @@ -496,7 +496,7 @@ xfs_buf_item_put(
>   }
>   
>   void
> -libxfs_trans_brelse(
> +xfs_trans_brelse(
>   	xfs_trans_t		*tp,
>   	xfs_buf_t		*bp)
>   {
> @@ -531,7 +531,7 @@ libxfs_trans_brelse(
>   }
>   
>   void
> -libxfs_trans_binval(
> +xfs_trans_binval(
>   	xfs_trans_t		*tp,
>   	xfs_buf_t		*bp)
>   {
> @@ -556,7 +556,7 @@ libxfs_trans_binval(
>   }
>   
>   void
> -libxfs_trans_bjoin(
> +xfs_trans_bjoin(
>   	xfs_trans_t		*tp,
>   	xfs_buf_t		*bp)
>   {
> @@ -574,7 +574,7 @@ libxfs_trans_bjoin(
>   }
>   
>   void
> -libxfs_trans_bhold(
> +xfs_trans_bhold(
>   	xfs_trans_t		*tp,
>   	xfs_buf_t		*bp)
>   {
> @@ -590,7 +590,7 @@ libxfs_trans_bhold(
>   }
>   
>   xfs_buf_t *
> -libxfs_trans_get_buf_map(
> +xfs_trans_get_buf_map(
>   	xfs_trans_t		*tp,
>   	struct xfs_buftarg	*btp,
>   	struct xfs_buf_map	*map,
> @@ -619,14 +619,14 @@ libxfs_trans_get_buf_map(
>   	fprintf(stderr, "trans_get_buf buffer %p, transaction %p\n", bp, tp);
>   #endif
>   
> -	libxfs_trans_bjoin(tp, bp);
> +	xfs_trans_bjoin(tp, bp);
>   	bip = bp->b_log_item;
>   	bip->bli_recur = 0;
>   	return bp;
>   }
>   
>   xfs_buf_t *
> -libxfs_trans_getsb(
> +xfs_trans_getsb(
>   	xfs_trans_t		*tp,
>   	xfs_mount_t		*mp,
>   	int			flags)
> @@ -637,7 +637,7 @@ libxfs_trans_getsb(
>   	DEFINE_SINGLE_BUF_MAP(map, XFS_SB_DADDR, len);
>   
>   	if (tp == NULL)
> -		return libxfs_getsb(mp, flags);
> +		return xfs_getsb(mp, flags);
>   
>   	bp = xfs_trans_buf_item_match(tp, mp->m_dev, &map, 1);
>   	if (bp != NULL) {
> @@ -648,19 +648,19 @@ libxfs_trans_getsb(
>   		return bp;
>   	}
>   
> -	bp = libxfs_getsb(mp, flags);
> +	bp = xfs_getsb(mp, flags);
>   #ifdef XACT_DEBUG
>   	fprintf(stderr, "trans_get_sb buffer %p, transaction %p\n", bp, tp);
>   #endif
>   
> -	libxfs_trans_bjoin(tp, bp);
> +	xfs_trans_bjoin(tp, bp);
>   	bip = bp->b_log_item;
>   	bip->bli_recur = 0;
>   	return bp;
>   }
>   
>   int
> -libxfs_trans_read_buf_map(
> +xfs_trans_read_buf_map(
>   	xfs_mount_t		*mp,
>   	xfs_trans_t		*tp,
>   	struct xfs_buftarg	*btp,
> @@ -728,7 +728,7 @@ out_relse:
>    * Originally derived from xfs_trans_mod_sb().
>    */
>   void
> -libxfs_trans_mod_sb(
> +xfs_trans_mod_sb(
>   	xfs_trans_t		*tp,
>   	uint			field,
>   	long			delta)
> @@ -814,7 +814,7 @@ inode_item_done(
>   	 * of whether the flush succeed or not. If we fail the flush, make sure
>   	 * we still release the buffer reference we currently hold.
>   	 */
> -	error = libxfs_iflush_int(ip, bp);
> +	error = xfs_iflush_int(ip, bp);
>   	bp->b_transp = NULL;	/* remove xact ptr */
>   
>   	if (error) {
> @@ -989,7 +989,7 @@ out_unreserve:
>   }
>   
>   int
> -libxfs_trans_commit(
> +xfs_trans_commit(
>   	struct xfs_trans	*tp)
>   {
>   	return __xfs_trans_commit(tp, false);
> diff --git a/libxfs/util.c b/libxfs/util.c
> index 8c9954f6..4901123a 100644
> --- a/libxfs/util.c
> +++ b/libxfs/util.c
> @@ -143,7 +143,7 @@ xfs_log_calc_unit_res(
>    * where it's no longer worth the hassle of maintaining common code.
>    */
>   void
> -libxfs_trans_ichgtime(
> +xfs_trans_ichgtime(
>   	struct xfs_trans	*tp,
>   	struct xfs_inode	*ip,
>   	int			flags)
> @@ -232,7 +232,7 @@ xfs_flags2diflags2(
>    * where it's no longer worth the hassle of maintaining common code.
>    */
>   static int
> -libxfs_ialloc(
> +xfs_ialloc(
>   	xfs_trans_t	*tp,
>   	xfs_inode_t	*pip,
>   	mode_t		mode,
> @@ -262,7 +262,7 @@ libxfs_ialloc(
>   	}
>   	ASSERT(*ialloc_context == NULL);
>   
> -	error = libxfs_iget(tp->t_mountp, tp, ino, 0, &ip,
> +	error = xfs_iget(tp->t_mountp, tp, ino, 0, &ip,
>   			&xfs_default_ifork_ops);
>   	if (error != 0)
>   		return error;
> @@ -388,7 +388,7 @@ libxfs_ialloc(
>    * Originally based on xfs_iflush_int() from xfs_inode.c in the kernel.
>    */
>   int
> -libxfs_iflush_int(xfs_inode_t *ip, xfs_buf_t *bp)
> +xfs_iflush_int(xfs_inode_t *ip, xfs_buf_t *bp)
>   {
>   	xfs_inode_log_item_t	*iip;
>   	xfs_dinode_t		*dip;
> @@ -421,7 +421,7 @@ libxfs_iflush_int(xfs_inode_t *ip, xfs_buf_t *bp)
>   		VFS_I(ip)->i_version++;
>   
>   	/* Check the inline fork data before we write out. */
> -	if (!libxfs_inode_verify_forks(ip))
> +	if (!xfs_inode_verify_forks(ip))
>   		return -EFSCORRUPTED;
>   
>   	/*
> @@ -467,10 +467,9 @@ libxfs_mod_incore_sb(
>   
>   /*
>    * This routine allocates disk space for the given file.
> - * Originally derived from xfs_alloc_file_space().
>    */
>   int
> -libxfs_alloc_file_space(
> +xfs_alloc_file_space(
>   	xfs_inode_t	*ip,
>   	xfs_off_t	offset,
>   	xfs_off_t	len,
> @@ -547,14 +546,14 @@ error0:	/* Cancel bmap, cancel trans */
>   }
>   
>   /*
> - * Wrapper around call to libxfs_ialloc. Takes care of committing and
> + * Wrapper around call to xfs_ialloc. Takes care of committing and
>    * allocating a new transaction as needed.
>    *
>    * Originally there were two copies of this code - one in mkfs, the
>    * other in repair - now there is just the one.
>    */
>   int
> -libxfs_inode_alloc(
> +xfs_inode_alloc(
>   	xfs_trans_t	**tp,
>   	xfs_inode_t	*pip,
>   	mode_t		mode,
> @@ -569,7 +568,7 @@ libxfs_inode_alloc(
>   	int		error;
>   
>   	ialloc_context = (xfs_buf_t *)0;
> -	error = libxfs_ialloc(*tp, pip, mode, nlink, rdev, cr, fsx,
> +	error = xfs_ialloc(*tp, pip, mode, nlink, rdev, cr, fsx,
>   			   &ialloc_context, &ip);
>   	if (error) {
>   		*ipp = NULL;
> @@ -591,7 +590,7 @@ libxfs_inode_alloc(
>   			exit(1);
>   		}
>   		xfs_trans_bjoin(*tp, ialloc_context);
> -		error = libxfs_ialloc(*tp, pip, mode, nlink, rdev, cr,
> +		error = xfs_ialloc(*tp, pip, mode, nlink, rdev, cr,
>   				   fsx, &ialloc_context, &ip);
>   		if (!ip)
>   			error = -ENOSPC;
> @@ -712,7 +711,7 @@ xfs_fsb_to_db(struct xfs_inode *ip, xfs_fsblock_t fsb)
>   }
>   
>   int
> -libxfs_zero_extent(
> +xfs_zero_extent(
>   	struct xfs_inode *ip,
>   	xfs_fsblock_t	start_fsb,
>   	xfs_off_t	count_fsb)
> 

Re: [PATCH 1/3] libxfs: rename shared kernel functions from libxfs_ to xfs_

[Eric Sandeen]

On 5/17/19 3:49 PM, Allison Collins wrote:
> 
> 
> On 5/16/19 10:43 AM, Eric Sandeen wrote:
>> The libxfs_* function namespace has gotten a bit confused and
>> muddled; the general idea is that functions called from userspace
>> utilities should use the libxfs_* namespace.  In many cases
>> we use #defines to define xfs_* namespace to libxfs_*; in other
>> cases we have explicitly defined libxfs_* functions which are clear
>> counterparts or even clones of kernel libxfs/* functions.
>>
>> For any function definitions within libxfs/* which match kernel
>> names, give them standard xfs_* names to further reduce differnces
>> between userspace and kernel libxfs/* code.
>>
>> Then add #defines to libxfs_* for any functions which are needed
>> by utilities, as is done with other core functionality.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/include/libxfs.h b/include/libxfs.h
>> index 230bc3e8..ceebccdc 100644
>> --- a/include/libxfs.h
>> +++ b/include/libxfs.h
>> @@ -151,7 +151,7 @@ extern int    libxfs_log_header(char *, uuid_t *, int, int, int, xfs_lsn_t,
>>     /* Shared utility routines */
>>   -extern int    libxfs_alloc_file_space (struct xfs_inode *, xfs_off_t,
>> +extern int    xfs_alloc_file_space (struct xfs_inode *, xfs_off_t,
>>                   xfs_off_t, int, int);
>>     /* XXX: this is messy and needs fixing */
>> diff --git a/include/xfs_inode.h b/include/xfs_inode.h
>> index 88b58ac3..3e7e80ea 100644
>> --- a/include/xfs_inode.h
>> +++ b/include/xfs_inode.h
>> @@ -139,21 +139,21 @@ typedef struct cred {
>>       gid_t    cr_gid;
>>   } cred_t;
>>   -extern int    libxfs_inode_alloc (struct xfs_trans **, struct xfs_inode *,
>> +extern int    xfs_inode_alloc (struct xfs_trans **, struct xfs_inode *,
>>                   mode_t, nlink_t, xfs_dev_t, struct cred *,
>>                   struct fsxattr *, struct xfs_inode **);
>> -extern void    libxfs_trans_inode_alloc_buf (struct xfs_trans *,
>> +extern void    xfs_trans_inode_alloc_buf (struct xfs_trans *,
>>                   struct xfs_buf *);
>>   -extern void    libxfs_trans_ichgtime(struct xfs_trans *,
>> +extern void    xfs_trans_ichgtime(struct xfs_trans *,
>>                   struct xfs_inode *, int);
>> -extern int    libxfs_iflush_int (struct xfs_inode *, struct xfs_buf *);
>> +extern int    xfs_iflush_int (struct xfs_inode *, struct xfs_buf *);
>>     /* Inode Cache Interfaces */
>> -extern bool    libxfs_inode_verify_forks(struct xfs_inode *ip);
>> -extern int    libxfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
>> +extern bool    xfs_inode_verify_forks(struct xfs_inode *ip);
>> +extern int    xfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
>>                   uint, struct xfs_inode **,
>>                   struct xfs_ifork_ops *);
>> -extern void    libxfs_irele(struct xfs_inode *ip);
>> +extern void    xfs_irele(struct xfs_inode *ip);
>>     #endif /* __XFS_INODE_H__ */
>> diff --git a/include/xfs_trans.h b/include/xfs_trans.h
>> index 10b74538..d32acc9e 100644
>> --- a/include/xfs_trans.h
>> +++ b/include/xfs_trans.h
>> @@ -75,46 +75,46 @@ typedef struct xfs_trans {
>>   void    xfs_trans_init(struct xfs_mount *);
>>   int    xfs_trans_roll(struct xfs_trans **);
>>   -int    libxfs_trans_alloc(struct xfs_mount *mp, struct xfs_trans_res *resp,
>> +int    xfs_trans_alloc(struct xfs_mount *mp, struct xfs_trans_res *resp,
>>                  uint blocks, uint rtextents, uint flags,
>>                  struct xfs_trans **tpp);
>>   int    libxfs_trans_alloc_rollable(struct xfs_mount *mp, uint blocks,
>>                       struct xfs_trans **tpp);
> 
> Did you mean to rename libxfs_trans_alloc_rollable too?  I notice the function name is changed later down in the patch.

Not sure - it's not actually a kernel-matching function, it should
probably get moved to util.c or something, since it's
userspace-specific.

But yeah, I guess the way I handled it is a little inconsistent,
probably saved by the #define.  :)

-Eric

> I think the rest of it looks pretty straight forward though.
> 
> Allison

Re: [PATCH 2/3] libxfs: remove libxfs API #defines for unexported xfs functions

[Allison Collins]

On 5/16/19 10:45 AM, Eric Sandeen wrote:
> We define "libxfs_*" functions for anything used by userspace,
> called from outside the libxfs/ directory.  However, many of the
> current redefinitions are for functions only used within libxfs/*
> so remove their API redefinitions.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

Ok, looks good to me.

Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> 
> diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
> index 2b8ac5ab..a53efa68 100644
> --- a/libxfs/libxfs_api_defs.h
> +++ b/libxfs/libxfs_api_defs.h
> @@ -18,37 +18,22 @@
>   
>   #define xfs_trans_alloc			libxfs_trans_alloc
>   #define xfs_trans_alloc_rollable	libxfs_trans_alloc_rollable
> -#define xfs_trans_alloc_empty		libxfs_trans_alloc_empty
> -#define xfs_trans_add_item		libxfs_trans_add_item
>   #define xfs_trans_bhold			libxfs_trans_bhold
>   #define xfs_trans_binval		libxfs_trans_binval
>   #define xfs_trans_bjoin			libxfs_trans_bjoin
>   #define xfs_trans_brelse		libxfs_trans_brelse
>   #define xfs_trans_commit		libxfs_trans_commit
>   #define xfs_trans_cancel		libxfs_trans_cancel
> -#define xfs_trans_del_item		libxfs_trans_del_item
>   #define xfs_trans_get_buf		libxfs_trans_get_buf
> -#define xfs_trans_getsb			libxfs_trans_getsb
>   #define xfs_trans_ichgtime		libxfs_trans_ichgtime
>   #define xfs_trans_ijoin			libxfs_trans_ijoin
> -#define xfs_trans_init			libxfs_trans_init
> -#define xfs_trans_inode_alloc_buf	libxfs_trans_inode_alloc_buf
> -#define xfs_trans_dirty_buf		libxfs_trans_dirty_buf
>   #define xfs_trans_log_buf		libxfs_trans_log_buf
> -#define xfs_trans_ordered_buf		libxfs_trans_ordered_buf
>   #define xfs_trans_log_inode		libxfs_trans_log_inode
>   #define xfs_trans_roll_inode		libxfs_trans_roll_inode
> -#define xfs_trans_mod_sb		libxfs_trans_mod_sb
>   #define xfs_trans_read_buf		libxfs_trans_read_buf
> -#define xfs_trans_read_buf_map		libxfs_trans_read_buf_map
> -#define xfs_trans_roll			libxfs_trans_roll
> -#define xfs_trans_get_buf_map		libxfs_trans_get_buf_map
> -#define xfs_trans_resv_calc		libxfs_trans_resv_calc
>   #define xfs_log_get_max_trans_res	libxfs_log_get_max_trans_res
> -#define xfs_attr_get			libxfs_attr_get
>   #define xfs_attr_set			libxfs_attr_set
>   #define xfs_attr_remove			libxfs_attr_remove
> -#define xfs_attr_leaf_newentsize	libxfs_attr_leaf_newentsize
>   
>   #define xfs_agfl_walk			libxfs_agfl_walk
>   #define xfs_alloc_fix_freelist		libxfs_alloc_fix_freelist
> @@ -57,15 +42,11 @@
>   #define xfs_bmap_last_offset		libxfs_bmap_last_offset
>   #define xfs_iext_lookup_extent		libxfs_iext_lookup_extent
>   #define xfs_bmapi_write			libxfs_bmapi_write
> -#define xfs_bmapi_read			libxfs_bmapi_read
>   #define xfs_bunmapi			libxfs_bunmapi
>   #define xfs_rtfree_extent		libxfs_rtfree_extent
> -#define xfs_verify_rtbno		libxfs_verify_rtbno
>   #define xfs_verify_ino			libxfs_verify_ino
> -#define xfs_zero_extent			libxfs_zero_extent
>   
>   #define xfs_defer_finish		libxfs_defer_finish
> -#define xfs_defer_cancel		libxfs_defer_cancel
>   
>   #define xfs_da_hashname			libxfs_da_hashname
>   #define xfs_da_shrink_inode		libxfs_da_shrink_inode
> @@ -85,14 +66,11 @@
>   #define xfs_da_get_buf			libxfs_da_get_buf
>   
>   #define xfs_inode_from_disk		libxfs_inode_from_disk
> -#define xfs_inode_to_disk		libxfs_inode_to_disk
>   #define xfs_dinode_calc_crc		libxfs_dinode_calc_crc
>   #define xfs_idata_realloc		libxfs_idata_realloc
> -#define xfs_idestroy_fork		libxfs_idestroy_fork
>   #define xfs_inode_validate_extsize	libxfs_inode_validate_extsize
>   #define xfs_inode_validate_cowextsize	libxfs_inode_validate_cowextsize
>   #define xfs_inode_alloc			libxfs_inode_alloc
> -#define xfs_iflush_int			libxfs_iflush_int
>   #define xfs_alloc_file_space		libxfs_alloc_file_space
>   
>   #define xfs_rmap_alloc			libxfs_rmap_alloc
> @@ -107,7 +85,6 @@
>   
>   #define xfs_log_sb			libxfs_log_sb
>   #define xfs_sb_from_disk		libxfs_sb_from_disk
> -#define xfs_sb_quota_from_disk		libxfs_sb_quota_from_disk
>   #define xfs_sb_to_disk			libxfs_sb_to_disk
>   
>   #define xfs_calc_dquots_per_chunk	libxfs_calc_dquots_per_chunk
> @@ -151,6 +128,5 @@
>   #define xfs_getsb			libxfs_getsb
>   #define xfs_irele			libxfs_irele
>   #define xfs_iget			libxfs_iget
> -#define xfs_inode_verify_forks		libxfs_inode_verify_forks
>   
>   #endif /* __LIBXFS_API_DEFS_H__ */
> 

Re: [PATCH 3/3] xfsprogs: remove unused flags arg from getsb interfaces

[Allison Collins]

On 5/16/19 10:46 AM, Eric Sandeen wrote:
> The flags value is always* passed as 0 so remove the argument.
> 
> *mkfs.xfs is the sole exception; it passes a flag to fail on read
> error, but we can easily detect the error and exit from main()
> manually instead.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks ok to me.

Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> ---
> 
> (This one might actually come in via a libxfs merge instead)
> 
> diff --git a/include/xfs_trans.h b/include/xfs_trans.h
> index d32acc9e..953da5d1 100644
> --- a/include/xfs_trans.h
> +++ b/include/xfs_trans.h
> @@ -87,7 +87,7 @@ void	xfs_trans_cancel(struct xfs_trans *);
>   /* cancel dfops associated with a transaction */
>   void xfs_defer_cancel(struct xfs_trans *);
>   
> -struct xfs_buf *xfs_trans_getsb(struct xfs_trans *, struct xfs_mount *, int);
> +struct xfs_buf *xfs_trans_getsb(struct xfs_trans *, struct xfs_mount *);
>   
>   void	xfs_trans_ijoin(struct xfs_trans *, struct xfs_inode *, uint);
>   void	xfs_trans_log_inode (struct xfs_trans *, struct xfs_inode *,
> diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
> index 8fa2c2c5..4dc4d5f3 100644
> --- a/libxfs/libxfs_io.h
> +++ b/libxfs/libxfs_io.h
> @@ -177,7 +177,7 @@ extern void	libxfs_putbuf (xfs_buf_t *);
>   
>   extern void	libxfs_readbuf_verify(struct xfs_buf *bp,
>   			const struct xfs_buf_ops *ops);
> -extern xfs_buf_t *xfs_getsb(struct xfs_mount *, int);
> +extern xfs_buf_t *xfs_getsb(struct xfs_mount *);
>   extern void	libxfs_bcache_purge(void);
>   extern void	libxfs_bcache_free(void);
>   extern void	libxfs_bcache_flush(void);
> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> index 8d6f958a..132ed0a9 100644
> --- a/libxfs/rdwr.c
> +++ b/libxfs/rdwr.c
> @@ -476,10 +476,10 @@ libxfs_trace_putbuf(const char *func, const char *file, int line, xfs_buf_t *bp)
>   
>   
>   xfs_buf_t *
> -xfs_getsb(xfs_mount_t *mp, int flags)
> +xfs_getsb(xfs_mount_t *mp)
>   {
>   	return libxfs_readbuf(mp->m_ddev_targp, XFS_SB_DADDR,
> -				XFS_FSS_TO_BB(mp, 1), flags, &xfs_sb_buf_ops);
> +				XFS_FSS_TO_BB(mp, 1), 0, &xfs_sb_buf_ops);
>   }
>   
>   kmem_zone_t			*xfs_buf_zone;
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index 5ef0c055..9e49def0 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -628,8 +628,7 @@ xfs_trans_get_buf_map(
>   xfs_buf_t *
>   xfs_trans_getsb(
>   	xfs_trans_t		*tp,
> -	xfs_mount_t		*mp,
> -	int			flags)
> +	xfs_mount_t		*mp)
>   {
>   	xfs_buf_t		*bp;
>   	xfs_buf_log_item_t	*bip;
> @@ -637,7 +636,7 @@ xfs_trans_getsb(
>   	DEFINE_SINGLE_BUF_MAP(map, XFS_SB_DADDR, len);
>   
>   	if (tp == NULL)
> -		return xfs_getsb(mp, flags);
> +		return xfs_getsb(mp);
>   
>   	bp = xfs_trans_buf_item_match(tp, mp->m_dev, &map, 1);
>   	if (bp != NULL) {
> @@ -648,7 +647,7 @@ xfs_trans_getsb(
>   		return bp;
>   	}
>   
> -	bp = xfs_getsb(mp, flags);
> +	bp = xfs_getsb(mp);
>   #ifdef XACT_DEBUG
>   	fprintf(stderr, "trans_get_sb buffer %p, transaction %p\n", bp, tp);
>   #endif
> diff --git a/libxfs/xfs_sb.c b/libxfs/xfs_sb.c
> index f1e2ba57..a1857e10 100644
> --- a/libxfs/xfs_sb.c
> +++ b/libxfs/xfs_sb.c
> @@ -915,7 +915,7 @@ xfs_log_sb(
>   	struct xfs_trans	*tp)
>   {
>   	struct xfs_mount	*mp = tp->t_mountp;
> -	struct xfs_buf		*bp = xfs_trans_getsb(tp, mp, 0);
> +	struct xfs_buf		*bp = xfs_trans_getsb(tp, mp);
>   
>   	mp->m_sb.sb_icount = percpu_counter_sum(&mp->m_icount);
>   	mp->m_sb.sb_ifree = percpu_counter_sum(&mp->m_ifree);
> @@ -1045,7 +1045,7 @@ xfs_sync_sb_buf(
>   	if (error)
>   		return error;
>   
> -	bp = xfs_trans_getsb(tp, mp, 0);
> +	bp = xfs_trans_getsb(tp, mp);
>   	xfs_log_sb(tp);
>   	xfs_trans_bhold(tp, bp);
>   	xfs_trans_set_sync(tp);
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 09106648..647e2bc2 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -4123,7 +4123,11 @@ main(
>   	/*
>   	 * Mark the filesystem ok.
>   	 */
> -	buf = libxfs_getsb(mp, LIBXFS_EXIT_ON_FAILURE);
> +	buf = libxfs_getsb(mp);
> +	if (!buf) {
> +		fprintf(stderr, _("couldn't get superblock\n"));
> +		exit(1);
> +	}
>   	(XFS_BUF_TO_SBP(buf))->sb_inprogress = 0;
>   	libxfs_writebuf(buf, LIBXFS_EXIT_ON_FAILURE);
>   
> diff --git a/repair/phase5.c b/repair/phase5.c
> index 0f6a8395..8ad32365 100644
> --- a/repair/phase5.c
> +++ b/repair/phase5.c
> @@ -2177,7 +2177,7 @@ sync_sb(xfs_mount_t *mp)
>   {
>   	xfs_buf_t	*bp;
>   
> -	bp = libxfs_getsb(mp, 0);
> +	bp = libxfs_getsb(mp);
>   	if (!bp)
>   		do_error(_("couldn't get superblock\n"));
>   
> diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
> index 9657503f..4000b3e6 100644
> --- a/repair/xfs_repair.c
> +++ b/repair/xfs_repair.c
> @@ -1044,7 +1044,7 @@ _("Warning:  project quota information would be cleared.\n"
>   	/*
>   	 * Clear the quota flags if they're on.
>   	 */
> -	sbp = libxfs_getsb(mp, 0);
> +	sbp = libxfs_getsb(mp);
>   	if (!sbp)
>   		do_error(_("couldn't get superblock\n"));
>   
> 

Re: [PATCH 4/3] libxfs: Remove XACT_DEBUG #ifdefs

[Allison Collins]

On 5/16/19 1:38 PM, Eric Sandeen wrote:
> Remove XACT_DEBUG #ifdefs to reduce more cosmetic differences
> between userspace & kernelspace libxfs.  Add in some corresponding
> (stubbed-out) tracepoint calls.
> 
> If these are felt to be particularly useful, the tracepoint calls
> could be fleshed out to provide similar information.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks ok.  It seems a shame to loose the extra tracing, but I confess 
most of the time when I'm chasing a particular bug I end up adding my 
own temporary traces anyway.  So I think trying to synchronize the code 
spaces is more important.  Do you think the stubs might be something to 
add to the kernel space side too?

Allison

> ---
>   include/xfs_trace.h | 13 +++++++
>   libxfs/trans.c      | 85 ++++++++++++---------------------------------
>   2 files changed, 35 insertions(+), 63 deletions(-)
> 
> diff --git a/include/xfs_trace.h b/include/xfs_trace.h
> index 793ac56e..43720040 100644
> --- a/include/xfs_trace.h
> +++ b/include/xfs_trace.h
> @@ -161,6 +161,19 @@
>   #define trace_xfs_perag_get_tag(a,b,c,d) ((c) = (c))
>   #define trace_xfs_perag_put(a,b,c,d)	((c) = (c))
>   
> +#define trace_xfs_trans_alloc(a,b)		((void) 0)
> +#define trace_xfs_trans_cancel(a,b)		((void) 0)
> +#define trace_xfs_trans_brelse(a)		((void) 0)
> +#define trace_xfs_trans_binval(a)		((void) 0)
> +#define trace_xfs_trans_bjoin(a)		((void) 0)
> +#define trace_xfs_trans_bhold(a)		((void) 0)
> +#define trace_xfs_trans_get_buf(a)		((void) 0)
> +#define trace_xfs_trans_getsb_recur(a)		((void) 0)
> +#define trace_xfs_trans_getsb(a)		((void) 0)
> +#define trace_xfs_trans_read_buf_recur(a)	((void) 0)
> +#define trace_xfs_trans_read_buf(a)		((void) 0)
> +#define trace_xfs_trans_commit(a,b)		((void) 0)
> +
>   #define trace_xfs_defer_cancel(a,b)		((void) 0)
>   #define trace_xfs_defer_pending_commit(a,b)	((void) 0)
>   #define trace_xfs_defer_pending_abort(a,b)	((void) 0)
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index 9e49def0..6967a1de 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -18,6 +18,7 @@
>   #include "xfs_trans.h"
>   #include "xfs_sb.h"
>   #include "xfs_defer.h"
> +#include "xfs_trace.h"
>   
>   static void xfs_trans_free_items(struct xfs_trans *tp);
>   STATIC struct xfs_trans *xfs_trans_dup(struct xfs_trans *tp);
> @@ -269,9 +270,9 @@ xfs_trans_alloc(
>   		xfs_trans_cancel(tp);
>   		return error;
>   	}
> -#ifdef XACT_DEBUG
> -	fprintf(stderr, "allocated new transaction %p\n", tp);
> -#endif
> +
> +	trace_xfs_trans_alloc(tp, _RET_IP_);
> +
>   	*tpp = tp;
>   	return 0;
>   }
> @@ -317,23 +318,16 @@ void
>   xfs_trans_cancel(
>   	struct xfs_trans	*tp)
>   {
> -#ifdef XACT_DEBUG
> -	struct xfs_trans	*otp = tp;
> -#endif
> +	trace_xfs_trans_cancel(tp, _RET_IP_);
> +
>   	if (tp == NULL)
> -		goto out;
> +		return;
>   
>   	if (tp->t_flags & XFS_TRANS_PERM_LOG_RES)
>   		xfs_defer_cancel(tp);
>   
>   	xfs_trans_free_items(tp);
>   	xfs_trans_free(tp);
> -
> -out:
> -#ifdef XACT_DEBUG
> -	fprintf(stderr, "## cancelled transaction %p\n", otp);
> -#endif
> -	return;
>   }
>   
>   void
> @@ -353,10 +347,6 @@ xfs_trans_ijoin(
>   	iip->ili_lock_flags = lock_flags;
>   
>   	xfs_trans_add_item(tp, (xfs_log_item_t *)(iip));
> -
> -#ifdef XACT_DEBUG
> -	fprintf(stderr, "ijoin'd inode %llu, transaction %p\n", ip->i_ino, tp);
> -#endif
>   }
>   
>   void
> @@ -388,9 +378,6 @@ xfs_trans_log_inode(
>   	uint			flags)
>   {
>   	ASSERT(ip->i_itemp != NULL);
> -#ifdef XACT_DEBUG
> -	fprintf(stderr, "dirtied inode %llu, transaction %p\n", ip->i_ino, tp);
> -#endif
>   
>   	tp->t_flags |= XFS_TRANS_DIRTY;
>   	set_bit(XFS_LI_DIRTY, &ip->i_itemp->ili_item.li_flags);
> @@ -434,9 +421,6 @@ xfs_trans_dirty_buf(
>   	ASSERT(bp->b_transp == tp);
>   	ASSERT(bip != NULL);
>   
> -#ifdef XACT_DEBUG
> -	fprintf(stderr, "dirtied buffer %p, transaction %p\n", bp, tp);
> -#endif
>   	tp->t_flags |= XFS_TRANS_DIRTY;
>   	set_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags);
>   }
> @@ -501,15 +485,14 @@ xfs_trans_brelse(
>   	xfs_buf_t		*bp)
>   {
>   	xfs_buf_log_item_t	*bip;
> -#ifdef XACT_DEBUG
> -	fprintf(stderr, "released buffer %p, transaction %p\n", bp, tp);
> -#endif
>   
>   	if (tp == NULL) {
>   		ASSERT(bp->b_transp == NULL);
>   		libxfs_putbuf(bp);
>   		return;
>   	}
> +
> +	trace_xfs_trans_brelse(bip);
>   	ASSERT(bp->b_transp == tp);
>   	bip = bp->b_log_item;
>   	ASSERT(bip->bli_item.li_type == XFS_LI_BUF);
> @@ -536,13 +519,12 @@ xfs_trans_binval(
>   	xfs_buf_t		*bp)
>   {
>   	xfs_buf_log_item_t	*bip = bp->b_log_item;
> -#ifdef XACT_DEBUG
> -	fprintf(stderr, "binval'd buffer %p, transaction %p\n", bp, tp);
> -#endif
>   
>   	ASSERT(bp->b_transp == tp);
>   	ASSERT(bip != NULL);
>   
> +	trace_xfs_trans_binval(bip);
> +
>   	if (bip->bli_flags & XFS_BLI_STALE)
>   		return;
>   	XFS_BUF_UNDELAYWRITE(bp);
> @@ -563,14 +545,12 @@ xfs_trans_bjoin(
>   	xfs_buf_log_item_t	*bip;
>   
>   	ASSERT(bp->b_transp == NULL);
> -#ifdef XACT_DEBUG
> -	fprintf(stderr, "bjoin'd buffer %p, transaction %p\n", bp, tp);
> -#endif
>   
>   	xfs_buf_item_init(bp, tp->t_mountp);
>   	bip = bp->b_log_item;
>   	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
>   	bp->b_transp = tp;
> +	trace_xfs_trans_bjoin(bp->b_log_item);
>   }
>   
>   void
> @@ -582,11 +562,9 @@ xfs_trans_bhold(
>   
>   	ASSERT(bp->b_transp == tp);
>   	ASSERT(bip != NULL);
> -#ifdef XACT_DEBUG
> -	fprintf(stderr, "bhold'd buffer %p, transaction %p\n", bp, tp);
> -#endif
>   
>   	bip->bli_flags |= XFS_BLI_HOLD;
> +	trace_xfs_trans_bhold(bip);
>   }
>   
>   xfs_buf_t *
> @@ -615,13 +593,11 @@ xfs_trans_get_buf_map(
>   	bp = libxfs_getbuf_map(btp, map, nmaps, 0);
>   	if (bp == NULL)
>   		return NULL;
> -#ifdef XACT_DEBUG
> -	fprintf(stderr, "trans_get_buf buffer %p, transaction %p\n", bp, tp);
> -#endif
>   
>   	xfs_trans_bjoin(tp, bp);
>   	bip = bp->b_log_item;
>   	bip->bli_recur = 0;
> +	trace_xfs_trans_get_buf(bp->b_log_item);
>   	return bp;
>   }
>   
> @@ -644,17 +620,16 @@ xfs_trans_getsb(
>   		bip = bp->b_log_item;
>   		ASSERT(bip != NULL);
>   		bip->bli_recur++;
> +		trace_xfs_trans_getsb_recur(bip);
>   		return bp;
>   	}
>   
>   	bp = xfs_getsb(mp);
> -#ifdef XACT_DEBUG
> -	fprintf(stderr, "trans_get_sb buffer %p, transaction %p\n", bp, tp);
> -#endif
>   
>   	xfs_trans_bjoin(tp, bp);
>   	bip = bp->b_log_item;
>   	bip->bli_recur = 0;
> +	trace_xfs_trans_getsb(bp->b_log_item);
>   	return bp;
>   }
>   
> @@ -691,6 +666,7 @@ xfs_trans_read_buf_map(
>   		ASSERT(bp->b_log_item != NULL);
>   		bip = bp->b_log_item;
>   		bip->bli_recur++;
> +		trace_xfs_trans_read_buf_recur(bip);
>   		goto done;
>   	}
>   
> @@ -701,14 +677,11 @@ xfs_trans_read_buf_map(
>   	if (bp->b_error)
>   		goto out_relse;
>   
> -#ifdef XACT_DEBUG
> -	fprintf(stderr, "trans_read_buf buffer %p, transaction %p\n", bp, tp);
> -#endif
> -
>   	xfs_trans_bjoin(tp, bp);
>   	bip = bp->b_log_item;
>   	bip->bli_recur = 0;
>   done:
> +	trace_xfs_trans_read_buf(bp->b_log_item);
>   	*bpp = bp;
>   	return 0;
>   out_relse:
> @@ -824,10 +797,6 @@ inode_item_done(
>   	}
>   
>   	libxfs_writebuf(bp, 0);
> -#ifdef XACT_DEBUG
> -	fprintf(stderr, "flushing dirty inode %llu, buffer %p\n",
> -			ip->i_ino, bp);
> -#endif
>   free:
>   	xfs_inode_item_put(iip);
>   }
> @@ -845,13 +814,8 @@ buf_item_done(
>   	bp->b_transp = NULL;			/* remove xact ptr */
>   
>   	hold = (bip->bli_flags & XFS_BLI_HOLD);
> -	if (bip->bli_flags & XFS_BLI_DIRTY) {
> -#ifdef XACT_DEBUG
> -		fprintf(stderr, "flushing/staling buffer %p (hold=%d)\n",
> -			bp, hold);
> -#endif
> +	if (bip->bli_flags & XFS_BLI_DIRTY)
>   		libxfs_writebuf_int(bp, 0);
> -	}
>   
>   	bip->bli_flags &= ~XFS_BLI_HOLD;
>   	xfs_buf_item_put(bip);
> @@ -937,6 +901,8 @@ __xfs_trans_commit(
>   	struct xfs_sb		*sbp;
>   	int			error = 0;
>   
> +	trace_xfs_trans_commit(tp, _RET_IP_);
> +
>   	if (tp == NULL)
>   		return 0;
>   
> @@ -952,12 +918,8 @@ __xfs_trans_commit(
>   			goto out_unreserve;
>   	}
>   
> -	if (!(tp->t_flags & XFS_TRANS_DIRTY)) {
> -#ifdef XACT_DEBUG
> -		fprintf(stderr, "committed clean transaction %p\n", tp);
> -#endif
> +	if (!(tp->t_flags & XFS_TRANS_DIRTY))
>   		goto out_unreserve;
> -	}
>   
>   	if (tp->t_flags & XFS_TRANS_SB_DIRTY) {
>   		sbp = &(tp->t_mountp->m_sb);
> @@ -972,9 +934,6 @@ __xfs_trans_commit(
>   		xfs_log_sb(tp);
>   	}
>   
> -#ifdef XACT_DEBUG
> -	fprintf(stderr, "committing dirty transaction %p\n", tp);
> -#endif
>   	trans_committed(tp);
>   
>   	/* That's it for the transaction structure.  Free it. */
> 

Re: [PATCH 5/3] libxfs: rename bli_format to avoid confusion with bli_formats

[Allison Collins]

On 5/16/19 1:39 PM, Eric Sandeen wrote:
> Rename the bli_format structure to __bli_format to avoid
> accidently confusing them with the bli_formats pointer.
> 
> (nb: userspace currently has no bli_formats pointer)
> 
> Source kernel commit: b94381737e9c4d014a4003e8ece9ba88670a2dd4
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>   include/xfs_trans.h | 2 +-
>   libxfs/logitem.c    | 6 +++---
>   libxfs/trans.c      | 4 ++--
>   3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/include/xfs_trans.h b/include/xfs_trans.h
> index 953da5d1..fe03ba64 100644
> --- a/include/xfs_trans.h
> +++ b/include/xfs_trans.h
> @@ -39,7 +39,7 @@ typedef struct xfs_buf_log_item {
>   	struct xfs_buf		*bli_buf;	/* real buffer pointer */
>   	unsigned int		bli_flags;	/* misc flags */
>   	unsigned int		bli_recur;	/* recursion count */
> -	xfs_buf_log_format_t	bli_format;	/* in-log header */
> +	xfs_buf_log_format_t	__bli_format;	/* in-log header */
>   } xfs_buf_log_item_t;
>   
>   #define XFS_BLI_DIRTY			(1<<0)
> diff --git a/libxfs/logitem.c b/libxfs/logitem.c
> index 4da9bc1b..e862ab4f 100644
> --- a/libxfs/logitem.c
> +++ b/libxfs/logitem.c
> @@ -107,9 +107,9 @@ xfs_buf_item_init(
>   	bip->bli_item.li_mountp = mp;
>   	INIT_LIST_HEAD(&bip->bli_item.li_trans);
>   	bip->bli_buf = bp;
> -	bip->bli_format.blf_type = XFS_LI_BUF;
> -	bip->bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
> -	bip->bli_format.blf_len = (unsigned short)BTOBB(bp->b_bcount);
> +	bip->__bli_format.blf_type = XFS_LI_BUF;
> +	bip->__bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
> +	bip->__bli_format.blf_len = (unsigned short)BTOBB(bp->b_bcount);
>   	bp->b_log_item = bip;

I had a look around this area of code, and I see where the bli_format is 
getting referenced, but I don't see a bli_formats.  So I feel like I'm 
missing the motivation for the change.  Did I miss the bli_formats 
somewhere?  Thanks!

Allison

>   }
>   
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index 6967a1de..f3c28fa7 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -531,8 +531,8 @@ xfs_trans_binval(
>   	xfs_buf_stale(bp);
>   	bip->bli_flags |= XFS_BLI_STALE;
>   	bip->bli_flags &= ~XFS_BLI_DIRTY;
> -	bip->bli_format.blf_flags &= ~XFS_BLF_INODE_BUF;
> -	bip->bli_format.blf_flags |= XFS_BLF_CANCEL;
> +	bip->__bli_format.blf_flags &= ~XFS_BLF_INODE_BUF;
> +	bip->__bli_format.blf_flags |= XFS_BLF_CANCEL;
>   	set_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags);
>   	tp->t_flags |= XFS_TRANS_DIRTY;
>   }
> 

Re: [PATCH 6/3] libxfs: factor common xfs_trans_bjoin code

[Allison Collins]

On 5/16/19 1:39 PM, Eric Sandeen wrote:
> Most of xfs_trans_bjoin is duplicated in xfs_trans_get_buf_map,
> xfs_trans_getsb and xfs_trans_read_buf_map.  Add a new
> _xfs_trans_bjoin which can be called by all three functions.
> 
> Source kernel commit: d7e84f413726876c0ec66bbf90770f69841f7663
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Dave Chinner <david@fromorbit.com>
> Signed-off-by: Alex Elder <aelder@sgi.com>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks ok to me
Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> ---
>   libxfs/trans.c | 53 +++++++++++++++++++++++++++++++++++++-------------
>   1 file changed, 39 insertions(+), 14 deletions(-)
> 
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index f3c28fa7..f78222fd 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -537,19 +537,50 @@ xfs_trans_binval(
>   	tp->t_flags |= XFS_TRANS_DIRTY;
>   }
>   
> -void
> -xfs_trans_bjoin(
> -	xfs_trans_t		*tp,
> -	xfs_buf_t		*bp)
> +/*
> + * Add the locked buffer to the transaction.
> + *
> + * The buffer must be locked, and it cannot be associated with any
> + * transaction.
> + *
> + * If the buffer does not yet have a buf log item associated with it,
> + * then allocate one for it.  Then add the buf item to the transaction.
> + */
> +STATIC void
> +_xfs_trans_bjoin(
> +	struct xfs_trans	*tp,
> +	struct xfs_buf		*bp,
> +	int			reset_recur)
>   {
> -	xfs_buf_log_item_t	*bip;
> +	struct xfs_buf_log_item	*bip;
>   
>   	ASSERT(bp->b_transp == NULL);
>   
> +        /*
> +	 * The xfs_buf_log_item pointer is stored in b_log_item.  If
> +	 * it doesn't have one yet, then allocate one and initialize it.
> +	 * The checks to see if one is there are in xfs_buf_item_init().
> +	 */
>   	xfs_buf_item_init(bp, tp->t_mountp);
>   	bip = bp->b_log_item;
> +	if (reset_recur)
> +		bip->bli_recur = 0;
> +
> +	/*
> +	 * Attach the item to the transaction so we can find it in
> +	 * xfs_trans_get_buf() and friends.
> +	 */
>   	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
>   	bp->b_transp = tp;
> +
> +}
> +
> +void
> +xfs_trans_bjoin(
> +	struct xfs_trans	*tp,
> +	struct xfs_buf		*bp)
> +{
> +	_xfs_trans_bjoin(tp, bp, 0);
>   	trace_xfs_trans_bjoin(bp->b_log_item);
>   }
>   
> @@ -594,9 +625,7 @@ xfs_trans_get_buf_map(
>   	if (bp == NULL)
>   		return NULL;
>   
> -	xfs_trans_bjoin(tp, bp);
> -	bip = bp->b_log_item;
> -	bip->bli_recur = 0;
> +	_xfs_trans_bjoin(tp, bp, 1);
>   	trace_xfs_trans_get_buf(bp->b_log_item);
>   	return bp;
>   }
> @@ -626,9 +655,7 @@ xfs_trans_getsb(
>   
>   	bp = xfs_getsb(mp);
>   
> -	xfs_trans_bjoin(tp, bp);
> -	bip = bp->b_log_item;
> -	bip->bli_recur = 0;
> +	_xfs_trans_bjoin(tp, bp, 1);
>   	trace_xfs_trans_getsb(bp->b_log_item);
>   	return bp;
>   }
> @@ -677,9 +704,7 @@ xfs_trans_read_buf_map(
>   	if (bp->b_error)
>   		goto out_relse;
>   
> -	xfs_trans_bjoin(tp, bp);
> -	bip = bp->b_log_item;
> -	bip->bli_recur = 0;
> +	_xfs_trans_bjoin(tp, bp, 1);
>   done:
>   	trace_xfs_trans_read_buf(bp->b_log_item);
>   	*bpp = bp;
> 

Re: [PATCH 7/3] libxfs: fix argument to xfs_trans_add_item

[Allison Collins]

On 5/16/19 1:40 PM, Eric Sandeen wrote:
> The hack of casting an inode_log_item or buf_log_item to a
> xfs_log_item_t is pretty gross; yes it's the first member in the
> structure, but yuk.  Pass in the correct structure member.
> 
> This was fixed in the kernel with commit e98c414f9
> ("xfs: simplify log item descriptor tracking")
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good.
Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> ---
>   libxfs/trans.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index f78222fd..6ef4841f 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -346,7 +346,7 @@ xfs_trans_ijoin(
>   	ASSERT(iip->ili_lock_flags == 0);
>   	iip->ili_lock_flags = lock_flags;
>   
> -	xfs_trans_add_item(tp, (xfs_log_item_t *)(iip));
> +	xfs_trans_add_item(tp, &iip->ili_item);
>   }
>   
>   void
> @@ -570,7 +570,7 @@ _xfs_trans_bjoin(
>   	 * Attach the item to the transaction so we can find it in
>   	 * xfs_trans_get_buf() and friends.
>   	 */
> -	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
> +	xfs_trans_add_item(tp, &bip->bli_item);
>   	bp->b_transp = tp;
>   
>   }
> 

Re: [PATCH 5/3] libxfs: rename bli_format to avoid confusion with bli_formats

[Eric Sandeen]

On 5/17/19 5:29 PM, Allison Collins wrote:
> On 5/16/19 1:39 PM, Eric Sandeen wrote:
>> Rename the bli_format structure to __bli_format to avoid
>> accidently confusing them with the bli_formats pointer.
>>
>> (nb: userspace currently has no bli_formats pointer)
>>
>> Source kernel commit: b94381737e9c4d014a4003e8ece9ba88670a2dd4
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>   include/xfs_trans.h | 2 +-
>>   libxfs/logitem.c    | 6 +++---
>>   libxfs/trans.c      | 4 ++--
>>   3 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/xfs_trans.h b/include/xfs_trans.h
>> index 953da5d1..fe03ba64 100644
>> --- a/include/xfs_trans.h
>> +++ b/include/xfs_trans.h
>> @@ -39,7 +39,7 @@ typedef struct xfs_buf_log_item {
>>       struct xfs_buf        *bli_buf;    /* real buffer pointer */
>>       unsigned int        bli_flags;    /* misc flags */
>>       unsigned int        bli_recur;    /* recursion count */
>> -    xfs_buf_log_format_t    bli_format;    /* in-log header */
>> +    xfs_buf_log_format_t    __bli_format;    /* in-log header */
>>   } xfs_buf_log_item_t;
>>     #define XFS_BLI_DIRTY            (1<<0)
>> diff --git a/libxfs/logitem.c b/libxfs/logitem.c
>> index 4da9bc1b..e862ab4f 100644
>> --- a/libxfs/logitem.c
>> +++ b/libxfs/logitem.c
>> @@ -107,9 +107,9 @@ xfs_buf_item_init(
>>       bip->bli_item.li_mountp = mp;
>>       INIT_LIST_HEAD(&bip->bli_item.li_trans);
>>       bip->bli_buf = bp;
>> -    bip->bli_format.blf_type = XFS_LI_BUF;
>> -    bip->bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
>> -    bip->bli_format.blf_len = (unsigned short)BTOBB(bp->b_bcount);
>> +    bip->__bli_format.blf_type = XFS_LI_BUF;
>> +    bip->__bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
>> +    bip->__bli_format.blf_len = (unsigned short)BTOBB(bp->b_bcount);
>>       bp->b_log_item = bip;
> 
> I had a look around this area of code, and I see where the bli_format is getting referenced, but I don't see a bli_formats.  So I feel like I'm missing the motivation for the change.  Did I miss the bli_formats somewhere?  Thanks!

see above :)

> (nb: userspace currently has no bli_formats pointer) 

(I guess copying the kernel commit log added confusion even w/ the note)

-Eric

Re: [PATCH 5/3] libxfs: rename bli_format to avoid confusion with bli_formats

[Allison Collins]

On 5/17/19 4:01 PM, Eric Sandeen wrote:
> On 5/17/19 5:29 PM, Allison Collins wrote:
>> On 5/16/19 1:39 PM, Eric Sandeen wrote:
>>> Rename the bli_format structure to __bli_format to avoid
>>> accidently confusing them with the bli_formats pointer.
>>>
>>> (nb: userspace currently has no bli_formats pointer)
>>>
>>> Source kernel commit: b94381737e9c4d014a4003e8ece9ba88670a2dd4
>>>
>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>> ---
>>>    include/xfs_trans.h | 2 +-
>>>    libxfs/logitem.c    | 6 +++---
>>>    libxfs/trans.c      | 4 ++--
>>>    3 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/include/xfs_trans.h b/include/xfs_trans.h
>>> index 953da5d1..fe03ba64 100644
>>> --- a/include/xfs_trans.h
>>> +++ b/include/xfs_trans.h
>>> @@ -39,7 +39,7 @@ typedef struct xfs_buf_log_item {
>>>        struct xfs_buf        *bli_buf;    /* real buffer pointer */
>>>        unsigned int        bli_flags;    /* misc flags */
>>>        unsigned int        bli_recur;    /* recursion count */
>>> -    xfs_buf_log_format_t    bli_format;    /* in-log header */
>>> +    xfs_buf_log_format_t    __bli_format;    /* in-log header */
>>>    } xfs_buf_log_item_t;
>>>      #define XFS_BLI_DIRTY            (1<<0)
>>> diff --git a/libxfs/logitem.c b/libxfs/logitem.c
>>> index 4da9bc1b..e862ab4f 100644
>>> --- a/libxfs/logitem.c
>>> +++ b/libxfs/logitem.c
>>> @@ -107,9 +107,9 @@ xfs_buf_item_init(
>>>        bip->bli_item.li_mountp = mp;
>>>        INIT_LIST_HEAD(&bip->bli_item.li_trans);
>>>        bip->bli_buf = bp;
>>> -    bip->bli_format.blf_type = XFS_LI_BUF;
>>> -    bip->bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
>>> -    bip->bli_format.blf_len = (unsigned short)BTOBB(bp->b_bcount);
>>> +    bip->__bli_format.blf_type = XFS_LI_BUF;
>>> +    bip->__bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
>>> +    bip->__bli_format.blf_len = (unsigned short)BTOBB(bp->b_bcount);
>>>        bp->b_log_item = bip;
>>
>> I had a look around this area of code, and I see where the bli_format is getting referenced, but I don't see a bli_formats.  So I feel like I'm missing the motivation for the change.  Did I miss the bli_formats somewhere?  Thanks!
> 
> see above :)
> 
>> (nb: userspace currently has no bli_formats pointer)
> 
> (I guess copying the kernel commit log added confusion even w/ the note)
> 
> -Eric

Oh I see.  No I think it's ok, I overlooked it.  You can add my review  :-)

Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> 

Re: [PATCH 8/3] xfs: factor log item initialisation

[Allison Collins]

On 5/16/19 1:41 PM, Eric Sandeen wrote:
> Each log item type does manual initialisation of the log item.
> Delayed logging introduces new fields that need initialisation, so
> factor all the open coded initialisation into a common function
> first.
> 
> Source kernel commit: 43f5efc5b59db1b66e39fe9fdfc4ba6a27152afa
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good to me.
Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> ---
>   libxfs/libxfs_priv.h |  1 +
>   libxfs/logitem.c     |  8 ++------
>   libxfs/util.c        | 12 ++++++++++++
>   3 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
> index 157a99d6..7551ed65 100644
> --- a/libxfs/libxfs_priv.h
> +++ b/libxfs/libxfs_priv.h
> @@ -564,6 +564,7 @@ int xfs_zero_extent(struct xfs_inode *ip, xfs_fsblock_t start_fsb,
>   
>   
>   bool xfs_log_check_lsn(struct xfs_mount *, xfs_lsn_t);
> +void xfs_log_item_init(struct xfs_mount *, struct xfs_log_item *, int);
>   #define xfs_log_in_recovery(mp)	(false)
>   
>   /* xfs_icache.c */
> diff --git a/libxfs/logitem.c b/libxfs/logitem.c
> index e862ab4f..14c62f67 100644
> --- a/libxfs/logitem.c
> +++ b/libxfs/logitem.c
> @@ -103,9 +103,7 @@ xfs_buf_item_init(
>   	fprintf(stderr, "adding buf item %p for not-logged buffer %p\n",
>   		bip, bp);
>   #endif
> -	bip->bli_item.li_type = XFS_LI_BUF;
> -	bip->bli_item.li_mountp = mp;
> -	INIT_LIST_HEAD(&bip->bli_item.li_trans);
> +	xfs_log_item_init(mp, &bip->bli_item, XFS_LI_BUF);
>   	bip->bli_buf = bp;
>   	bip->__bli_format.blf_type = XFS_LI_BUF;
>   	bip->__bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
> @@ -149,8 +147,6 @@ xfs_inode_item_init(
>   		ip->i_ino, iip);
>   #endif
>   
> -	iip->ili_item.li_type = XFS_LI_INODE;
> -	iip->ili_item.li_mountp = mp;
> -	INIT_LIST_HEAD(&iip->ili_item.li_trans);
> +	xfs_log_item_init(mp, &iip->ili_item, XFS_LI_INODE);
>   	iip->ili_inode = ip;
>   }
> diff --git a/libxfs/util.c b/libxfs/util.c
> index 4901123a..aff91080 100644
> --- a/libxfs/util.c
> +++ b/libxfs/util.c
> @@ -691,6 +691,18 @@ xfs_log_check_lsn(
>   	return true;
>   }
>   
> +void
> +xfs_log_item_init(
> +	struct xfs_mount	*mp,
> +	struct xfs_log_item	*item,
> +	int			type)
> +{
> +	item->li_mountp = mp;
> +	item->li_type = type;
> +
> +	INIT_LIST_HEAD(&item->li_trans);
> +}
> +
>   static struct xfs_buftarg *
>   xfs_find_bdev_for_inode(
>   	struct xfs_inode	*ip)
> 

Re: [PATCH 4/3] libxfs: Remove XACT_DEBUG #ifdefs

[Darrick J. Wong]

On Fri, May 17, 2019 at 02:36:49PM -0700, Allison Collins wrote:
> On 5/16/19 1:38 PM, Eric Sandeen wrote:
> > Remove XACT_DEBUG #ifdefs to reduce more cosmetic differences
> > between userspace & kernelspace libxfs.  Add in some corresponding
> > (stubbed-out) tracepoint calls.
> > 
> > If these are felt to be particularly useful, the tracepoint calls
> > could be fleshed out to provide similar information.

I've occasionally wondered this myself.

> > Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> 
> Looks ok.  It seems a shame to loose the extra tracing, but I confess most
> of the time when I'm chasing a particular bug I end up adding my own
> temporary traces anyway.  So I think trying to synchronize the code spaces
> is more important.  Do you think the stubs might be something to add to the
> kernel space side too?

I think most of them already exist in the kernel...

--D

> Allison
> 
> > ---
> >   include/xfs_trace.h | 13 +++++++
> >   libxfs/trans.c      | 85 ++++++++++++---------------------------------
> >   2 files changed, 35 insertions(+), 63 deletions(-)
> > 
> > diff --git a/include/xfs_trace.h b/include/xfs_trace.h
> > index 793ac56e..43720040 100644
> > --- a/include/xfs_trace.h
> > +++ b/include/xfs_trace.h
> > @@ -161,6 +161,19 @@
> >   #define trace_xfs_perag_get_tag(a,b,c,d) ((c) = (c))
> >   #define trace_xfs_perag_put(a,b,c,d)	((c) = (c))
> > +#define trace_xfs_trans_alloc(a,b)		((void) 0)
> > +#define trace_xfs_trans_cancel(a,b)		((void) 0)
> > +#define trace_xfs_trans_brelse(a)		((void) 0)
> > +#define trace_xfs_trans_binval(a)		((void) 0)
> > +#define trace_xfs_trans_bjoin(a)		((void) 0)
> > +#define trace_xfs_trans_bhold(a)		((void) 0)
> > +#define trace_xfs_trans_get_buf(a)		((void) 0)
> > +#define trace_xfs_trans_getsb_recur(a)		((void) 0)
> > +#define trace_xfs_trans_getsb(a)		((void) 0)
> > +#define trace_xfs_trans_read_buf_recur(a)	((void) 0)
> > +#define trace_xfs_trans_read_buf(a)		((void) 0)
> > +#define trace_xfs_trans_commit(a,b)		((void) 0)
> > +
> >   #define trace_xfs_defer_cancel(a,b)		((void) 0)
> >   #define trace_xfs_defer_pending_commit(a,b)	((void) 0)
> >   #define trace_xfs_defer_pending_abort(a,b)	((void) 0)
> > diff --git a/libxfs/trans.c b/libxfs/trans.c
> > index 9e49def0..6967a1de 100644
> > --- a/libxfs/trans.c
> > +++ b/libxfs/trans.c
> > @@ -18,6 +18,7 @@
> >   #include "xfs_trans.h"
> >   #include "xfs_sb.h"
> >   #include "xfs_defer.h"
> > +#include "xfs_trace.h"
> >   static void xfs_trans_free_items(struct xfs_trans *tp);
> >   STATIC struct xfs_trans *xfs_trans_dup(struct xfs_trans *tp);
> > @@ -269,9 +270,9 @@ xfs_trans_alloc(
> >   		xfs_trans_cancel(tp);
> >   		return error;
> >   	}
> > -#ifdef XACT_DEBUG
> > -	fprintf(stderr, "allocated new transaction %p\n", tp);
> > -#endif
> > +
> > +	trace_xfs_trans_alloc(tp, _RET_IP_);
> > +
> >   	*tpp = tp;
> >   	return 0;
> >   }
> > @@ -317,23 +318,16 @@ void
> >   xfs_trans_cancel(
> >   	struct xfs_trans	*tp)
> >   {
> > -#ifdef XACT_DEBUG
> > -	struct xfs_trans	*otp = tp;
> > -#endif
> > +	trace_xfs_trans_cancel(tp, _RET_IP_);
> > +
> >   	if (tp == NULL)
> > -		goto out;
> > +		return;
> >   	if (tp->t_flags & XFS_TRANS_PERM_LOG_RES)
> >   		xfs_defer_cancel(tp);
> >   	xfs_trans_free_items(tp);
> >   	xfs_trans_free(tp);
> > -
> > -out:
> > -#ifdef XACT_DEBUG
> > -	fprintf(stderr, "## cancelled transaction %p\n", otp);
> > -#endif
> > -	return;
> >   }
> >   void
> > @@ -353,10 +347,6 @@ xfs_trans_ijoin(
> >   	iip->ili_lock_flags = lock_flags;
> >   	xfs_trans_add_item(tp, (xfs_log_item_t *)(iip));
> > -
> > -#ifdef XACT_DEBUG
> > -	fprintf(stderr, "ijoin'd inode %llu, transaction %p\n", ip->i_ino, tp);
> > -#endif
> >   }
> >   void
> > @@ -388,9 +378,6 @@ xfs_trans_log_inode(
> >   	uint			flags)
> >   {
> >   	ASSERT(ip->i_itemp != NULL);
> > -#ifdef XACT_DEBUG
> > -	fprintf(stderr, "dirtied inode %llu, transaction %p\n", ip->i_ino, tp);
> > -#endif
> >   	tp->t_flags |= XFS_TRANS_DIRTY;
> >   	set_bit(XFS_LI_DIRTY, &ip->i_itemp->ili_item.li_flags);
> > @@ -434,9 +421,6 @@ xfs_trans_dirty_buf(
> >   	ASSERT(bp->b_transp == tp);
> >   	ASSERT(bip != NULL);
> > -#ifdef XACT_DEBUG
> > -	fprintf(stderr, "dirtied buffer %p, transaction %p\n", bp, tp);
> > -#endif
> >   	tp->t_flags |= XFS_TRANS_DIRTY;
> >   	set_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags);
> >   }
> > @@ -501,15 +485,14 @@ xfs_trans_brelse(
> >   	xfs_buf_t		*bp)
> >   {
> >   	xfs_buf_log_item_t	*bip;
> > -#ifdef XACT_DEBUG
> > -	fprintf(stderr, "released buffer %p, transaction %p\n", bp, tp);
> > -#endif
> >   	if (tp == NULL) {
> >   		ASSERT(bp->b_transp == NULL);
> >   		libxfs_putbuf(bp);
> >   		return;
> >   	}
> > +
> > +	trace_xfs_trans_brelse(bip);
> >   	ASSERT(bp->b_transp == tp);
> >   	bip = bp->b_log_item;
> >   	ASSERT(bip->bli_item.li_type == XFS_LI_BUF);
> > @@ -536,13 +519,12 @@ xfs_trans_binval(
> >   	xfs_buf_t		*bp)
> >   {
> >   	xfs_buf_log_item_t	*bip = bp->b_log_item;
> > -#ifdef XACT_DEBUG
> > -	fprintf(stderr, "binval'd buffer %p, transaction %p\n", bp, tp);
> > -#endif
> >   	ASSERT(bp->b_transp == tp);
> >   	ASSERT(bip != NULL);
> > +	trace_xfs_trans_binval(bip);
> > +
> >   	if (bip->bli_flags & XFS_BLI_STALE)
> >   		return;
> >   	XFS_BUF_UNDELAYWRITE(bp);
> > @@ -563,14 +545,12 @@ xfs_trans_bjoin(
> >   	xfs_buf_log_item_t	*bip;
> >   	ASSERT(bp->b_transp == NULL);
> > -#ifdef XACT_DEBUG
> > -	fprintf(stderr, "bjoin'd buffer %p, transaction %p\n", bp, tp);
> > -#endif
> >   	xfs_buf_item_init(bp, tp->t_mountp);
> >   	bip = bp->b_log_item;
> >   	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
> >   	bp->b_transp = tp;
> > +	trace_xfs_trans_bjoin(bp->b_log_item);
> >   }
> >   void
> > @@ -582,11 +562,9 @@ xfs_trans_bhold(
> >   	ASSERT(bp->b_transp == tp);
> >   	ASSERT(bip != NULL);
> > -#ifdef XACT_DEBUG
> > -	fprintf(stderr, "bhold'd buffer %p, transaction %p\n", bp, tp);
> > -#endif
> >   	bip->bli_flags |= XFS_BLI_HOLD;
> > +	trace_xfs_trans_bhold(bip);
> >   }
> >   xfs_buf_t *
> > @@ -615,13 +593,11 @@ xfs_trans_get_buf_map(
> >   	bp = libxfs_getbuf_map(btp, map, nmaps, 0);
> >   	if (bp == NULL)
> >   		return NULL;
> > -#ifdef XACT_DEBUG
> > -	fprintf(stderr, "trans_get_buf buffer %p, transaction %p\n", bp, tp);
> > -#endif
> >   	xfs_trans_bjoin(tp, bp);
> >   	bip = bp->b_log_item;
> >   	bip->bli_recur = 0;
> > +	trace_xfs_trans_get_buf(bp->b_log_item);
> >   	return bp;
> >   }
> > @@ -644,17 +620,16 @@ xfs_trans_getsb(
> >   		bip = bp->b_log_item;
> >   		ASSERT(bip != NULL);
> >   		bip->bli_recur++;
> > +		trace_xfs_trans_getsb_recur(bip);
> >   		return bp;
> >   	}
> >   	bp = xfs_getsb(mp);
> > -#ifdef XACT_DEBUG
> > -	fprintf(stderr, "trans_get_sb buffer %p, transaction %p\n", bp, tp);
> > -#endif
> >   	xfs_trans_bjoin(tp, bp);
> >   	bip = bp->b_log_item;
> >   	bip->bli_recur = 0;
> > +	trace_xfs_trans_getsb(bp->b_log_item);
> >   	return bp;
> >   }
> > @@ -691,6 +666,7 @@ xfs_trans_read_buf_map(
> >   		ASSERT(bp->b_log_item != NULL);
> >   		bip = bp->b_log_item;
> >   		bip->bli_recur++;
> > +		trace_xfs_trans_read_buf_recur(bip);
> >   		goto done;
> >   	}
> > @@ -701,14 +677,11 @@ xfs_trans_read_buf_map(
> >   	if (bp->b_error)
> >   		goto out_relse;
> > -#ifdef XACT_DEBUG
> > -	fprintf(stderr, "trans_read_buf buffer %p, transaction %p\n", bp, tp);
> > -#endif
> > -
> >   	xfs_trans_bjoin(tp, bp);
> >   	bip = bp->b_log_item;
> >   	bip->bli_recur = 0;
> >   done:
> > +	trace_xfs_trans_read_buf(bp->b_log_item);
> >   	*bpp = bp;
> >   	return 0;
> >   out_relse:
> > @@ -824,10 +797,6 @@ inode_item_done(
> >   	}
> >   	libxfs_writebuf(bp, 0);
> > -#ifdef XACT_DEBUG
> > -	fprintf(stderr, "flushing dirty inode %llu, buffer %p\n",
> > -			ip->i_ino, bp);
> > -#endif
> >   free:
> >   	xfs_inode_item_put(iip);
> >   }
> > @@ -845,13 +814,8 @@ buf_item_done(
> >   	bp->b_transp = NULL;			/* remove xact ptr */
> >   	hold = (bip->bli_flags & XFS_BLI_HOLD);
> > -	if (bip->bli_flags & XFS_BLI_DIRTY) {
> > -#ifdef XACT_DEBUG
> > -		fprintf(stderr, "flushing/staling buffer %p (hold=%d)\n",
> > -			bp, hold);
> > -#endif
> > +	if (bip->bli_flags & XFS_BLI_DIRTY)
> >   		libxfs_writebuf_int(bp, 0);
> > -	}
> >   	bip->bli_flags &= ~XFS_BLI_HOLD;
> >   	xfs_buf_item_put(bip);
> > @@ -937,6 +901,8 @@ __xfs_trans_commit(
> >   	struct xfs_sb		*sbp;
> >   	int			error = 0;
> > +	trace_xfs_trans_commit(tp, _RET_IP_);
> > +
> >   	if (tp == NULL)
> >   		return 0;
> > @@ -952,12 +918,8 @@ __xfs_trans_commit(
> >   			goto out_unreserve;
> >   	}
> > -	if (!(tp->t_flags & XFS_TRANS_DIRTY)) {
> > -#ifdef XACT_DEBUG
> > -		fprintf(stderr, "committed clean transaction %p\n", tp);
> > -#endif
> > +	if (!(tp->t_flags & XFS_TRANS_DIRTY))
> >   		goto out_unreserve;
> > -	}
> >   	if (tp->t_flags & XFS_TRANS_SB_DIRTY) {
> >   		sbp = &(tp->t_mountp->m_sb);
> > @@ -972,9 +934,6 @@ __xfs_trans_commit(
> >   		xfs_log_sb(tp);
> >   	}
> > -#ifdef XACT_DEBUG
> > -	fprintf(stderr, "committing dirty transaction %p\n", tp);
> > -#endif
> >   	trans_committed(tp);
> >   	/* That's it for the transaction structure.  Free it. */
> > 

Re: [PATCH 6/3] libxfs: factor common xfs_trans_bjoin code

[Darrick J. Wong]

On Thu, May 16, 2019 at 03:39:49PM -0500, Eric Sandeen wrote:
> Most of xfs_trans_bjoin is duplicated in xfs_trans_get_buf_map,
> xfs_trans_getsb and xfs_trans_read_buf_map.  Add a new
> _xfs_trans_bjoin which can be called by all three functions.
> 
> Source kernel commit: d7e84f413726876c0ec66bbf90770f69841f7663
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Dave Chinner <david@fromorbit.com>
> Signed-off-by: Alex Elder <aelder@sgi.com>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>  libxfs/trans.c | 53 +++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 39 insertions(+), 14 deletions(-)
> 
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index f3c28fa7..f78222fd 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -537,19 +537,50 @@ xfs_trans_binval(
>  	tp->t_flags |= XFS_TRANS_DIRTY;
>  }
>  
> -void
> -xfs_trans_bjoin(
> -	xfs_trans_t		*tp,
> -	xfs_buf_t		*bp)
> +/*
> + * Add the locked buffer to the transaction.
> + *
> + * The buffer must be locked, and it cannot be associated with any
> + * transaction.
> + *
> + * If the buffer does not yet have a buf log item associated with it,
> + * then allocate one for it.  Then add the buf item to the transaction.
> + */
> +STATIC void
> +_xfs_trans_bjoin(
> +	struct xfs_trans	*tp,
> +	struct xfs_buf		*bp,
> +	int			reset_recur)

bool?

>  {
> -	xfs_buf_log_item_t	*bip;
> +	struct xfs_buf_log_item	*bip;
>  
>  	ASSERT(bp->b_transp == NULL);
>  
> +        /*
> +	 * The xfs_buf_log_item pointer is stored in b_log_item.  If
> +	 * it doesn't have one yet, then allocate one and initialize it.
> +	 * The checks to see if one is there are in xfs_buf_item_init().
> +	 */
>  	xfs_buf_item_init(bp, tp->t_mountp);
>  	bip = bp->b_log_item;
> +	if (reset_recur)
> +		bip->bli_recur = 0;
> +
> +	/*
> +	 * Attach the item to the transaction so we can find it in
> +	 * xfs_trans_get_buf() and friends.
> +	 */
>  	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);

Kill typedef here               ^^^^^^?

--D

>  	bp->b_transp = tp;
> +
> +}
> +
> +void
> +xfs_trans_bjoin(
> +	struct xfs_trans	*tp,
> +	struct xfs_buf		*bp)
> +{
> +	_xfs_trans_bjoin(tp, bp, 0);
>  	trace_xfs_trans_bjoin(bp->b_log_item);
>  }
>  
> @@ -594,9 +625,7 @@ xfs_trans_get_buf_map(
>  	if (bp == NULL)
>  		return NULL;
>  
> -	xfs_trans_bjoin(tp, bp);
> -	bip = bp->b_log_item;
> -	bip->bli_recur = 0;
> +	_xfs_trans_bjoin(tp, bp, 1);
>  	trace_xfs_trans_get_buf(bp->b_log_item);
>  	return bp;
>  }
> @@ -626,9 +655,7 @@ xfs_trans_getsb(
>  
>  	bp = xfs_getsb(mp);
>  
> -	xfs_trans_bjoin(tp, bp);
> -	bip = bp->b_log_item;
> -	bip->bli_recur = 0;
> +	_xfs_trans_bjoin(tp, bp, 1);
>  	trace_xfs_trans_getsb(bp->b_log_item);
>  	return bp;
>  }
> @@ -677,9 +704,7 @@ xfs_trans_read_buf_map(
>  	if (bp->b_error)
>  		goto out_relse;
>  
> -	xfs_trans_bjoin(tp, bp);
> -	bip = bp->b_log_item;
> -	bip->bli_recur = 0;
> +	_xfs_trans_bjoin(tp, bp, 1);
>  done:
>  	trace_xfs_trans_read_buf(bp->b_log_item);
>  	*bpp = bp;
> -- 
> 2.17.0
> 

Re: [PATCH 6/3] libxfs: factor common xfs_trans_bjoin code

[Eric Sandeen]

On 5/20/19 5:56 PM, Darrick J. Wong wrote:
> On Thu, May 16, 2019 at 03:39:49PM -0500, Eric Sandeen wrote:
>> Most of xfs_trans_bjoin is duplicated in xfs_trans_get_buf_map,
>> xfs_trans_getsb and xfs_trans_read_buf_map.  Add a new
>> _xfs_trans_bjoin which can be called by all three functions.
>>
>> Source kernel commit: d7e84f413726876c0ec66bbf90770f69841f7663
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> Reviewed-by: Dave Chinner <david@fromorbit.com>
>> Signed-off-by: Alex Elder <aelder@sgi.com>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>  libxfs/trans.c | 53 +++++++++++++++++++++++++++++++++++++-------------
>>  1 file changed, 39 insertions(+), 14 deletions(-)
>>
>> diff --git a/libxfs/trans.c b/libxfs/trans.c
>> index f3c28fa7..f78222fd 100644
>> --- a/libxfs/trans.c
>> +++ b/libxfs/trans.c
>> @@ -537,19 +537,50 @@ xfs_trans_binval(
>>  	tp->t_flags |= XFS_TRANS_DIRTY;
>>  }
>>  
>> -void
>> -xfs_trans_bjoin(
>> -	xfs_trans_t		*tp,
>> -	xfs_buf_t		*bp)
>> +/*
>> + * Add the locked buffer to the transaction.
>> + *
>> + * The buffer must be locked, and it cannot be associated with any
>> + * transaction.
>> + *
>> + * If the buffer does not yet have a buf log item associated with it,
>> + * then allocate one for it.  Then add the buf item to the transaction.
>> + */
>> +STATIC void
>> +_xfs_trans_bjoin(
>> +	struct xfs_trans	*tp,
>> +	struct xfs_buf		*bp,
>> +	int			reset_recur)
> 
> bool?

If you fix it in the kernel I'll merge it to xfsprogs ;)

>>  {
>> -	xfs_buf_log_item_t	*bip;
>> +	struct xfs_buf_log_item	*bip;
>>  
>>  	ASSERT(bp->b_transp == NULL);
>>  
>> +        /*
>> +	 * The xfs_buf_log_item pointer is stored in b_log_item.  If
>> +	 * it doesn't have one yet, then allocate one and initialize it.
>> +	 * The checks to see if one is there are in xfs_buf_item_init().
>> +	 */
>>  	xfs_buf_item_init(bp, tp->t_mountp);
>>  	bip = bp->b_log_item;
>> +	if (reset_recur)
>> +		bip->bli_recur = 0;
>> +
>> +	/*
>> +	 * Attach the item to the transaction so we can find it in
>> +	 * xfs_trans_get_buf() and friends.
>> +	 */
>>  	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
> 
> Kill typedef here               ^^^^^^?

ditto.

Point of all this is to match the kernel for easier future syncups.

(oh wait, actually, this is fixed in a later patch, getting rid of the dumb
cast altogether)

-Eric

> --D
> 
>>  	bp->b_transp = tp;
>> +
>> +}
>> +
>> +void
>> +xfs_trans_bjoin(
>> +	struct xfs_trans	*tp,
>> +	struct xfs_buf		*bp)
>> +{
>> +	_xfs_trans_bjoin(tp, bp, 0);
>>  	trace_xfs_trans_bjoin(bp->b_log_item);
>>  }
>>  
>> @@ -594,9 +625,7 @@ xfs_trans_get_buf_map(
>>  	if (bp == NULL)
>>  		return NULL;
>>  
>> -	xfs_trans_bjoin(tp, bp);
>> -	bip = bp->b_log_item;
>> -	bip->bli_recur = 0;
>> +	_xfs_trans_bjoin(tp, bp, 1);
>>  	trace_xfs_trans_get_buf(bp->b_log_item);
>>  	return bp;
>>  }
>> @@ -626,9 +655,7 @@ xfs_trans_getsb(
>>  
>>  	bp = xfs_getsb(mp);
>>  
>> -	xfs_trans_bjoin(tp, bp);
>> -	bip = bp->b_log_item;
>> -	bip->bli_recur = 0;
>> +	_xfs_trans_bjoin(tp, bp, 1);
>>  	trace_xfs_trans_getsb(bp->b_log_item);
>>  	return bp;
>>  }
>> @@ -677,9 +704,7 @@ xfs_trans_read_buf_map(
>>  	if (bp->b_error)
>>  		goto out_relse;
>>  
>> -	xfs_trans_bjoin(tp, bp);
>> -	bip = bp->b_log_item;
>> -	bip->bli_recur = 0;
>> +	_xfs_trans_bjoin(tp, bp, 1);
>>  done:
>>  	trace_xfs_trans_read_buf(bp->b_log_item);
>>  	*bpp = bp;
>> -- 
>> 2.17.0
>>

Re: [PATCH 10/3] libxfs: share kernel's xfs_trans_inode.c

[Darrick J. Wong]

On Fri, May 17, 2019 at 02:50:15PM -0500, Eric Sandeen wrote:
> Now that the majority of cosmetic changes and compat shims
> are in place, we can directly share kernelspace's
> xfs_trans_inode.c with just a couple more small tweaks.
> In addition to the file move,
> 
> * ili_fsync_fields is added to xfs_inode_log_item (but not used)
> * test_and_set_bit() helper is created
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> To finish the sync, we probably need to move the file under
> libxfs/ in kernelspace, and add XFS_ICHGTIME_CREATE handling
> unless that's deemed undesirable, in which case we can probably
> just carry the delta in userspace...

I already did that as part of the metadir feature series I sent last
December... :)

Nevertheless, I eventually need xfs_trans_ichgtime to accept all four of
the MOD/CHG/CREATE/ACCESS flags, so this is fine.

--D

> diff --git a/include/xfs_trans.h b/include/xfs_trans.h
> index fe03ba64..cdaa69d8 100644
> --- a/include/xfs_trans.h
> +++ b/include/xfs_trans.h
> @@ -30,8 +30,9 @@ typedef struct xfs_inode_log_item {
>  	xfs_log_item_t		ili_item;		/* common portion */
>  	struct xfs_inode	*ili_inode;		/* inode pointer */
>  	unsigned short		ili_lock_flags;		/* lock flags */
> -	unsigned int		ili_fields;		/* fields to be logged */
>  	unsigned int		ili_last_fields;	/* fields when flushed*/
> +	unsigned int		ili_fields;		/* fields to be logged */
> +	unsigned int		ili_fsync_fields;	/* ignored by userspace */
>  } xfs_inode_log_item_t;
>  
>  typedef struct xfs_buf_log_item {
> diff --git a/libxfs/Makefile b/libxfs/Makefile
> index 160498d7..8c681e0b 100644
> --- a/libxfs/Makefile
> +++ b/libxfs/Makefile
> @@ -93,6 +93,7 @@ CFILES = cache.c \
>  	xfs_rtbitmap.c \
>  	xfs_sb.c \
>  	xfs_symlink_remote.c \
> +	xfs_trans_inode.c \
>  	xfs_trans_resv.c \
>  	xfs_types.c
>  
> diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
> index 7551ed65..fc785664 100644
> --- a/libxfs/libxfs_priv.h
> +++ b/libxfs/libxfs_priv.h
> @@ -608,6 +608,15 @@ static inline int test_bit(int nr, const volatile unsigned long *addr)
>  	return *p & mask;
>  }
>  
> +/* Sets and returns original value of the bit */
> +static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
> +{
> +	if (test_bit(nr, addr))
> +		return 1;
> +	set_bit(nr, addr);
> +	return 0;
> +}
> +
>  /* Keep static checkers quiet about nonstatic functions by exporting */
>  int xfs_inode_hasattr(struct xfs_inode *ip);
>  int xfs_attr_get_ilocked(struct xfs_inode *ip, struct xfs_da_args *args);
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index 6ef4841f..29dd10f7 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -330,25 +330,6 @@ xfs_trans_cancel(
>  	xfs_trans_free(tp);
>  }
>  
> -void
> -xfs_trans_ijoin(
> -	xfs_trans_t		*tp,
> -	xfs_inode_t		*ip,
> -	uint			lock_flags)
> -{
> -	xfs_inode_log_item_t	*iip;
> -
> -	if (ip->i_itemp == NULL)
> -		xfs_inode_item_init(ip, ip->i_mount);
> -	iip = ip->i_itemp;
> -	ASSERT(iip->ili_inode != NULL);
> -
> -	ASSERT(iip->ili_lock_flags == 0);
> -	iip->ili_lock_flags = lock_flags;
> -
> -	xfs_trans_add_item(tp, &iip->ili_item);
> -}
> -
>  void
>  xfs_trans_inode_alloc_buf(
>  	xfs_trans_t		*tp,
> @@ -362,52 +343,6 @@ xfs_trans_inode_alloc_buf(
>  	xfs_trans_buf_set_type(tp, bp, XFS_BLFT_DINO_BUF);
>  }
>  
> -/*
> - * This is called to mark the fields indicated in fieldmask as needing
> - * to be logged when the transaction is committed.  The inode must
> - * already be associated with the given transaction.
> - *
> - * The values for fieldmask are defined in xfs_log_format.h.  We always
> - * log all of the core inode if any of it has changed, and we always log
> - * all of the inline data/extents/b-tree root if any of them has changed.
> - */
> -void
> -xfs_trans_log_inode(
> -	xfs_trans_t		*tp,
> -	xfs_inode_t		*ip,
> -	uint			flags)
> -{
> -	ASSERT(ip->i_itemp != NULL);
> -
> -	tp->t_flags |= XFS_TRANS_DIRTY;
> -	set_bit(XFS_LI_DIRTY, &ip->i_itemp->ili_item.li_flags);
> -
> -	/*
> -	 * Always OR in the bits from the ili_last_fields field.
> -	 * This is to coordinate with the xfs_iflush() and xfs_iflush_done()
> -	 * routines in the eventual clearing of the ilf_fields bits.
> -	 * See the big comment in xfs_iflush() for an explanation of
> -	 * this coordination mechanism.
> -	 */
> -	flags |= ip->i_itemp->ili_last_fields;
> -	ip->i_itemp->ili_fields |= flags;
> -}
> -
> -int
> -xfs_trans_roll_inode(
> -	struct xfs_trans	**tpp,
> -	struct xfs_inode	*ip)
> -{
> -	int			error;
> -
> -	xfs_trans_log_inode(*tpp, ip, XFS_ILOG_CORE);
> -	error = xfs_trans_roll(tpp);
> -	if (!error)
> -		xfs_trans_ijoin(*tpp, ip, 0);
> -	return error;
> -}
> -
> -
>  /*
>   * Mark a buffer dirty in the transaction.
>   */
> diff --git a/libxfs/util.c b/libxfs/util.c
> index 1734ae9a..5a89bd03 100644
> --- a/libxfs/util.c
> +++ b/libxfs/util.c
> @@ -149,33 +149,6 @@ current_time(struct inode *inode)
>  	return tv;
>  }
>  
> -/*
> - * Change the requested timestamp in the given inode.
> - */
> -void
> -xfs_trans_ichgtime(
> -	struct xfs_trans	*tp,
> -	struct xfs_inode	*ip,
> -	int			flags)
> -{
> -	struct inode		*inode = VFS_I(ip);
> -	struct timespec64	tv;
> -
> -	ASSERT(tp);
> -	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> -
> -	tv = current_time(inode);
> -
> -	if (flags & XFS_ICHGTIME_MOD)
> -		VFS_I(ip)->i_mtime = tv;
> -	if (flags & XFS_ICHGTIME_CHG)
> -		VFS_I(ip)->i_ctime = tv;
> -	if (flags & XFS_ICHGTIME_CREATE) {
> -		ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec;
> -		ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec;
> -	}
> -}
> -
>  STATIC uint16_t
>  xfs_flags2diflags(
>  	struct xfs_inode	*ip,
> diff --git a/libxfs/xfs_trans_inode.c b/libxfs/xfs_trans_inode.c
> new file mode 100644
> index 00000000..87e6335b
> --- /dev/null
> +++ b/libxfs/xfs_trans_inode.c
> @@ -0,0 +1,155 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2000,2005 Silicon Graphics, Inc.
> + * All Rights Reserved.
> + */
> +#include "libxfs_priv.h"
> +#include "xfs_fs.h"
> +#include "xfs_shared.h"
> +#include "xfs_format.h"
> +#include "xfs_log_format.h"
> +#include "xfs_trans_resv.h"
> +#include "xfs_mount.h"
> +#include "xfs_inode.h"
> +#include "xfs_trans.h"
> +#include "xfs_trace.h"
> +
> +/*
> + * Add a locked inode to the transaction.
> + *
> + * The inode must be locked, and it cannot be associated with any transaction.
> + * If lock_flags is non-zero the inode will be unlocked on transaction commit.
> + */
> +void
> +xfs_trans_ijoin(
> +	struct xfs_trans	*tp,
> +	struct xfs_inode	*ip,
> +	uint			lock_flags)
> +{
> +	xfs_inode_log_item_t	*iip;
> +
> +	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> +	if (ip->i_itemp == NULL)
> +		xfs_inode_item_init(ip, ip->i_mount);
> +	iip = ip->i_itemp;
> +
> +	ASSERT(iip->ili_lock_flags == 0);
> +	iip->ili_lock_flags = lock_flags;
> +
> +	/*
> +	 * Get a log_item_desc to point at the new item.
> +	 */
> +	xfs_trans_add_item(tp, &iip->ili_item);
> +}
> +
> +/*
> + * Transactional inode timestamp update. Requires the inode to be locked and
> + * joined to the transaction supplied. Relies on the transaction subsystem to
> + * track dirty state and update/writeback the inode accordingly.
> + */
> +void
> +xfs_trans_ichgtime(
> +	struct xfs_trans	*tp,
> +	struct xfs_inode	*ip,
> +	int			flags)
> +{
> +	struct inode		*inode = VFS_I(ip);
> +	struct timespec64 tv;
> +
> +	ASSERT(tp);
> +	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> +
> +	tv = current_time(inode);
> +
> +	if (flags & XFS_ICHGTIME_MOD)
> +		inode->i_mtime = tv;
> +	if (flags & XFS_ICHGTIME_CHG)
> +		inode->i_ctime = tv;
> +	if (flags & XFS_ICHGTIME_CREATE) {
> +		ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec;
> +		ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec;
> +	}
> +}
> +
> +/*
> + * This is called to mark the fields indicated in fieldmask as needing
> + * to be logged when the transaction is committed.  The inode must
> + * already be associated with the given transaction.
> + *
> + * The values for fieldmask are defined in xfs_inode_item.h.  We always
> + * log all of the core inode if any of it has changed, and we always log
> + * all of the inline data/extents/b-tree root if any of them has changed.
> + */
> +void
> +xfs_trans_log_inode(
> +	xfs_trans_t	*tp,
> +	xfs_inode_t	*ip,
> +	uint		flags)
> +{
> +	struct inode	*inode = VFS_I(ip);
> +
> +	ASSERT(ip->i_itemp != NULL);
> +	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> +
> +	/*
> +	 * Don't bother with i_lock for the I_DIRTY_TIME check here, as races
> +	 * don't matter - we either will need an extra transaction in 24 hours
> +	 * to log the timestamps, or will clear already cleared fields in the
> +	 * worst case.
> +	 */
> +	if (inode->i_state & (I_DIRTY_TIME | I_DIRTY_TIME_EXPIRED)) {
> +		spin_lock(&inode->i_lock);
> +		inode->i_state &= ~(I_DIRTY_TIME | I_DIRTY_TIME_EXPIRED);
> +		spin_unlock(&inode->i_lock);
> +	}
> +
> +	/*
> +	 * Record the specific change for fdatasync optimisation. This
> +	 * allows fdatasync to skip log forces for inodes that are only
> +	 * timestamp dirty. We do this before the change count so that
> +	 * the core being logged in this case does not impact on fdatasync
> +	 * behaviour.
> +	 */
> +	ip->i_itemp->ili_fsync_fields |= flags;
> +
> +	/*
> +	 * First time we log the inode in a transaction, bump the inode change
> +	 * counter if it is configured for this to occur. While we have the
> +	 * inode locked exclusively for metadata modification, we can usually
> +	 * avoid setting XFS_ILOG_CORE if no one has queried the value since
> +	 * the last time it was incremented. If we have XFS_ILOG_CORE already
> +	 * set however, then go ahead and bump the i_version counter
> +	 * unconditionally.
> +	 */
> +	if (!test_and_set_bit(XFS_LI_DIRTY, &ip->i_itemp->ili_item.li_flags) &&
> +	    IS_I_VERSION(VFS_I(ip))) {
> +		if (inode_maybe_inc_iversion(VFS_I(ip), flags & XFS_ILOG_CORE))
> +			flags |= XFS_ILOG_CORE;
> +	}
> +
> +	tp->t_flags |= XFS_TRANS_DIRTY;
> +
> +	/*
> +	 * Always OR in the bits from the ili_last_fields field.
> +	 * This is to coordinate with the xfs_iflush() and xfs_iflush_done()
> +	 * routines in the eventual clearing of the ili_fields bits.
> +	 * See the big comment in xfs_iflush() for an explanation of
> +	 * this coordination mechanism.
> +	 */
> +	flags |= ip->i_itemp->ili_last_fields;
> +	ip->i_itemp->ili_fields |= flags;
> +}
> +
> +int
> +xfs_trans_roll_inode(
> +	struct xfs_trans	**tpp,
> +	struct xfs_inode	*ip)
> +{
> +	int			error;
> +
> +	xfs_trans_log_inode(*tpp, ip, XFS_ILOG_CORE);
> +	error = xfs_trans_roll(tpp);
> +	if (!error)
> +		xfs_trans_ijoin(*tpp, ip, 0);
> +	return error;
> +}
> 

Re: [PATCH 0/3] xfsprogs: more libxfs/ spring cleaning

[Darrick J. Wong]

On Thu, May 16, 2019 at 12:42:45PM -0500, Eric Sandeen wrote:
> It wasn't super clear before - my goal here is to keep reducing
> cosmetic differences between kernelspace & userspace libxfs/*
> files and functions.
> 
> To that end, 3 more patches ... the first one may requires someone
> who groks the libxfs_* API namespace picture (looking at you, Dave!)
> 
> (this abandons the "make new files" patches I sent before, at least
> for now, I'll heed dave's advice to minimize moves...)

Er, can you turn on diffstat for each patch that gets sent?  That'll
make it much easier for me to figure out if any of these patches have to
go in the kernel first.

I /think/ the answer is 'no'....?

--D

> Thanks,
> -Eric

Re: [PATCH 0/3] xfsprogs: more libxfs/ spring cleaning

[Darrick J. Wong]

On Mon, May 20, 2019 at 04:03:11PM -0700, Darrick J. Wong wrote:
> On Thu, May 16, 2019 at 12:42:45PM -0500, Eric Sandeen wrote:
> > It wasn't super clear before - my goal here is to keep reducing
> > cosmetic differences between kernelspace & userspace libxfs/*
> > files and functions.
> > 
> > To that end, 3 more patches ... the first one may requires someone
> > who groks the libxfs_* API namespace picture (looking at you, Dave!)
> > 
> > (this abandons the "make new files" patches I sent before, at least
> > for now, I'll heed dave's advice to minimize moves...)
> 
> Er, can you turn on diffstat for each patch that gets sent?  That'll
> make it much easier for me to figure out if any of these patches have to
> go in the kernel first.
> 
> I /think/ the answer is 'no'....?

Yes, this is purely userspace stuff, no kernel patches required.

For the whole series (modulo everyone else's comments),
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D


> --D
> 
> > Thanks,
> > -Eric

Re: [PATCH 6/3] libxfs: factor common xfs_trans_bjoin code

[Darrick J. Wong]

On Mon, May 20, 2019 at 05:58:29PM -0500, Eric Sandeen wrote:
> On 5/20/19 5:56 PM, Darrick J. Wong wrote:
> > On Thu, May 16, 2019 at 03:39:49PM -0500, Eric Sandeen wrote:
> >> Most of xfs_trans_bjoin is duplicated in xfs_trans_get_buf_map,
> >> xfs_trans_getsb and xfs_trans_read_buf_map.  Add a new
> >> _xfs_trans_bjoin which can be called by all three functions.
> >>
> >> Source kernel commit: d7e84f413726876c0ec66bbf90770f69841f7663
> >>
> >> Signed-off-by: Christoph Hellwig <hch@lst.de>
> >> Reviewed-by: Dave Chinner <david@fromorbit.com>
> >> Signed-off-by: Alex Elder <aelder@sgi.com>
> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> >> ---
> >>  libxfs/trans.c | 53 +++++++++++++++++++++++++++++++++++++-------------
> >>  1 file changed, 39 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/libxfs/trans.c b/libxfs/trans.c
> >> index f3c28fa7..f78222fd 100644
> >> --- a/libxfs/trans.c
> >> +++ b/libxfs/trans.c
> >> @@ -537,19 +537,50 @@ xfs_trans_binval(
> >>  	tp->t_flags |= XFS_TRANS_DIRTY;
> >>  }
> >>  
> >> -void
> >> -xfs_trans_bjoin(
> >> -	xfs_trans_t		*tp,
> >> -	xfs_buf_t		*bp)
> >> +/*
> >> + * Add the locked buffer to the transaction.
> >> + *
> >> + * The buffer must be locked, and it cannot be associated with any
> >> + * transaction.
> >> + *
> >> + * If the buffer does not yet have a buf log item associated with it,
> >> + * then allocate one for it.  Then add the buf item to the transaction.
> >> + */
> >> +STATIC void
> >> +_xfs_trans_bjoin(
> >> +	struct xfs_trans	*tp,
> >> +	struct xfs_buf		*bp,
> >> +	int			reset_recur)
> > 
> > bool?
> 
> If you fix it in the kernel I'll merge it to xfsprogs ;)

LOL, ok.

--D

> >>  {
> >> -	xfs_buf_log_item_t	*bip;
> >> +	struct xfs_buf_log_item	*bip;
> >>  
> >>  	ASSERT(bp->b_transp == NULL);
> >>  
> >> +        /*
> >> +	 * The xfs_buf_log_item pointer is stored in b_log_item.  If
> >> +	 * it doesn't have one yet, then allocate one and initialize it.
> >> +	 * The checks to see if one is there are in xfs_buf_item_init().
> >> +	 */
> >>  	xfs_buf_item_init(bp, tp->t_mountp);
> >>  	bip = bp->b_log_item;
> >> +	if (reset_recur)
> >> +		bip->bli_recur = 0;
> >> +
> >> +	/*
> >> +	 * Attach the item to the transaction so we can find it in
> >> +	 * xfs_trans_get_buf() and friends.
> >> +	 */
> >>  	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
> > 
> > Kill typedef here               ^^^^^^?
> 
> ditto.
> 
> Point of all this is to match the kernel for easier future syncups.
> 
> (oh wait, actually, this is fixed in a later patch, getting rid of the dumb
> cast altogether)
> 
> -Eric
> 
> > --D
> > 
> >>  	bp->b_transp = tp;
> >> +
> >> +}
> >> +
> >> +void
> >> +xfs_trans_bjoin(
> >> +	struct xfs_trans	*tp,
> >> +	struct xfs_buf		*bp)
> >> +{
> >> +	_xfs_trans_bjoin(tp, bp, 0);
> >>  	trace_xfs_trans_bjoin(bp->b_log_item);
> >>  }
> >>  
> >> @@ -594,9 +625,7 @@ xfs_trans_get_buf_map(
> >>  	if (bp == NULL)
> >>  		return NULL;
> >>  
> >> -	xfs_trans_bjoin(tp, bp);
> >> -	bip = bp->b_log_item;
> >> -	bip->bli_recur = 0;
> >> +	_xfs_trans_bjoin(tp, bp, 1);
> >>  	trace_xfs_trans_get_buf(bp->b_log_item);
> >>  	return bp;
> >>  }
> >> @@ -626,9 +655,7 @@ xfs_trans_getsb(
> >>  
> >>  	bp = xfs_getsb(mp);
> >>  
> >> -	xfs_trans_bjoin(tp, bp);
> >> -	bip = bp->b_log_item;
> >> -	bip->bli_recur = 0;
> >> +	_xfs_trans_bjoin(tp, bp, 1);
> >>  	trace_xfs_trans_getsb(bp->b_log_item);
> >>  	return bp;
> >>  }
> >> @@ -677,9 +704,7 @@ xfs_trans_read_buf_map(
> >>  	if (bp->b_error)
> >>  		goto out_relse;
> >>  
> >> -	xfs_trans_bjoin(tp, bp);
> >> -	bip = bp->b_log_item;
> >> -	bip->bli_recur = 0;
> >> +	_xfs_trans_bjoin(tp, bp, 1);
> >>  done:
> >>  	trace_xfs_trans_read_buf(bp->b_log_item);
> >>  	*bpp = bp;
> >> -- 
> >> 2.17.0
> >>
>