[PATCH 00/11] libxfs: spring cleaning

[PATCH 00/11] libxfs: spring cleaning

[Eric Sandeen]

The first 4 should be easy, just basic cleanups.

Patch 5 gets rid of some "libxfs-ification" of what I think are core libxfs
functions that don't need the prefix, but please correct me if I've got that
wrong.

Patches 6-10 are splitting out the misc functions we've accumulated over
time in various files, and tries to re-organize them to more or less
match their kernel counterparts.  This is just a first step toward being
able to see what we've got, and how we've diverged from kernelspace.
Hopefully it'll help guide us to sharing more files - but in some cases
it's only a couple shared functions, so maye splitting this way doesn't
make sense.

Patch 11 is just cosmetic changes to the resulting kernel-ish files,
to eliminate cosmetic differences and really start to show where things
diverge.  With all these patches applied, a graphical diff between
userspace and kernelspace starts to make a little sense.

If the spawning of new files doesn't seem prudent, I could keep them
all in 1 or 2 bigger files, but keep them in order to match the kernel,
and achieve similar purposes.  Curious to see what folks think.

My main goal with the 2nd half was to get things sliced, diced, and sorted
enough that I could even spot the differences.

Thanks,
-Eric

[PATCH 01/11] libxfs: remove i_transp

[Eric Sandeen]

i_transp was removed from kernel code back in 2011, but it was left
in userspace.  It's only used in a few asserts in transaction code
(as it was in the kernel) so there doesnt' seem to be a compelling
reason to carry it around anymore.

Source kernel commit: f3ca87389dbff0a3dc1a7cb2fa7c62e25421c66c

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 include/xfs_inode.h |  1 -
 libxfs/trans.c      | 11 -----------
 2 files changed, 12 deletions(-)

diff --git a/include/xfs_inode.h b/include/xfs_inode.h
index 52d79f3..88b58ac 100644
--- a/include/xfs_inode.h
+++ b/include/xfs_inode.h
@@ -41,7 +41,6 @@ typedef struct xfs_inode {
 	struct xfs_ifork	*i_afp;		/* attribute fork pointer */
 	struct xfs_ifork	*i_cowfp;	/* copy on write extents */
 	struct xfs_ifork	i_df;		/* data fork */
-	struct xfs_trans	*i_transp;	/* ptr to owning transaction */
 	struct xfs_inode_log_item *i_itemp;	/* logging information */
 	unsigned int		i_delayed_blks;	/* count of delay alloc blks */
 	struct xfs_icdinode	i_d;		/* most of ondisk inode */
diff --git a/libxfs/trans.c b/libxfs/trans.c
index db90624..101019b 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -344,7 +344,6 @@ libxfs_trans_ijoin(
 {
 	xfs_inode_log_item_t	*iip;
 
-	ASSERT(ip->i_transp == NULL);
 	if (ip->i_itemp == NULL)
 		xfs_inode_item_init(ip, ip->i_mount);
 	iip = ip->i_itemp;
@@ -356,7 +355,6 @@ libxfs_trans_ijoin(
 
 	xfs_trans_add_item(tp, (xfs_log_item_t *)(iip));
 
-	ip->i_transp = tp;
 #ifdef XACT_DEBUG
 	fprintf(stderr, "ijoin'd inode %llu, transaction %p\n", ip->i_ino, tp);
 #endif
@@ -368,7 +366,6 @@ libxfs_trans_ijoin_ref(
 	xfs_inode_t		*ip,
 	int			lock_flags)
 {
-	ASSERT(ip->i_transp == tp);
 	ASSERT(ip->i_itemp != NULL);
 
 	xfs_trans_ijoin(tp, ip, lock_flags);
@@ -406,7 +403,6 @@ xfs_trans_log_inode(
 	xfs_inode_t		*ip,
 	uint			flags)
 {
-	ASSERT(ip->i_transp == tp);
 	ASSERT(ip->i_itemp != NULL);
 #ifdef XACT_DEBUG
 	fprintf(stderr, "dirtied inode %llu, transaction %p\n", ip->i_ino, tp);
@@ -817,7 +813,6 @@ inode_item_done(
 	ASSERT(ip != NULL);
 
 	if (!(iip->ili_fields & XFS_ILOG_ALL)) {
-		ip->i_transp = NULL;	/* disassociate from transaction */
 		iip->ili_flags = 0;	/* reset all flags */
 		goto free;
 	}
@@ -838,7 +833,6 @@ inode_item_done(
 	 * we still release the buffer reference we currently hold.
 	 */
 	error = libxfs_iflush_int(ip, bp);
-	ip->i_transp = NULL;	/* disassociate from transaction */
 	bp->b_transp = NULL;	/* remove xact ptr */
 
 	if (error) {
@@ -927,11 +921,6 @@ static void
 inode_item_unlock(
 	xfs_inode_log_item_t	*iip)
 {
-	xfs_inode_t		*ip = iip->ili_inode;
-
-	/* Clear the transaction pointer in the inode. */
-	ip->i_transp = NULL;
-
 	iip->ili_flags = 0;
 	xfs_inode_item_put(iip);
 }
-- 
1.8.3.1

Re: [PATCH 01/11] libxfs: remove i_transp

[Dave Chinner]

On Fri, May 10, 2019 at 03:18:20PM -0500, Eric Sandeen wrote:
> i_transp was removed from kernel code back in 2011, but it was left
> in userspace.  It's only used in a few asserts in transaction code
> (as it was in the kernel) so there doesnt' seem to be a compelling
> reason to carry it around anymore.
> 
> Source kernel commit: f3ca87389dbff0a3dc1a7cb2fa7c62e25421c66c
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

looks good.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 01/11] libxfs: remove i_transp

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 02/11] libxfs: remove xfs_inode_log_item ili_flags

[Eric Sandeen]

ili_flags is only set to zero and asserted to be zero; it serves
no purpose, so remove it.

(it was renamed to ili_lock_flags in the kernel in commit 898621d5,
for some reason userspace had both, with ili_flags ~unused)

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

diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index e6bb74c..832bde1 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -29,7 +29,6 @@ typedef struct xfs_log_item {
 typedef struct xfs_inode_log_item {
 	xfs_log_item_t		ili_item;		/* common portion */
 	struct xfs_inode	*ili_inode;		/* inode pointer */
-	unsigned short		ili_flags;		/* misc flags */
 	unsigned short		ili_lock_flags;		/* lock flags */
 	unsigned int		ili_fields;		/* fields to be logged */
 	unsigned int		ili_last_fields;	/* fields when flushed*/
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 101019b..64131b2 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -347,7 +347,6 @@ libxfs_trans_ijoin(
 	if (ip->i_itemp == NULL)
 		xfs_inode_item_init(ip, ip->i_mount);
 	iip = ip->i_itemp;
-	ASSERT(iip->ili_flags == 0);
 	ASSERT(iip->ili_inode != NULL);
 
 	ASSERT(iip->ili_lock_flags == 0);
@@ -812,10 +811,8 @@ inode_item_done(
 	mp = iip->ili_item.li_mountp;
 	ASSERT(ip != NULL);
 
-	if (!(iip->ili_fields & XFS_ILOG_ALL)) {
-		iip->ili_flags = 0;	/* reset all flags */
+	if (!(iip->ili_fields & XFS_ILOG_ALL))
 		goto free;
-	}
 
 	/*
 	 * Get the buffer containing the on-disk inode.
@@ -921,7 +918,6 @@ static void
 inode_item_unlock(
 	xfs_inode_log_item_t	*iip)
 {
-	iip->ili_flags = 0;
 	xfs_inode_item_put(iip);
 }
 
-- 
1.8.3.1

Re: [PATCH 02/11] libxfs: remove xfs_inode_log_item ili_flags

[Dave Chinner]

On Fri, May 10, 2019 at 03:18:21PM -0500, Eric Sandeen wrote:
> ili_flags is only set to zero and asserted to be zero; it serves
> no purpose, so remove it.
> 
> (it was renamed to ili_lock_flags in the kernel in commit 898621d5,
> for some reason userspace had both, with ili_flags ~unused)

Probably something that wasn't properly cleaned up in a big merge.
Good to get rid of it, thought.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 02/11] libxfs: remove xfs_inode_log_item ili_flags

[Christoph Hellwig]

On Fri, May 10, 2019 at 03:18:21PM -0500, Eric Sandeen wrote:
> ili_flags is only set to zero and asserted to be zero; it serves
> no purpose, so remove it.
> 
> (it was renamed to ili_lock_flags in the kernel in commit 898621d5,
> for some reason userspace had both, with ili_flags ~unused)

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 03/11] libxfs: remove unused cruft

[Eric Sandeen]

Remove many unused #defines and functions.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 include/libxfs.h         |  3 ---
 include/xfs_trans.h      |  8 --------
 libxfs/libxfs_api_defs.h |  5 -----
 libxfs/libxfs_priv.h     |  9 ---------
 libxfs/util.c            | 29 -----------------------------
 5 files changed, 54 deletions(-)

diff --git a/include/libxfs.h b/include/libxfs.h
index 2bdef70..230bc3e 100644
--- a/include/libxfs.h
+++ b/include/libxfs.h
@@ -154,9 +154,6 @@ extern int	libxfs_log_header(char *, uuid_t *, int, int, int, xfs_lsn_t,
 extern int	libxfs_alloc_file_space (struct xfs_inode *, xfs_off_t,
 				xfs_off_t, int, int);
 
-extern void	libxfs_fs_repair_cmn_err(int, struct xfs_mount *, char *, ...);
-extern void	libxfs_fs_cmn_err(int, struct xfs_mount *, char *, ...);
-
 /* XXX: this is messy and needs fixing */
 #ifndef __LIBXFS_INTERNAL_XFS_H__
 extern void cmn_err(int, char *, ...);
diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index 832bde1..c1b20a7 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -47,13 +47,6 @@ typedef struct xfs_buf_log_item {
 #define XFS_BLI_STALE			(1<<2)
 #define XFS_BLI_INODE_ALLOC_BUF		(1<<3)
 
-typedef struct xfs_dq_logitem {
-	xfs_log_item_t		qli_item;	/* common portion */
-	struct xfs_dquot	*qli_dquot;	/* dquot ptr */
-	xfs_lsn_t		qli_flush_lsn;	/* lsn at last flush */
-	xfs_dq_logformat_t	qli_format;	/* logged structure */
-} xfs_dq_logitem_t;
-
 typedef struct xfs_qoff_logitem {
 	xfs_log_item_t		qql_item;	/* common portion */
 	struct xfs_qoff_logitem	*qql_start_lip;	/* qoff-start logitem, if any */
@@ -64,7 +57,6 @@ typedef struct xfs_qoff_logitem {
 #define XFS_DEFER_OPS_NR_BUFS	2	/* join up to two buffers */
 
 typedef struct xfs_trans {
-	unsigned int	t_type;			/* transaction type */
 	unsigned int	t_log_res;		/* amt of log space resvd */
 	unsigned int	t_log_count;		/* count for perm log res */
 	unsigned int	t_blk_res;		/* # of blocks resvd */
diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
index bb0f07b..1150ec9 100644
--- a/libxfs/libxfs_api_defs.h
+++ b/libxfs/libxfs_api_defs.h
@@ -16,9 +16,6 @@
 #define xfs_highbit32			libxfs_highbit32
 #define xfs_highbit64			libxfs_highbit64
 
-#define xfs_fs_repair_cmn_err		libxfs_fs_repair_cmn_err
-#define xfs_fs_cmn_err			libxfs_fs_cmn_err
-
 #define xfs_trans_alloc			libxfs_trans_alloc
 #define xfs_trans_alloc_empty		libxfs_trans_alloc_empty
 #define xfs_trans_add_item		libxfs_trans_add_item
@@ -61,7 +58,6 @@
 #define xfs_bmapi_write			libxfs_bmapi_write
 #define xfs_bmapi_read			libxfs_bmapi_read
 #define xfs_bunmapi			libxfs_bunmapi
-#define xfs_bmbt_get_all		libxfs_bmbt_get_all
 #define xfs_rtfree_extent		libxfs_rtfree_extent
 #define xfs_verify_rtbno		libxfs_verify_rtbno
 #define xfs_verify_ino			libxfs_verify_ino
@@ -70,7 +66,6 @@
 #define xfs_defer_finish		libxfs_defer_finish
 #define xfs_defer_cancel		libxfs_defer_cancel
 
-#define xfs_da_brelse			libxfs_da_brelse
 #define xfs_da_hashname			libxfs_da_hashname
 #define xfs_da_shrink_inode		libxfs_da_shrink_inode
 #define xfs_da_read_buf			libxfs_da_read_buf
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index a2a8388..d668a15 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -122,7 +122,6 @@ enum ce { CE_DEBUG, CE_CONT, CE_NOTE, CE_WARN, CE_ALERT, CE_PANIC };
 #define xfs_warn(mp,fmt,args...)		cmn_err(CE_WARN,fmt, ## args)
 #define xfs_err(mp,fmt,args...)			cmn_err(CE_ALERT,fmt, ## args)
 #define xfs_alert(mp,fmt,args...)		cmn_err(CE_ALERT,fmt, ## args)
-#define xfs_alert_tag(mp,tag,fmt,args...)	cmn_err(CE_ALERT,fmt, ## args)
 
 #define xfs_hex_dump(d,n)		((void) 0)
 #define xfs_stack_trace()		((void) 0)
@@ -195,8 +194,6 @@ enum ce { CE_DEBUG, CE_CONT, CE_NOTE, CE_WARN, CE_ALERT, CE_PANIC };
 #endif
 
 /* miscellaneous kernel routines not in user space */
-#define down_read(a)		((void) 0)
-#define up_read(a)		((void) 0)
 #define spin_lock_init(a)	((void) 0)
 #define spin_lock(a)		((void) 0)
 #define spin_unlock(a)		((void) 0)
@@ -400,7 +397,6 @@ roundup_64(uint64_t x, uint32_t y)
 
 #define XBRW_READ			LIBXFS_BREAD
 #define XBRW_WRITE			LIBXFS_BWRITE
-#define xfs_buf_iomove(bp,off,len,data,f)	libxfs_iomove(bp,off,len,data,f)
 #define xfs_buf_zero(bp,off,len)     libxfs_iomove(bp,off,len,NULL,LIBXFS_BZERO)
 
 /* mount stuff */
@@ -436,8 +432,6 @@ roundup_64(uint64_t x, uint32_t y)
 #define xfs_sort					qsort
 
 #define xfs_ilock(ip,mode)				((void) 0)
-#define xfs_ilock_nowait(ip,mode)			((void) 0)
-#define xfs_ilock_demote(ip,mode)			((void) 0)
 #define xfs_ilock_data_map_shared(ip)			(0)
 #define xfs_ilock_attr_map_shared(ip)			(0)
 #define xfs_iunlock(ip,mode)				({	\
@@ -470,9 +464,6 @@ roundup_64(uint64_t x, uint32_t y)
 #define xfs_filestream_lookup_ag(ip)		(0)
 #define xfs_filestream_new_ag(ip,ag)		(0)
 
-#define xfs_log_force(mp,flags)			((void) 0)
-#define XFS_LOG_SYNC				1
-
 /* quota bits */
 #define xfs_trans_mod_dquot_byino(t,i,f,d)		((void) 0)
 #define xfs_trans_reserve_quota_nblks(t,i,b,n,f)	(0)
diff --git a/libxfs/util.c b/libxfs/util.c
index 9fe9a36..8c9954f 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -603,35 +603,6 @@ libxfs_inode_alloc(
 	return error;
 }
 
-/*
- * Userspace versions of common diagnostic routines (varargs fun).
- */
-void
-libxfs_fs_repair_cmn_err(int level, xfs_mount_t *mp, char *fmt, ...)
-{
-	va_list	ap;
-
-	va_start(ap, fmt);
-	vfprintf(stderr, fmt, ap);
-	fprintf(stderr, "  This is a bug.\n");
-	fprintf(stderr, "%s version %s\n", progname, VERSION);
-	fprintf(stderr,
-		"Please capture the filesystem metadata with xfs_metadump and\n"
-		"report it to linux-xfs@vger.kernel.org\n");
-	va_end(ap);
-}
-
-void
-libxfs_fs_cmn_err(int level, xfs_mount_t *mp, char *fmt, ...)
-{
-	va_list	ap;
-
-	va_start(ap, fmt);
-	vfprintf(stderr, fmt, ap);
-	fputs("\n", stderr);
-	va_end(ap);
-}
-
 void
 cmn_err(int level, char *fmt, ...)
 {
-- 
1.8.3.1

[PATCH 03/11 V2] libxfs: remove unused cruft

[Eric Sandeen]

Remove many unused #defines and functions.

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

V2: Remove libxfs_trans_ijoin_ref as well

diff --git a/include/libxfs.h b/include/libxfs.h
index 2bdef70..230bc3e 100644
--- a/include/libxfs.h
+++ b/include/libxfs.h
@@ -154,9 +154,6 @@ extern int	libxfs_log_header(char *, uuid_t *, int, int, int, xfs_lsn_t,
 extern int	libxfs_alloc_file_space (struct xfs_inode *, xfs_off_t,
 				xfs_off_t, int, int);
 
-extern void	libxfs_fs_repair_cmn_err(int, struct xfs_mount *, char *, ...);
-extern void	libxfs_fs_cmn_err(int, struct xfs_mount *, char *, ...);
-
 /* XXX: this is messy and needs fixing */
 #ifndef __LIBXFS_INTERNAL_XFS_H__
 extern void cmn_err(int, char *, ...);
diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index 832bde1..10b7453 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -47,13 +47,6 @@ typedef struct xfs_buf_log_item {
 #define XFS_BLI_STALE			(1<<2)
 #define XFS_BLI_INODE_ALLOC_BUF		(1<<3)
 
-typedef struct xfs_dq_logitem {
-	xfs_log_item_t		qli_item;	/* common portion */
-	struct xfs_dquot	*qli_dquot;	/* dquot ptr */
-	xfs_lsn_t		qli_flush_lsn;	/* lsn at last flush */
-	xfs_dq_logformat_t	qli_format;	/* logged structure */
-} xfs_dq_logitem_t;
-
 typedef struct xfs_qoff_logitem {
 	xfs_log_item_t		qql_item;	/* common portion */
 	struct xfs_qoff_logitem	*qql_start_lip;	/* qoff-start logitem, if any */
@@ -64,7 +57,6 @@ typedef struct xfs_qoff_logitem {
 #define XFS_DEFER_OPS_NR_BUFS	2	/* join up to two buffers */
 
 typedef struct xfs_trans {
-	unsigned int	t_type;			/* transaction type */
 	unsigned int	t_log_res;		/* amt of log space resvd */
 	unsigned int	t_log_count;		/* count for perm log res */
 	unsigned int	t_blk_res;		/* # of blocks resvd */
@@ -98,7 +90,6 @@ void xfs_defer_cancel(struct xfs_trans *);
 struct xfs_buf *libxfs_trans_getsb(struct xfs_trans *, struct xfs_mount *, int);
 
 void	libxfs_trans_ijoin(struct xfs_trans *, struct xfs_inode *, uint);
-void	libxfs_trans_ijoin_ref(struct xfs_trans *, struct xfs_inode *, int);
 void	libxfs_trans_log_inode (struct xfs_trans *, struct xfs_inode *,
 				uint);
 int	libxfs_trans_roll_inode (struct xfs_trans **, struct xfs_inode *);
diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
index bb0f07b..1150ec9 100644
--- a/libxfs/libxfs_api_defs.h
+++ b/libxfs/libxfs_api_defs.h
@@ -16,9 +16,6 @@
 #define xfs_highbit32			libxfs_highbit32
 #define xfs_highbit64			libxfs_highbit64
 
-#define xfs_fs_repair_cmn_err		libxfs_fs_repair_cmn_err
-#define xfs_fs_cmn_err			libxfs_fs_cmn_err
-
 #define xfs_trans_alloc			libxfs_trans_alloc
 #define xfs_trans_alloc_empty		libxfs_trans_alloc_empty
 #define xfs_trans_add_item		libxfs_trans_add_item
@@ -61,7 +58,6 @@
 #define xfs_bmapi_write			libxfs_bmapi_write
 #define xfs_bmapi_read			libxfs_bmapi_read
 #define xfs_bunmapi			libxfs_bunmapi
-#define xfs_bmbt_get_all		libxfs_bmbt_get_all
 #define xfs_rtfree_extent		libxfs_rtfree_extent
 #define xfs_verify_rtbno		libxfs_verify_rtbno
 #define xfs_verify_ino			libxfs_verify_ino
@@ -70,7 +66,6 @@
 #define xfs_defer_finish		libxfs_defer_finish
 #define xfs_defer_cancel		libxfs_defer_cancel
 
-#define xfs_da_brelse			libxfs_da_brelse
 #define xfs_da_hashname			libxfs_da_hashname
 #define xfs_da_shrink_inode		libxfs_da_shrink_inode
 #define xfs_da_read_buf			libxfs_da_read_buf
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index a2a8388..d668a15 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -122,7 +122,6 @@ enum ce { CE_DEBUG, CE_CONT, CE_NOTE, CE_WARN, CE_ALERT, CE_PANIC };
 #define xfs_warn(mp,fmt,args...)		cmn_err(CE_WARN,fmt, ## args)
 #define xfs_err(mp,fmt,args...)			cmn_err(CE_ALERT,fmt, ## args)
 #define xfs_alert(mp,fmt,args...)		cmn_err(CE_ALERT,fmt, ## args)
-#define xfs_alert_tag(mp,tag,fmt,args...)	cmn_err(CE_ALERT,fmt, ## args)
 
 #define xfs_hex_dump(d,n)		((void) 0)
 #define xfs_stack_trace()		((void) 0)
@@ -195,8 +194,6 @@ enum ce { CE_DEBUG, CE_CONT, CE_NOTE, CE_WARN, CE_ALERT, CE_PANIC };
 #endif
 
 /* miscellaneous kernel routines not in user space */
-#define down_read(a)		((void) 0)
-#define up_read(a)		((void) 0)
 #define spin_lock_init(a)	((void) 0)
 #define spin_lock(a)		((void) 0)
 #define spin_unlock(a)		((void) 0)
@@ -400,7 +397,6 @@ roundup_64(uint64_t x, uint32_t y)
 
 #define XBRW_READ			LIBXFS_BREAD
 #define XBRW_WRITE			LIBXFS_BWRITE
-#define xfs_buf_iomove(bp,off,len,data,f)	libxfs_iomove(bp,off,len,data,f)
 #define xfs_buf_zero(bp,off,len)     libxfs_iomove(bp,off,len,NULL,LIBXFS_BZERO)
 
 /* mount stuff */
@@ -436,8 +432,6 @@ roundup_64(uint64_t x, uint32_t y)
 #define xfs_sort					qsort
 
 #define xfs_ilock(ip,mode)				((void) 0)
-#define xfs_ilock_nowait(ip,mode)			((void) 0)
-#define xfs_ilock_demote(ip,mode)			((void) 0)
 #define xfs_ilock_data_map_shared(ip)			(0)
 #define xfs_ilock_attr_map_shared(ip)			(0)
 #define xfs_iunlock(ip,mode)				({	\
@@ -470,9 +464,6 @@ roundup_64(uint64_t x, uint32_t y)
 #define xfs_filestream_lookup_ag(ip)		(0)
 #define xfs_filestream_new_ag(ip,ag)		(0)
 
-#define xfs_log_force(mp,flags)			((void) 0)
-#define XFS_LOG_SYNC				1
-
 /* quota bits */
 #define xfs_trans_mod_dquot_byino(t,i,f,d)		((void) 0)
 #define xfs_trans_reserve_quota_nblks(t,i,b,n,f)	(0)
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 64131b2..ee79047 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -360,21 +360,6 @@ libxfs_trans_ijoin(
 }
 
 void
-libxfs_trans_ijoin_ref(
-	xfs_trans_t		*tp,
-	xfs_inode_t		*ip,
-	int			lock_flags)
-{
-	ASSERT(ip->i_itemp != NULL);
-
-	xfs_trans_ijoin(tp, ip, lock_flags);
-
-#ifdef XACT_DEBUG
-	fprintf(stderr, "ijoin_ref'd inode %llu, transaction %p\n", ip->i_ino, tp);
-#endif
-}
-
-void
 libxfs_trans_inode_alloc_buf(
 	xfs_trans_t		*tp,
 	xfs_buf_t		*bp)
diff --git a/libxfs/util.c b/libxfs/util.c
index 9fe9a36..8c9954f 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -603,35 +603,6 @@ libxfs_inode_alloc(
 	return error;
 }
 
-/*
- * Userspace versions of common diagnostic routines (varargs fun).
- */
-void
-libxfs_fs_repair_cmn_err(int level, xfs_mount_t *mp, char *fmt, ...)
-{
-	va_list	ap;
-
-	va_start(ap, fmt);
-	vfprintf(stderr, fmt, ap);
-	fprintf(stderr, "  This is a bug.\n");
-	fprintf(stderr, "%s version %s\n", progname, VERSION);
-	fprintf(stderr,
-		"Please capture the filesystem metadata with xfs_metadump and\n"
-		"report it to linux-xfs@vger.kernel.org\n");
-	va_end(ap);
-}
-
-void
-libxfs_fs_cmn_err(int level, xfs_mount_t *mp, char *fmt, ...)
-{
-	va_list	ap;
-
-	va_start(ap, fmt);
-	vfprintf(stderr, fmt, ap);
-	fputs("\n", stderr);
-	va_end(ap);
-}
-
 void
 cmn_err(int level, char *fmt, ...)
 {

Re: [PATCH 03/11 V2] libxfs: remove unused cruft

[Dave Chinner]

On Tue, May 14, 2019 at 07:17:44PM -0500, Eric Sandeen wrote:
> Remove many unused #defines and functions.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
....
>  /* miscellaneous kernel routines not in user space */
> -#define down_read(a)		((void) 0)
> -#define up_read(a)		((void) 0)
>  #define spin_lock_init(a)	((void) 0)
>  #define spin_lock(a)		((void) 0)
>  #define spin_unlock(a)		((void) 0)

The lack of locking in the userspace code scares me somewhat :P

> @@ -400,7 +397,6 @@ roundup_64(uint64_t x, uint32_t y)
>  
>  #define XBRW_READ			LIBXFS_BREAD
>  #define XBRW_WRITE			LIBXFS_BWRITE
> -#define xfs_buf_iomove(bp,off,len,data,f)	libxfs_iomove(bp,off,len,data,f)
>  #define xfs_buf_zero(bp,off,len)     libxfs_iomove(bp,off,len,NULL,LIBXFS_BZERO)
>  
>  /* mount stuff */
> @@ -436,8 +432,6 @@ roundup_64(uint64_t x, uint32_t y)
>  #define xfs_sort					qsort
>  
>  #define xfs_ilock(ip,mode)				((void) 0)
> -#define xfs_ilock_nowait(ip,mode)			((void) 0)
> -#define xfs_ilock_demote(ip,mode)			((void) 0)

Especially that we have transactions that run without inode locks.

But that's not a problem this patch solves, so may as well get rid
of the unused interfaces...

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 03/11] libxfs: remove unused cruft

[Christoph Hellwig]

On Fri, May 10, 2019 at 03:18:22PM -0500, Eric Sandeen wrote:
> Remove many unused #defines and functions.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 04/11] libxfs: rename bp_transp to b_transp in ASSERTs

[Eric Sandeen]

xfs_buf no longer has a bp_transp member; it's b_transp now.
These ASSERTs get #defined away, but it's still best to not have
invalid structure members cluttering up the code.

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

diff --git a/libxfs/trans.c b/libxfs/trans.c
index 64131b2..581ece3 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -381,7 +381,7 @@ libxfs_trans_inode_alloc_buf(
 {
 	xfs_buf_log_item_t	*bip = bp->b_log_item;
 
-	ASSERT(bp->bp_transp == tp);
+	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
 	bip->bli_flags |= XFS_BLI_INODE_ALLOC_BUF;
 	xfs_trans_buf_set_type(tp, bp, XFS_BLFT_DINO_BUF);
@@ -446,7 +446,7 @@ libxfs_trans_dirty_buf(
 {
 	struct xfs_buf_log_item	*bip = bp->b_log_item;
 
-	ASSERT(bp->bp_transp == tp);
+	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
 
 #ifdef XACT_DEBUG
@@ -521,11 +521,11 @@ libxfs_trans_brelse(
 #endif
 
 	if (tp == NULL) {
-		ASSERT(bp->bp_transp == NULL);
+		ASSERT(bp->b_transp == NULL);
 		libxfs_putbuf(bp);
 		return;
 	}
-	ASSERT(bp->bp_transp == tp);
+	ASSERT(bp->b_transp == tp);
 	bip = bp->b_log_item;
 	ASSERT(bip->bli_item.li_type == XFS_LI_BUF);
 	if (bip->bli_recur > 0) {
@@ -555,7 +555,7 @@ libxfs_trans_binval(
 	fprintf(stderr, "binval'd buffer %p, transaction %p\n", bp, tp);
 #endif
 
-	ASSERT(bp->bp_transp == tp);
+	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
 
 	if (bip->bli_flags & XFS_BLI_STALE)
@@ -577,7 +577,7 @@ libxfs_trans_bjoin(
 {
 	xfs_buf_log_item_t	*bip;
 
-	ASSERT(bp->bp_transp == NULL);
+	ASSERT(bp->b_transp == NULL);
 #ifdef XACT_DEBUG
 	fprintf(stderr, "bjoin'd buffer %p, transaction %p\n", bp, tp);
 #endif
@@ -595,7 +595,7 @@ libxfs_trans_bhold(
 {
 	xfs_buf_log_item_t	*bip = bp->b_log_item;
 
-	ASSERT(bp->bp_transp == tp);
+	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
 #ifdef XACT_DEBUG
 	fprintf(stderr, "bhold'd buffer %p, transaction %p\n", bp, tp);
@@ -620,7 +620,7 @@ libxfs_trans_get_buf_map(
 
 	bp = xfs_trans_buf_item_match(tp, btp, map, nmaps);
 	if (bp != NULL) {
-		ASSERT(bp->bp_transp == tp);
+		ASSERT(bp->b_transp == tp);
 		bip = bp->b_log_item;
 		ASSERT(bip != NULL);
 		bip->bli_recur++;
@@ -656,7 +656,7 @@ libxfs_trans_getsb(
 
 	bp = xfs_trans_buf_item_match(tp, mp->m_dev, &map, 1);
 	if (bp != NULL) {
-		ASSERT(bp->bp_transp == tp);
+		ASSERT(bp->b_transp == tp);
 		bip = bp->b_log_item;
 		ASSERT(bip != NULL);
 		bip->bli_recur++;
@@ -703,7 +703,7 @@ libxfs_trans_read_buf_map(
 
 	bp = xfs_trans_buf_item_match(tp, btp, map, nmaps);
 	if (bp != NULL) {
-		ASSERT(bp->bp_transp == tp);
+		ASSERT(bp->b_transp == tp);
 		ASSERT(bp->b_log_item != NULL);
 		bip = bp->b_log_item;
 		bip->bli_recur++;
-- 
1.8.3.1

Re: [PATCH 04/11] libxfs: rename bp_transp to b_transp in ASSERTs

[Dave Chinner]

On Fri, May 10, 2019 at 03:18:23PM -0500, Eric Sandeen wrote:
> xfs_buf no longer has a bp_transp member; it's b_transp now.
> These ASSERTs get #defined away, but it's still best to not have
> invalid structure members cluttering up the code.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

looks good.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 04/11] libxfs: rename bp_transp to b_transp in ASSERTs

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 05/11] libxfs: de-libxfsify core(-ish) functions.

[Eric Sandeen]

There are a ton of "libxfs_" prefixed functions in libxfs/trans.c which
are only called internally by code in libxfs/ - As I understand it,
these should probably be just "xfs_" functions, and indeed many
of them have counterparts in the kernel libxfs/ code.  This is one
small step towards better sync-up of some of the misc libxfs/*
transaction code with kernel code.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/libxfs_api_defs.h |  1 +
 libxfs/trans.c           | 48 ++++++++++++++++++++++++------------------------
 2 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
index 1150ec9..64030af 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
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 581ece3..85c3a50 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,7 +304,7 @@ 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)
@@ -314,7 +314,7 @@ libxfs_trans_alloc_rollable(
 }
 
 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_ijoin_ref(
+xfs_trans_ijoin_ref(
 	xfs_trans_t		*tp,
 	xfs_inode_t		*ip,
 	int			lock_flags)
@@ -375,7 +375,7 @@ libxfs_trans_ijoin_ref(
 }
 
 void
-libxfs_trans_inode_alloc_buf(
+xfs_trans_inode_alloc_buf(
 	xfs_trans_t		*tp,
 	xfs_buf_t		*bp)
 {
@@ -422,7 +422,7 @@ xfs_trans_log_inode(
 }
 
 int
-libxfs_trans_roll_inode(
+xfs_trans_roll_inode(
 	struct xfs_trans	**tpp,
 	struct xfs_inode	*ip)
 {
@@ -440,7 +440,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)
 {
@@ -466,7 +466,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,
@@ -488,7 +488,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)
 {
@@ -511,7 +511,7 @@ xfs_buf_item_put(
 }
 
 void
-libxfs_trans_brelse(
+xfs_trans_brelse(
 	xfs_trans_t		*tp,
 	xfs_buf_t		*bp)
 {
@@ -546,7 +546,7 @@ libxfs_trans_brelse(
 }
 
 void
-libxfs_trans_binval(
+xfs_trans_binval(
 	xfs_trans_t		*tp,
 	xfs_buf_t		*bp)
 {
@@ -571,7 +571,7 @@ libxfs_trans_binval(
 }
 
 void
-libxfs_trans_bjoin(
+xfs_trans_bjoin(
 	xfs_trans_t		*tp,
 	xfs_buf_t		*bp)
 {
@@ -589,7 +589,7 @@ libxfs_trans_bjoin(
 }
 
 void
-libxfs_trans_bhold(
+xfs_trans_bhold(
 	xfs_trans_t		*tp,
 	xfs_buf_t		*bp)
 {
@@ -605,7 +605,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,
@@ -641,7 +641,7 @@ libxfs_trans_get_buf_map(
 }
 
 xfs_buf_t *
-libxfs_trans_getsb(
+xfs_trans_getsb(
 	xfs_trans_t		*tp,
 	xfs_mount_t		*mp,
 	int			flags)
@@ -675,7 +675,7 @@ libxfs_trans_getsb(
 }
 
 int
-libxfs_trans_read_buf_map(
+xfs_trans_read_buf_map(
 	xfs_mount_t		*mp,
 	xfs_trans_t		*tp,
 	struct xfs_buftarg	*btp,
@@ -743,7 +743,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)
@@ -1004,7 +1004,7 @@ out_unreserve:
 }
 
 int
-libxfs_trans_commit(
+xfs_trans_commit(
 	struct xfs_trans	*tp)
 {
 	return __xfs_trans_commit(tp, false);
-- 
1.8.3.1

Re: [PATCH 05/11] libxfs: de-libxfsify core(-ish) functions.

[Eric Sandeen]

On 5/10/19 3:18 PM, Eric Sandeen wrote:
> There are a ton of "libxfs_" prefixed functions in libxfs/trans.c which
> are only called internally by code in libxfs/ - As I understand it,
> these should probably be just "xfs_" functions, and indeed many
> of them have counterparts in the kernel libxfs/ code.  This is one
> small step towards better sync-up of some of the misc libxfs/*
> transaction code with kernel code.

I should have changed internal callers too, will resend after other
review.

> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>  libxfs/libxfs_api_defs.h |  1 +
>  libxfs/trans.c           | 48 ++++++++++++++++++++++++------------------------
>  2 files changed, 25 insertions(+), 24 deletions(-)
> 
> diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
> index 1150ec9..64030af 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
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index 581ece3..85c3a50 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,7 +304,7 @@ 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)
> @@ -314,7 +314,7 @@ libxfs_trans_alloc_rollable(
>  }
>  
>  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_ijoin_ref(
> +xfs_trans_ijoin_ref(
>  	xfs_trans_t		*tp,
>  	xfs_inode_t		*ip,
>  	int			lock_flags)
> @@ -375,7 +375,7 @@ libxfs_trans_ijoin_ref(
>  }
>  
>  void
> -libxfs_trans_inode_alloc_buf(
> +xfs_trans_inode_alloc_buf(
>  	xfs_trans_t		*tp,
>  	xfs_buf_t		*bp)
>  {
> @@ -422,7 +422,7 @@ xfs_trans_log_inode(
>  }
>  
>  int
> -libxfs_trans_roll_inode(
> +xfs_trans_roll_inode(
>  	struct xfs_trans	**tpp,
>  	struct xfs_inode	*ip)
>  {
> @@ -440,7 +440,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)
>  {
> @@ -466,7 +466,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,
> @@ -488,7 +488,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)
>  {
> @@ -511,7 +511,7 @@ xfs_buf_item_put(
>  }
>  
>  void
> -libxfs_trans_brelse(
> +xfs_trans_brelse(
>  	xfs_trans_t		*tp,
>  	xfs_buf_t		*bp)
>  {
> @@ -546,7 +546,7 @@ libxfs_trans_brelse(
>  }
>  
>  void
> -libxfs_trans_binval(
> +xfs_trans_binval(
>  	xfs_trans_t		*tp,
>  	xfs_buf_t		*bp)
>  {
> @@ -571,7 +571,7 @@ libxfs_trans_binval(
>  }
>  
>  void
> -libxfs_trans_bjoin(
> +xfs_trans_bjoin(
>  	xfs_trans_t		*tp,
>  	xfs_buf_t		*bp)
>  {
> @@ -589,7 +589,7 @@ libxfs_trans_bjoin(
>  }
>  
>  void
> -libxfs_trans_bhold(
> +xfs_trans_bhold(
>  	xfs_trans_t		*tp,
>  	xfs_buf_t		*bp)
>  {
> @@ -605,7 +605,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,
> @@ -641,7 +641,7 @@ libxfs_trans_get_buf_map(
>  }
>  
>  xfs_buf_t *
> -libxfs_trans_getsb(
> +xfs_trans_getsb(
>  	xfs_trans_t		*tp,
>  	xfs_mount_t		*mp,
>  	int			flags)
> @@ -675,7 +675,7 @@ libxfs_trans_getsb(
>  }
>  
>  int
> -libxfs_trans_read_buf_map(
> +xfs_trans_read_buf_map(
>  	xfs_mount_t		*mp,
>  	xfs_trans_t		*tp,
>  	struct xfs_buftarg	*btp,
> @@ -743,7 +743,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)
> @@ -1004,7 +1004,7 @@ out_unreserve:
>  }
>  
>  int
> -libxfs_trans_commit(
> +xfs_trans_commit(
>  	struct xfs_trans	*tp)
>  {
>  	return __xfs_trans_commit(tp, false);
> 

Re: [PATCH 05/11] libxfs: de-libxfsify core(-ish) functions.

[Christoph Hellwig]

On Fri, May 10, 2019 at 04:41:03PM -0500, Eric Sandeen wrote:
> On 5/10/19 3:18 PM, Eric Sandeen wrote:
> > There are a ton of "libxfs_" prefixed functions in libxfs/trans.c which
> > are only called internally by code in libxfs/ - As I understand it,
> > these should probably be just "xfs_" functions, and indeed many
> > of them have counterparts in the kernel libxfs/ code.  This is one
> > small step towards better sync-up of some of the misc libxfs/*
> > transaction code with kernel code.
> 
> I should have changed internal callers too, will resend after other
> review.

I have to admit I never got this whole libxfs renaming.  Why can't
we just use the xfs_* namespace for libxfs as well to start with?

Re: [PATCH 05/11] libxfs: de-libxfsify core(-ish) functions.

[Eric Sandeen]

On 5/15/19 1:52 AM, Christoph Hellwig wrote:
> On Fri, May 10, 2019 at 04:41:03PM -0500, Eric Sandeen wrote:
>> On 5/10/19 3:18 PM, Eric Sandeen wrote:
>>> There are a ton of "libxfs_" prefixed functions in libxfs/trans.c which
>>> are only called internally by code in libxfs/ - As I understand it,
>>> these should probably be just "xfs_" functions, and indeed many
>>> of them have counterparts in the kernel libxfs/ code.  This is one
>>> small step towards better sync-up of some of the misc libxfs/*
>>> transaction code with kernel code.
>>
>> I should have changed internal callers too, will resend after other
>> review.
> 
> I have to admit I never got this whole libxfs renaming.  Why can't
> we just use the xfs_* namespace for libxfs as well to start with?
> 

I'm struggling to understand it as well.  I have a 2nd patch to move
still more of the shared-ish xfs functions to strip off the "lib" part,
and for the rest I don't know - I'm trying to get to where I understand
the original intent of all this ...

Thanks,
-Eric

[PATCH 06/11] libxfs: create new file trans_buf.c

[Eric Sandeen]

Pull functions out of libxfs/*.c into trans_buf.c, if they roughly match
the kernel's xfs_trans_buf.c file.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/Makefile      |   1 +
 libxfs/libxfs_priv.h |   1 +
 libxfs/logitem.c     |  36 ------
 libxfs/trans.c       | 301 -------------------------------------------
 libxfs/trans_buf.c   | 354 +++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 356 insertions(+), 337 deletions(-)
 create mode 100644 libxfs/trans_buf.c

diff --git a/libxfs/Makefile b/libxfs/Makefile
index 160498d..da0ce79 100644
--- a/libxfs/Makefile
+++ b/libxfs/Makefile
@@ -58,6 +58,7 @@ CFILES = cache.c \
 	logitem.c \
 	rdwr.c \
 	trans.c \
+	trans_buf.c \
 	util.c \
 	xfs_ag.c \
 	xfs_ag_resv.c \
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index d668a15..7c07188 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -519,6 +519,7 @@ void xfs_inode_item_init(struct xfs_inode *, struct xfs_mount *);
 /* xfs_buf_item.c */
 void xfs_buf_item_init(struct xfs_buf *, struct xfs_mount *);
 void xfs_buf_item_log(struct xfs_buf_log_item *, uint, uint);
+void xfs_buf_item_put(struct xfs_buf_log_item *bip);
 
 /* xfs_trans_buf.c */
 struct xfs_buf *xfs_trans_buf_item_match(struct xfs_trans *,
diff --git a/libxfs/logitem.c b/libxfs/logitem.c
index 4da9bc1..ce34c68 100644
--- a/libxfs/logitem.c
+++ b/libxfs/logitem.c
@@ -20,42 +20,6 @@ kmem_zone_t	*xfs_buf_item_zone;
 kmem_zone_t	*xfs_ili_zone;		/* inode log item zone */
 
 /*
- * Following functions from fs/xfs/xfs_trans_buf.c
- */
-
-/*
- * Check to see if a buffer matching the given parameters is already
- * a part of the given transaction.
- */
-xfs_buf_t *
-xfs_trans_buf_item_match(
-	xfs_trans_t		*tp,
-	struct xfs_buftarg	*btp,
-	struct xfs_buf_map	*map,
-	int			nmaps)
-{
-	struct xfs_log_item	*lip;
-	struct xfs_buf_log_item *blip;
-	int			len = 0;
-	int			i;
-
-	for (i = 0; i < nmaps; i++)
-		len += map[i].bm_len;
-
-	list_for_each_entry(lip, &tp->t_items, li_trans) {
-		blip = (struct xfs_buf_log_item *)lip;
-		if (blip->bli_item.li_type == XFS_LI_BUF &&
-		    blip->bli_buf->b_target->dev == btp->dev &&
-		    XFS_BUF_ADDR(blip->bli_buf) == map[0].bm_bn &&
-		    blip->bli_buf->b_bcount == BBTOB(len)) {
-			ASSERT(blip->bli_buf->b_map_count == nmaps);
-			return blip->bli_buf;
-		}
-	}
-
-	return NULL;
-}
-/*
  * The following are from fs/xfs/xfs_buf_item.c
  */
 
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 85c3a50..295f167 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -374,19 +374,6 @@ xfs_trans_ijoin_ref(
 #endif
 }
 
-void
-xfs_trans_inode_alloc_buf(
-	xfs_trans_t		*tp,
-	xfs_buf_t		*bp)
-{
-	xfs_buf_log_item_t	*bip = bp->b_log_item;
-
-	ASSERT(bp->b_transp == tp);
-	ASSERT(bip != NULL);
-	bip->bli_flags |= XFS_BLI_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
@@ -435,72 +422,7 @@ xfs_trans_roll_inode(
 	return error;
 }
 
-
-/*
- * Mark a buffer dirty in the transaction.
- */
-void
-xfs_trans_dirty_buf(
-	struct xfs_trans	*tp,
-	struct xfs_buf		*bp)
-{
-	struct xfs_buf_log_item	*bip = bp->b_log_item;
-
-	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);
-}
-
-/*
- * This is called to mark bytes first through last inclusive of the given
- * buffer as needing to be logged when the transaction is committed.
- * The buffer must already be associated with the given transaction.
- *
- * First and last are numbers relative to the beginning of this buffer,
- * so the first byte in the buffer is numbered 0 regardless of the
- * value of b_blkno.
- */
 void
-xfs_trans_log_buf(
-	struct xfs_trans	*tp,
-	struct xfs_buf		*bp,
-	uint			first,
-	uint			last)
-{
-	struct xfs_buf_log_item	*bip = bp->b_log_item;
-
-	ASSERT((first <= last) && (last < bp->b_bcount));
-
-	xfs_trans_dirty_buf(tp, bp);
-	xfs_buf_item_log(bip, first, last);
-}
-
-/*
- * For userspace, ordered buffers just need to be marked dirty so
- * the transaction commit will write them and mark them up-to-date.
- * In essence, they are just like any other logged buffer in userspace.
- *
- * If the buffer is already dirty, trigger the "already logged" return condition.
- */
-bool
-xfs_trans_ordered_buf(
-	struct xfs_trans	*tp,
-	struct xfs_buf		*bp)
-{
-	struct xfs_buf_log_item	*bip = bp->b_log_item;
-	bool			ret;
-
-	ret = test_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags);
-	libxfs_trans_log_buf(tp, bp, 0, bp->b_bcount);
-	return ret;
-}
-
-static void
 xfs_buf_item_put(
 	struct xfs_buf_log_item	*bip)
 {
@@ -510,229 +432,6 @@ xfs_buf_item_put(
 	kmem_zone_free(xfs_buf_item_zone, bip);
 }
 
-void
-xfs_trans_brelse(
-	xfs_trans_t		*tp,
-	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;
-	}
-	ASSERT(bp->b_transp == tp);
-	bip = bp->b_log_item;
-	ASSERT(bip->bli_item.li_type == XFS_LI_BUF);
-	if (bip->bli_recur > 0) {
-		bip->bli_recur--;
-		return;
-	}
-	/* If dirty/stale, can't release till transaction committed */
-	if (bip->bli_flags & XFS_BLI_STALE)
-		return;
-	if (test_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags))
-		return;
-	xfs_trans_del_item(&bip->bli_item);
-	if (bip->bli_flags & XFS_BLI_HOLD)
-		bip->bli_flags &= ~XFS_BLI_HOLD;
-	xfs_buf_item_put(bip);
-	bp->b_transp = NULL;
-	libxfs_putbuf(bp);
-}
-
-void
-xfs_trans_binval(
-	xfs_trans_t		*tp,
-	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);
-
-	if (bip->bli_flags & XFS_BLI_STALE)
-		return;
-	XFS_BUF_UNDELAYWRITE(bp);
-	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;
-	set_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags);
-	tp->t_flags |= XFS_TRANS_DIRTY;
-}
-
-void
-xfs_trans_bjoin(
-	xfs_trans_t		*tp,
-	xfs_buf_t		*bp)
-{
-	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;
-}
-
-void
-xfs_trans_bhold(
-	xfs_trans_t		*tp,
-	xfs_buf_t		*bp)
-{
-	xfs_buf_log_item_t	*bip = bp->b_log_item;
-
-	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;
-}
-
-xfs_buf_t *
-xfs_trans_get_buf_map(
-	xfs_trans_t		*tp,
-	struct xfs_buftarg	*btp,
-	struct xfs_buf_map	*map,
-	int			nmaps,
-	uint			f)
-{
-	xfs_buf_t		*bp;
-	xfs_buf_log_item_t	*bip;
-
-	if (tp == NULL)
-		return libxfs_getbuf_map(btp, map, nmaps, 0);
-
-	bp = xfs_trans_buf_item_match(tp, btp, map, nmaps);
-	if (bp != NULL) {
-		ASSERT(bp->b_transp == tp);
-		bip = bp->b_log_item;
-		ASSERT(bip != NULL);
-		bip->bli_recur++;
-		return bp;
-	}
-
-	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
-
-	libxfs_trans_bjoin(tp, bp);
-	bip = bp->b_log_item;
-	bip->bli_recur = 0;
-	return bp;
-}
-
-xfs_buf_t *
-xfs_trans_getsb(
-	xfs_trans_t		*tp,
-	xfs_mount_t		*mp,
-	int			flags)
-{
-	xfs_buf_t		*bp;
-	xfs_buf_log_item_t	*bip;
-	int			len = XFS_FSS_TO_BB(mp, 1);
-	DEFINE_SINGLE_BUF_MAP(map, XFS_SB_DADDR, len);
-
-	if (tp == NULL)
-		return libxfs_getsb(mp, flags);
-
-	bp = xfs_trans_buf_item_match(tp, mp->m_dev, &map, 1);
-	if (bp != NULL) {
-		ASSERT(bp->b_transp == tp);
-		bip = bp->b_log_item;
-		ASSERT(bip != NULL);
-		bip->bli_recur++;
-		return bp;
-	}
-
-	bp = libxfs_getsb(mp, flags);
-#ifdef XACT_DEBUG
-	fprintf(stderr, "trans_get_sb buffer %p, transaction %p\n", bp, tp);
-#endif
-
-	libxfs_trans_bjoin(tp, bp);
-	bip = bp->b_log_item;
-	bip->bli_recur = 0;
-	return bp;
-}
-
-int
-xfs_trans_read_buf_map(
-	xfs_mount_t		*mp,
-	xfs_trans_t		*tp,
-	struct xfs_buftarg	*btp,
-	struct xfs_buf_map	*map,
-	int			nmaps,
-	uint			flags,
-	xfs_buf_t		**bpp,
-	const struct xfs_buf_ops *ops)
-{
-	xfs_buf_t		*bp;
-	xfs_buf_log_item_t	*bip;
-	int			error;
-
-	*bpp = NULL;
-
-	if (tp == NULL) {
-		bp = libxfs_readbuf_map(btp, map, nmaps, flags, ops);
-		if (!bp) {
-			return (flags & XBF_TRYLOCK) ?  -EAGAIN : -ENOMEM;
-		}
-		if (bp->b_error)
-			goto out_relse;
-		goto done;
-	}
-
-	bp = xfs_trans_buf_item_match(tp, btp, map, nmaps);
-	if (bp != NULL) {
-		ASSERT(bp->b_transp == tp);
-		ASSERT(bp->b_log_item != NULL);
-		bip = bp->b_log_item;
-		bip->bli_recur++;
-		goto done;
-	}
-
-	bp = libxfs_readbuf_map(btp, map, nmaps, flags, ops);
-	if (!bp) {
-		return (flags & XBF_TRYLOCK) ?  -EAGAIN : -ENOMEM;
-	}
-	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:
-	*bpp = bp;
-	return 0;
-out_relse:
-	error = bp->b_error;
-	xfs_buf_relse(bp);
-	return error;
-}
-
 /*
  * Record the indicated change to the given field for application
  * to the file system's superblock when the transaction commits.
diff --git a/libxfs/trans_buf.c b/libxfs/trans_buf.c
new file mode 100644
index 0000000..9c2c36e
--- /dev/null
+++ b/libxfs/trans_buf.c
@@ -0,0 +1,354 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2000-2001,2005-2006 Silicon Graphics, Inc.
+ * Copyright (C) 2010 Red Hat, 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_sb.h"
+
+/*
+ * Following functions from fs/xfs/xfs_trans_buf.c
+ */
+
+/*
+ * Check to see if a buffer matching the given parameters is already
+ * a part of the given transaction.
+ */
+xfs_buf_t *
+xfs_trans_buf_item_match(
+	xfs_trans_t		*tp,
+	struct xfs_buftarg	*btp,
+	struct xfs_buf_map	*map,
+	int			nmaps)
+{
+	struct xfs_log_item	*lip;
+	struct xfs_buf_log_item *blip;
+	int			len = 0;
+	int			i;
+
+	for (i = 0; i < nmaps; i++)
+		len += map[i].bm_len;
+
+	list_for_each_entry(lip, &tp->t_items, li_trans) {
+		blip = (struct xfs_buf_log_item *)lip;
+		if (blip->bli_item.li_type == XFS_LI_BUF &&
+		    blip->bli_buf->b_target->dev == btp->dev &&
+		    XFS_BUF_ADDR(blip->bli_buf) == map[0].bm_bn &&
+		    blip->bli_buf->b_bcount == BBTOB(len)) {
+			ASSERT(blip->bli_buf->b_map_count == nmaps);
+			return blip->bli_buf;
+		}
+	}
+
+	return NULL;
+}
+
+void
+xfs_trans_bjoin(
+	xfs_trans_t		*tp,
+	xfs_buf_t		*bp)
+{
+	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;
+}
+
+xfs_buf_t *
+xfs_trans_get_buf_map(
+	xfs_trans_t		*tp,
+	struct xfs_buftarg	*btp,
+	struct xfs_buf_map	*map,
+	int			nmaps,
+	uint			f)
+{
+	xfs_buf_t		*bp;
+	xfs_buf_log_item_t	*bip;
+
+	if (tp == NULL)
+		return libxfs_getbuf_map(btp, map, nmaps, 0);
+
+	bp = xfs_trans_buf_item_match(tp, btp, map, nmaps);
+	if (bp != NULL) {
+		ASSERT(bp->b_transp == tp);
+		bip = bp->b_log_item;
+		ASSERT(bip != NULL);
+		bip->bli_recur++;
+		return bp;
+	}
+
+	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
+
+	libxfs_trans_bjoin(tp, bp);
+	bip = bp->b_log_item;
+	bip->bli_recur = 0;
+	return bp;
+}
+
+xfs_buf_t *
+xfs_trans_getsb(
+	xfs_trans_t		*tp,
+	xfs_mount_t		*mp,
+	int			flags)
+{
+	xfs_buf_t		*bp;
+	xfs_buf_log_item_t	*bip;
+	int			len = XFS_FSS_TO_BB(mp, 1);
+	DEFINE_SINGLE_BUF_MAP(map, XFS_SB_DADDR, len);
+
+	if (tp == NULL)
+		return libxfs_getsb(mp, flags);
+
+	bp = xfs_trans_buf_item_match(tp, mp->m_dev, &map, 1);
+	if (bp != NULL) {
+		ASSERT(bp->b_transp == tp);
+		bip = bp->b_log_item;
+		ASSERT(bip != NULL);
+		bip->bli_recur++;
+		return bp;
+	}
+
+	bp = libxfs_getsb(mp, flags);
+#ifdef XACT_DEBUG
+	fprintf(stderr, "trans_get_sb buffer %p, transaction %p\n", bp, tp);
+#endif
+
+	libxfs_trans_bjoin(tp, bp);
+	bip = bp->b_log_item;
+	bip->bli_recur = 0;
+	return bp;
+}
+
+int
+xfs_trans_read_buf_map(
+	xfs_mount_t		*mp,
+	xfs_trans_t		*tp,
+	struct xfs_buftarg	*btp,
+	struct xfs_buf_map	*map,
+	int			nmaps,
+	uint			flags,
+	xfs_buf_t		**bpp,
+	const struct xfs_buf_ops *ops)
+{
+	xfs_buf_t		*bp;
+	xfs_buf_log_item_t	*bip;
+	int			error;
+
+	*bpp = NULL;
+
+	if (tp == NULL) {
+		bp = libxfs_readbuf_map(btp, map, nmaps, flags, ops);
+		if (!bp) {
+			return (flags & XBF_TRYLOCK) ?  -EAGAIN : -ENOMEM;
+		}
+		if (bp->b_error)
+			goto out_relse;
+		goto done;
+	}
+
+	bp = xfs_trans_buf_item_match(tp, btp, map, nmaps);
+	if (bp != NULL) {
+		ASSERT(bp->b_transp == tp);
+		ASSERT(bp->b_log_item != NULL);
+		bip = bp->b_log_item;
+		bip->bli_recur++;
+		goto done;
+	}
+
+	bp = libxfs_readbuf_map(btp, map, nmaps, flags, ops);
+	if (!bp) {
+		return (flags & XBF_TRYLOCK) ?  -EAGAIN : -ENOMEM;
+	}
+	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:
+	*bpp = bp;
+	return 0;
+out_relse:
+	error = bp->b_error;
+	xfs_buf_relse(bp);
+	return error;
+}
+
+void
+xfs_trans_brelse(
+	xfs_trans_t		*tp,
+	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;
+	}
+	ASSERT(bp->b_transp == tp);
+	bip = bp->b_log_item;
+	ASSERT(bip->bli_item.li_type == XFS_LI_BUF);
+	if (bip->bli_recur > 0) {
+		bip->bli_recur--;
+		return;
+	}
+	/* If dirty/stale, can't release till transaction committed */
+	if (bip->bli_flags & XFS_BLI_STALE)
+		return;
+	if (test_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags))
+		return;
+	xfs_trans_del_item(&bip->bli_item);
+	if (bip->bli_flags & XFS_BLI_HOLD)
+		bip->bli_flags &= ~XFS_BLI_HOLD;
+	xfs_buf_item_put(bip);
+	bp->b_transp = NULL;
+	libxfs_putbuf(bp);
+}
+
+void
+xfs_trans_bhold(
+	xfs_trans_t		*tp,
+	xfs_buf_t		*bp)
+{
+	xfs_buf_log_item_t	*bip = bp->b_log_item;
+
+	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;
+}
+
+/*
+ * Mark a buffer dirty in the transaction.
+ */
+void
+xfs_trans_dirty_buf(
+	struct xfs_trans	*tp,
+	struct xfs_buf		*bp)
+{
+	struct xfs_buf_log_item	*bip = bp->b_log_item;
+
+	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);
+}
+
+/*
+ * This is called to mark bytes first through last inclusive of the given
+ * buffer as needing to be logged when the transaction is committed.
+ * The buffer must already be associated with the given transaction.
+ *
+ * First and last are numbers relative to the beginning of this buffer,
+ * so the first byte in the buffer is numbered 0 regardless of the
+ * value of b_blkno.
+ */
+void
+xfs_trans_log_buf(
+	struct xfs_trans	*tp,
+	struct xfs_buf		*bp,
+	uint			first,
+	uint			last)
+{
+	struct xfs_buf_log_item	*bip = bp->b_log_item;
+
+	ASSERT((first <= last) && (last < bp->b_bcount));
+
+	xfs_trans_dirty_buf(tp, bp);
+	xfs_buf_item_log(bip, first, last);
+}
+
+void
+xfs_trans_binval(
+	xfs_trans_t		*tp,
+	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);
+
+	if (bip->bli_flags & XFS_BLI_STALE)
+		return;
+	XFS_BUF_UNDELAYWRITE(bp);
+	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;
+	set_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags);
+	tp->t_flags |= XFS_TRANS_DIRTY;
+}
+
+void
+xfs_trans_inode_alloc_buf(
+	xfs_trans_t		*tp,
+	xfs_buf_t		*bp)
+{
+	xfs_buf_log_item_t	*bip = bp->b_log_item;
+
+	ASSERT(bp->b_transp == tp);
+	ASSERT(bip != NULL);
+	bip->bli_flags |= XFS_BLI_INODE_ALLOC_BUF;
+	xfs_trans_buf_set_type(tp, bp, XFS_BLFT_DINO_BUF);
+}
+
+/*
+ * For userspace, ordered buffers just need to be marked dirty so
+ * the transaction commit will write them and mark them up-to-date.
+ * In essence, they are just like any other logged buffer in userspace.
+ *
+ * If the buffer is already dirty, trigger the "already logged" return condition.
+ */
+bool
+xfs_trans_ordered_buf(
+	struct xfs_trans	*tp,
+	struct xfs_buf		*bp)
+{
+	struct xfs_buf_log_item	*bip = bp->b_log_item;
+	bool			ret;
+
+	ret = test_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags);
+	libxfs_trans_log_buf(tp, bp, 0, bp->b_bcount);
+	return ret;
+}
-- 
1.8.3.1

Re: [PATCH 06/11] libxfs: create new file trans_buf.c

[Dave Chinner]

On Fri, May 10, 2019 at 03:18:25PM -0500, Eric Sandeen wrote:
> Pull functions out of libxfs/*.c into trans_buf.c, if they roughly match
> the kernel's xfs_trans_buf.c file.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

So I have no problems with this, but I'm not sure what the eventual
goal is? Just sharing code, or is there some functionality that
requires a more complete transaction subsystem in userspace?

I'm asking because if the goal is eventual unification with the
kernel code, then we probably should name the files the same as the
kernel code so we don't have to rename them again when we do the
unification. That will make history searching a bit easier - less
file names to follow across and git blame works a whole lot better...

> +int
> +xfs_trans_read_buf_map(
> +	xfs_mount_t		*mp,
> +	xfs_trans_t		*tp,
> +	struct xfs_buftarg	*btp,
> +	struct xfs_buf_map	*map,
> +	int			nmaps,
> +	uint			flags,
> +	xfs_buf_t		**bpp,
> +	const struct xfs_buf_ops *ops)

Hmmmm. Will there be any follow-up to de-typedef these new files?

/me would love to just have a flag day that de-typedefs all of the
userspace code.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 06/11] libxfs: create new file trans_buf.c

[Christoph Hellwig]

On Wed, May 15, 2019 at 04:07:50PM +1000, Dave Chinner wrote:
> On Fri, May 10, 2019 at 03:18:25PM -0500, Eric Sandeen wrote:
> > Pull functions out of libxfs/*.c into trans_buf.c, if they roughly match
> > the kernel's xfs_trans_buf.c file.
> > 
> > Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> 
> So I have no problems with this, but I'm not sure what the eventual
> goal is? Just sharing code, or is there some functionality that
> requires a more complete transaction subsystem in userspace?
> 
> I'm asking because if the goal is eventual unification with the
> kernel code, then we probably should name the files the same as the
> kernel code so we don't have to rename them again when we do the
> unification. That will make history searching a bit easier - less
> file names to follow across and git blame works a whole lot better...

Even if we don't want to directly share code having the same file
name would still be nice..

Re: [PATCH 06/11] libxfs: create new file trans_buf.c

[Eric Sandeen]

On 5/15/19 1:52 AM, Christoph Hellwig wrote:
> On Wed, May 15, 2019 at 04:07:50PM +1000, Dave Chinner wrote:
>> On Fri, May 10, 2019 at 03:18:25PM -0500, Eric Sandeen wrote:
>>> Pull functions out of libxfs/*.c into trans_buf.c, if they roughly match
>>> the kernel's xfs_trans_buf.c file.
>>>
>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>
>> So I have no problems with this, but I'm not sure what the eventual
>> goal is? Just sharing code, or is there some functionality that
>> requires a more complete transaction subsystem in userspace?
>>
>> I'm asking because if the goal is eventual unification with the
>> kernel code, then we probably should name the files the same as the
>> kernel code so we don't have to rename them again when we do the
>> unification. That will make history searching a bit easier - less
>> file names to follow across and git blame works a whole lot better...
> 
> Even if we don't want to directly share code having the same file
> name would still be nice..

That's the goal, but I didn't want to do it until they really were
shared.  Right now scripts to merge and diff assume that 
libxfs/xfs_* are shared w/ kernel.

We could work around that, but I just figured I wouldn't give them
the same name until they are really the same file.

Doesn't matter much to me either way tbh.

-Eric

Re: [PATCH 06/11] libxfs: create new file trans_buf.c

[Eric Sandeen]

On 5/15/19 1:07 AM, Dave Chinner wrote:
> On Fri, May 10, 2019 at 03:18:25PM -0500, Eric Sandeen wrote:
>> Pull functions out of libxfs/*.c into trans_buf.c, if they roughly match
>> the kernel's xfs_trans_buf.c file.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> 
> So I have no problems with this, but I'm not sure what the eventual
> goal is? Just sharing code, or is there some functionality that
> requires a more complete transaction subsystem in userspace?
> 
> I'm asking because if the goal is eventual unification with the
> kernel code, then we probably should name the files the same as the
> kernel code so we don't have to rename them again when we do the
> unification. That will make history searching a bit easier - less
> file names to follow across and git blame works a whole lot better...
> 
>> +int
>> +xfs_trans_read_buf_map(
>> +	xfs_mount_t		*mp,
>> +	xfs_trans_t		*tp,
>> +	struct xfs_buftarg	*btp,
>> +	struct xfs_buf_map	*map,
>> +	int			nmaps,
>> +	uint			flags,
>> +	xfs_buf_t		**bpp,
>> +	const struct xfs_buf_ops *ops)
> 
> Hmmmm. Will there be any follow-up to de-typedef these new files?
> 
> /me would love to just have a flag day that de-typedefs all of the
> userspace code.


Yes, after I moved stuff enough to see how/if it matches the kernel,
I de-typdefed to match where the kernel has done so.

I could certainly roll that into this patch.  I was trying to do small
steps but maybe that just makes things more complex to follow...

-Eric

[PATCH 07/11] libxfs: create new file buf_item.c

[Eric Sandeen]

Pull functions out of libxfs/* into buf_item.c, if they roughly match
the kernel's xfs_buf_item.c file.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/Makefile      |   1 +
 libxfs/buf_item.c    | 146 +++++++++++++++++++++++++++++++++++++++++++++++++++
 libxfs/libxfs_priv.h |   2 +
 libxfs/logitem.c     |  77 ---------------------------
 libxfs/trans.c       |  55 -------------------
 5 files changed, 149 insertions(+), 132 deletions(-)
 create mode 100644 libxfs/buf_item.c

diff --git a/libxfs/Makefile b/libxfs/Makefile
index da0ce79..820ffb0 100644
--- a/libxfs/Makefile
+++ b/libxfs/Makefile
@@ -52,6 +52,7 @@ HFILES = \
 	xfs_dir2_priv.h
 
 CFILES = cache.c \
+	buf_item.c \
 	defer_item.c \
 	init.c \
 	kmem.c \
diff --git a/libxfs/buf_item.c b/libxfs/buf_item.c
new file mode 100644
index 0000000..2e64c8c
--- /dev/null
+++ b/libxfs/buf_item.c
@@ -0,0 +1,146 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2000-2001,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_trans.h"
+#include "xfs_inode.h"
+
+kmem_zone_t	*xfs_buf_item_zone;
+
+/*
+ * The following are from fs/xfs/xfs_buf_item.c
+ */
+
+void
+xfs_buf_item_put(
+	struct xfs_buf_log_item	*bip)
+{
+	struct xfs_buf		*bp = bip->bli_buf;
+
+	bp->b_log_item = NULL;
+	kmem_zone_free(xfs_buf_item_zone, bip);
+}
+
+void
+buf_item_unlock(
+	xfs_buf_log_item_t	*bip)
+{
+	xfs_buf_t		*bp = bip->bli_buf;
+	uint			hold;
+
+	/* Clear the buffer's association with this transaction. */
+	bip->bli_buf->b_transp = NULL;
+
+	hold = bip->bli_flags & XFS_BLI_HOLD;
+	bip->bli_flags &= ~XFS_BLI_HOLD;
+	xfs_buf_item_put(bip);
+	if (!hold)
+		libxfs_putbuf(bp);
+}
+
+/*
+ * Allocate a new buf log item to go with the given buffer.
+ * Set the buffer's b_log_item field to point to the new
+ * buf log item.  If there are other item's attached to the
+ * buffer (see xfs_buf_attach_iodone() below), then put the
+ * buf log item at the front.
+ */
+void
+xfs_buf_item_init(
+	xfs_buf_t		*bp,
+	xfs_mount_t		*mp)
+{
+	xfs_log_item_t		*lip;
+	xfs_buf_log_item_t	*bip;
+
+#ifdef LI_DEBUG
+	fprintf(stderr, "buf_item_init for buffer %p\n", bp);
+#endif
+
+	/*
+	 * Check to see if there is already a buf log item for
+	 * this buffer.	 If there is, it is guaranteed to be
+	 * the first.  If we do already have one, there is
+	 * nothing to do here so return.
+	 */
+	XFS_BUF_SET_BDSTRAT_FUNC(bp, xfs_bdstrat_cb);
+	if (bp->b_log_item != NULL) {
+		lip = bp->b_log_item;
+		if (lip->li_type == XFS_LI_BUF) {
+#ifdef LI_DEBUG
+			fprintf(stderr,
+				"reused buf item %p for pre-logged buffer %p\n",
+				lip, bp);
+#endif
+			return;
+		}
+	}
+
+	bip = (xfs_buf_log_item_t *)kmem_zone_zalloc(xfs_buf_item_zone,
+						    KM_SLEEP);
+#ifdef LI_DEBUG
+	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);
+	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);
+	bp->b_log_item = bip;
+}
+
+/*
+ * Mark bytes first through last inclusive as dirty in the buf
+ * item's bitmap.
+ */
+void
+xfs_buf_item_log(
+	xfs_buf_log_item_t	*bip,
+	uint			first,
+	uint			last)
+{
+	/*
+	 * Mark the item as having some dirty data for
+	 * quick reference in xfs_buf_item_dirty.
+	 */
+	bip->bli_flags |= XFS_BLI_DIRTY;
+}
+
+void
+buf_item_done(
+	xfs_buf_log_item_t	*bip)
+{
+	xfs_buf_t		*bp;
+	int			hold;
+
+	bp = bip->bli_buf;
+	ASSERT(bp != NULL);
+	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
+		libxfs_writebuf_int(bp, 0);
+	}
+
+	bip->bli_flags &= ~XFS_BLI_HOLD;
+	xfs_buf_item_put(bip);
+	if (hold)
+		return;
+	libxfs_putbuf(bp);
+}
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index 7c07188..155b782 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -520,6 +520,8 @@ void xfs_inode_item_init(struct xfs_inode *, struct xfs_mount *);
 void xfs_buf_item_init(struct xfs_buf *, struct xfs_mount *);
 void xfs_buf_item_log(struct xfs_buf_log_item *, uint, uint);
 void xfs_buf_item_put(struct xfs_buf_log_item *bip);
+void buf_item_unlock(struct xfs_buf_log_item *bip);
+void buf_item_done(struct xfs_buf_log_item *bip);
 
 /* xfs_trans_buf.c */
 struct xfs_buf *xfs_trans_buf_item_match(struct xfs_trans *,
diff --git a/libxfs/logitem.c b/libxfs/logitem.c
index ce34c68..c80e2ed 100644
--- a/libxfs/logitem.c
+++ b/libxfs/logitem.c
@@ -16,86 +16,9 @@
 #include "xfs_inode.h"
 #include "xfs_trans.h"
 
-kmem_zone_t	*xfs_buf_item_zone;
 kmem_zone_t	*xfs_ili_zone;		/* inode log item zone */
 
 /*
- * The following are from fs/xfs/xfs_buf_item.c
- */
-
-/*
- * Allocate a new buf log item to go with the given buffer.
- * Set the buffer's b_log_item field to point to the new
- * buf log item.  If there are other item's attached to the
- * buffer (see xfs_buf_attach_iodone() below), then put the
- * buf log item at the front.
- */
-void
-xfs_buf_item_init(
-	xfs_buf_t		*bp,
-	xfs_mount_t		*mp)
-{
-	xfs_log_item_t		*lip;
-	xfs_buf_log_item_t	*bip;
-
-#ifdef LI_DEBUG
-	fprintf(stderr, "buf_item_init for buffer %p\n", bp);
-#endif
-
-	/*
-	 * Check to see if there is already a buf log item for
-	 * this buffer.	 If there is, it is guaranteed to be
-	 * the first.  If we do already have one, there is
-	 * nothing to do here so return.
-	 */
-	XFS_BUF_SET_BDSTRAT_FUNC(bp, xfs_bdstrat_cb);
-	if (bp->b_log_item != NULL) {
-		lip = bp->b_log_item;
-		if (lip->li_type == XFS_LI_BUF) {
-#ifdef LI_DEBUG
-			fprintf(stderr,
-				"reused buf item %p for pre-logged buffer %p\n",
-				lip, bp);
-#endif
-			return;
-		}
-	}
-
-	bip = (xfs_buf_log_item_t *)kmem_zone_zalloc(xfs_buf_item_zone,
-						    KM_SLEEP);
-#ifdef LI_DEBUG
-	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);
-	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);
-	bp->b_log_item = bip;
-}
-
-
-/*
- * Mark bytes first through last inclusive as dirty in the buf
- * item's bitmap.
- */
-void
-xfs_buf_item_log(
-	xfs_buf_log_item_t	*bip,
-	uint			first,
-	uint			last)
-{
-	/*
-	 * Mark the item as having some dirty data for
-	 * quick reference in xfs_buf_item_dirty.
-	 */
-	bip->bli_flags |= XFS_BLI_DIRTY;
-}
-
-/*
  * Initialize the inode log item for a newly allocated (in-core) inode.
  */
 void
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 295f167..7d3899c 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -422,16 +422,6 @@ xfs_trans_roll_inode(
 	return error;
 }
 
-void
-xfs_buf_item_put(
-	struct xfs_buf_log_item	*bip)
-{
-	struct xfs_buf		*bp = bip->bli_buf;
-
-	bp->b_log_item = NULL;
-	kmem_zone_free(xfs_buf_item_zone, bip);
-}
-
 /*
  * Record the indicated change to the given field for application
  * to the file system's superblock when the transaction commits.
@@ -548,34 +538,6 @@ free:
 }
 
 static void
-buf_item_done(
-	xfs_buf_log_item_t	*bip)
-{
-	xfs_buf_t		*bp;
-	int			hold;
-	extern kmem_zone_t	*xfs_buf_item_zone;
-
-	bp = bip->bli_buf;
-	ASSERT(bp != NULL);
-	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
-		libxfs_writebuf_int(bp, 0);
-	}
-
-	bip->bli_flags &= ~XFS_BLI_HOLD;
-	xfs_buf_item_put(bip);
-	if (hold)
-		return;
-	libxfs_putbuf(bp);
-}
-
-static void
 trans_committed(
 	xfs_trans_t		*tp)
 {
@@ -597,23 +559,6 @@ trans_committed(
 }
 
 static void
-buf_item_unlock(
-	xfs_buf_log_item_t	*bip)
-{
-	xfs_buf_t		*bp = bip->bli_buf;
-	uint			hold;
-
-	/* Clear the buffer's association with this transaction. */
-	bip->bli_buf->b_transp = NULL;
-
-	hold = bip->bli_flags & XFS_BLI_HOLD;
-	bip->bli_flags &= ~XFS_BLI_HOLD;
-	xfs_buf_item_put(bip);
-	if (!hold)
-		libxfs_putbuf(bp);
-}
-
-static void
 inode_item_unlock(
 	xfs_inode_log_item_t	*iip)
 {
-- 
1.8.3.1

[PATCH 08/11] libxfs: create new file inode_item.c

[Eric Sandeen]

Pull functions out of libxfs/* into inode_item.c, if they roughly match
the kernel's xfs_inode_item.c file.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/Makefile      |   2 +-
 libxfs/inode_item.c  | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++
 libxfs/libxfs_priv.h |   3 ++
 libxfs/logitem.c     |  43 -------------------
 libxfs/trans.c       |  75 ---------------------------------
 5 files changed, 121 insertions(+), 119 deletions(-)
 create mode 100644 libxfs/inode_item.c
 delete mode 100644 libxfs/logitem.c

diff --git a/libxfs/Makefile b/libxfs/Makefile
index 820ffb0..fe58ce9 100644
--- a/libxfs/Makefile
+++ b/libxfs/Makefile
@@ -55,8 +55,8 @@ CFILES = cache.c \
 	buf_item.c \
 	defer_item.c \
 	init.c \
+	inode_item.c \
 	kmem.c \
-	logitem.c \
 	rdwr.c \
 	trans.c \
 	trans_buf.c \
diff --git a/libxfs/inode_item.c b/libxfs/inode_item.c
new file mode 100644
index 0000000..4e9b1af
--- /dev/null
+++ b/libxfs/inode_item.c
@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2000-2001,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_buf.h"
+#include "xfs_inode_fork.h"
+#include "xfs_inode.h"
+#include "xfs_trans.h"
+
+kmem_zone_t	*xfs_ili_zone;		/* inode log item zone */
+
+/*
+ * Initialize the inode log item for a newly allocated (in-core) inode.
+ */
+void
+xfs_inode_item_init(
+	xfs_inode_t		*ip,
+	xfs_mount_t		*mp)
+{
+	xfs_inode_log_item_t	*iip;
+
+	ASSERT(ip->i_itemp == NULL);
+	iip = ip->i_itemp = (xfs_inode_log_item_t *)
+			kmem_zone_zalloc(xfs_ili_zone, KM_SLEEP);
+#ifdef LI_DEBUG
+	fprintf(stderr, "inode_item_init for inode %llu, iip=%p\n",
+		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);
+	iip->ili_inode = ip;
+}
+
+static void
+xfs_inode_item_put(
+	struct xfs_inode_log_item	*iip)
+{
+	struct xfs_inode		*ip = iip->ili_inode;
+
+	ip->i_itemp = NULL;
+	kmem_zone_free(xfs_ili_zone, iip);
+}
+
+/*
+ * Transaction commital code follows (i.e. write to disk in libxfs)
+ *
+ * XXX (dgc): should failure to flush the inode (e.g. due to uncorrected
+ * corruption) result in transaction commit failure w/ EFSCORRUPTED?
+ */
+void
+inode_item_done(
+	xfs_inode_log_item_t	*iip)
+{
+	xfs_dinode_t		*dip;
+	xfs_inode_t		*ip;
+	xfs_mount_t		*mp;
+	xfs_buf_t		*bp;
+	int			error;
+
+	ip = iip->ili_inode;
+	mp = iip->ili_item.li_mountp;
+	ASSERT(ip != NULL);
+
+	if (!(iip->ili_fields & XFS_ILOG_ALL))
+		goto free;
+
+	/*
+	 * Get the buffer containing the on-disk inode.
+	 */
+	error = xfs_imap_to_bp(mp, NULL, &ip->i_imap, &dip, &bp, 0, 0);
+	if (error) {
+		fprintf(stderr, _("%s: warning - imap_to_bp failed (%d)\n"),
+			progname, error);
+		goto free;
+	}
+
+	/*
+	 * Flush the inode and disassociate it from the transaction regardless
+	 * 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);
+	bp->b_transp = NULL;	/* remove xact ptr */
+
+	if (error) {
+		fprintf(stderr, _("%s: warning - iflush_int failed (%d)\n"),
+			progname, error);
+		libxfs_putbuf(bp);
+		goto free;
+	}
+
+	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);
+}
+
+void
+inode_item_unlock(
+	xfs_inode_log_item_t    *iip)
+{
+	xfs_inode_item_put(iip);
+}
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index 155b782..cf808d3 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -491,6 +491,7 @@ struct xfs_log_item;
 struct xfs_buf;
 struct xfs_buf_map;
 struct xfs_buf_log_item;
+struct xfs_inode_log_item;
 struct xfs_buftarg;
 
 /* xfs_attr.c */
@@ -515,6 +516,8 @@ void xfs_trans_del_item(struct xfs_log_item *);
 
 /* xfs_inode_item.c */
 void xfs_inode_item_init(struct xfs_inode *, struct xfs_mount *);
+void inode_item_done(struct xfs_inode_log_item *iip);
+void inode_item_unlock(struct xfs_inode_log_item *iip);
 
 /* xfs_buf_item.c */
 void xfs_buf_item_init(struct xfs_buf *, struct xfs_mount *);
diff --git a/libxfs/logitem.c b/libxfs/logitem.c
deleted file mode 100644
index c80e2ed..0000000
--- a/libxfs/logitem.c
+++ /dev/null
@@ -1,43 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Copyright (c) 2000-2001,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_buf.h"
-#include "xfs_inode_fork.h"
-#include "xfs_inode.h"
-#include "xfs_trans.h"
-
-kmem_zone_t	*xfs_ili_zone;		/* inode log item zone */
-
-/*
- * Initialize the inode log item for a newly allocated (in-core) inode.
- */
-void
-xfs_inode_item_init(
-	xfs_inode_t		*ip,
-	xfs_mount_t		*mp)
-{
-	xfs_inode_log_item_t	*iip;
-
-	ASSERT(ip->i_itemp == NULL);
-	iip = ip->i_itemp = (xfs_inode_log_item_t *)
-			kmem_zone_zalloc(xfs_ili_zone, KM_SLEEP);
-#ifdef LI_DEBUG
-	fprintf(stderr, "inode_item_init for inode %llu, iip=%p\n",
-		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);
-	iip->ili_inode = ip;
-}
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 7d3899c..b062e07 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -470,74 +470,6 @@ _("Transaction block reservation exceeded! %u > %u\n"),
 }
 
 static void
-xfs_inode_item_put(
-	struct xfs_inode_log_item	*iip)
-{
-	struct xfs_inode		*ip = iip->ili_inode;
-
-	ip->i_itemp = NULL;
-	kmem_zone_free(xfs_ili_zone, iip);
-}
-
-
-/*
- * Transaction commital code follows (i.e. write to disk in libxfs)
- *
- * XXX (dgc): should failure to flush the inode (e.g. due to uncorrected
- * corruption) result in transaction commit failure w/ EFSCORRUPTED?
- */
-static void
-inode_item_done(
-	xfs_inode_log_item_t	*iip)
-{
-	xfs_dinode_t		*dip;
-	xfs_inode_t		*ip;
-	xfs_mount_t		*mp;
-	xfs_buf_t		*bp;
-	int			error;
-
-	ip = iip->ili_inode;
-	mp = iip->ili_item.li_mountp;
-	ASSERT(ip != NULL);
-
-	if (!(iip->ili_fields & XFS_ILOG_ALL))
-		goto free;
-
-	/*
-	 * Get the buffer containing the on-disk inode.
-	 */
-	error = xfs_imap_to_bp(mp, NULL, &ip->i_imap, &dip, &bp, 0, 0);
-	if (error) {
-		fprintf(stderr, _("%s: warning - imap_to_bp failed (%d)\n"),
-			progname, error);
-		goto free;
-	}
-
-	/*
-	 * Flush the inode and disassociate it from the transaction regardless
-	 * 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);
-	bp->b_transp = NULL;	/* remove xact ptr */
-
-	if (error) {
-		fprintf(stderr, _("%s: warning - iflush_int failed (%d)\n"),
-			progname, error);
-		libxfs_putbuf(bp);
-		goto free;
-	}
-
-	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);
-}
-
-static void
 trans_committed(
 	xfs_trans_t		*tp)
 {
@@ -558,13 +490,6 @@ trans_committed(
 	}
 }
 
-static void
-inode_item_unlock(
-	xfs_inode_log_item_t	*iip)
-{
-	xfs_inode_item_put(iip);
-}
-
 /* Detach and unlock all of the items in a transaction */
 static void
 xfs_trans_free_items(
-- 
1.8.3.1

[PATCH 09/11] libxfs: create new file trans_inode.c

[Eric Sandeen]

Pull functions out of libxfs/trans.c into trans_inode.c, if they roughly
match the kernel's xfs_trans_inode.c file.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/Makefile      |   1 +
 libxfs/trans.c       |  86 -----------------------------------------
 libxfs/trans_inode.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 107 insertions(+), 86 deletions(-)
 create mode 100644 libxfs/trans_inode.c

diff --git a/libxfs/Makefile b/libxfs/Makefile
index fe58ce9..82544d2 100644
--- a/libxfs/Makefile
+++ b/libxfs/Makefile
@@ -60,6 +60,7 @@ CFILES = cache.c \
 	rdwr.c \
 	trans.c \
 	trans_buf.c \
+	trans_inode.c \
 	util.c \
 	xfs_ag.c \
 	xfs_ag_resv.c \
diff --git a/libxfs/trans.c b/libxfs/trans.c
index b062e07..f199d15 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -336,92 +336,6 @@ out:
 	return;
 }
 
-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, (xfs_log_item_t *)(iip));
-
-#ifdef XACT_DEBUG
-	fprintf(stderr, "ijoin'd inode %llu, transaction %p\n", ip->i_ino, tp);
-#endif
-}
-
-void
-xfs_trans_ijoin_ref(
-	xfs_trans_t		*tp,
-	xfs_inode_t		*ip,
-	int			lock_flags)
-{
-	ASSERT(ip->i_itemp != NULL);
-
-	xfs_trans_ijoin(tp, ip, lock_flags);
-
-#ifdef XACT_DEBUG
-	fprintf(stderr, "ijoin_ref'd inode %llu, transaction %p\n", ip->i_ino, tp);
-#endif
-}
-
-/*
- * 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);
-#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);
-
-	/*
-	 * 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;
-}
-
 /*
  * Record the indicated change to the given field for application
  * to the file system's superblock when the transaction commits.
diff --git a/libxfs/trans_inode.c b/libxfs/trans_inode.c
new file mode 100644
index 0000000..cfca147
--- /dev/null
+++ b/libxfs/trans_inode.c
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2000-2001,2005-2006 Silicon Graphics, Inc.
+ * Copyright (C) 2010 Red Hat, 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_buf.h"
+#include "xfs_inode_fork.h"
+#include "xfs_inode.h"
+#include "xfs_trans.h"
+#include "xfs_sb.h"
+#include "xfs_defer.h"
+
+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, (xfs_log_item_t *)(iip));
+
+#ifdef XACT_DEBUG
+	fprintf(stderr, "ijoin'd inode %llu, transaction %p\n", ip->i_ino, tp);
+#endif
+}
+
+void
+xfs_trans_ijoin_ref(
+	xfs_trans_t		*tp,
+	xfs_inode_t		*ip,
+	int			lock_flags)
+{
+	ASSERT(ip->i_itemp != NULL);
+
+	xfs_trans_ijoin(tp, ip, lock_flags);
+
+#ifdef XACT_DEBUG
+	fprintf(stderr, "ijoin_ref'd inode %llu, transaction %p\n", ip->i_ino, tp);
+#endif
+}
+
+/*
+ * 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);
+#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);
+
+	/*
+	 * 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;
+}
-- 
1.8.3.1

[PATCH 10/11] libxfs: reorder trans.c to match xfs_trans.c

[Eric Sandeen]

Reorder the functions in libxfs/trans.c to more closely match
the order in the kernel's xfs_trans.c.  It's not 1:1 but much
closer now, and the forward declarations can be removed as well.

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

diff --git a/libxfs/trans.c b/libxfs/trans.c
index f199d15..4d83ec5 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -12,19 +12,10 @@
 #include "xfs_log_format.h"
 #include "xfs_trans_resv.h"
 #include "xfs_mount.h"
-#include "xfs_inode_buf.h"
-#include "xfs_inode_fork.h"
-#include "xfs_inode.h"
 #include "xfs_trans.h"
 #include "xfs_sb.h"
 #include "xfs_defer.h"
 
-static void xfs_trans_free_items(struct xfs_trans *tp);
-STATIC struct xfs_trans *xfs_trans_dup(struct xfs_trans *tp);
-static int xfs_trans_reserve(struct xfs_trans *tp, struct xfs_trans_res *resp,
-		uint blocks, uint rtextents);
-static int __xfs_trans_commit(struct xfs_trans *tp, bool regrant);
-
 /*
  * Simple transaction interface
  */
@@ -43,79 +34,6 @@ xfs_trans_init(
 }
 
 /*
- * Add the given log item to the transaction's list of log items.
- */
-void
-xfs_trans_add_item(
-	struct xfs_trans	*tp,
-	struct xfs_log_item	*lip)
-{
-	ASSERT(lip->li_mountp == tp->t_mountp);
-	ASSERT(lip->li_ailp == tp->t_mountp->m_ail);
-	ASSERT(list_empty(&lip->li_trans));
-	ASSERT(!test_bit(XFS_LI_DIRTY, &lip->li_flags));
-
-	list_add_tail(&lip->li_trans, &tp->t_items);
-}
-
-/*
- * Unlink and free the given descriptor.
- */
-void
-xfs_trans_del_item(
-	struct xfs_log_item	*lip)
-{
-	clear_bit(XFS_LI_DIRTY, &lip->li_flags);
-	list_del_init(&lip->li_trans);
-}
-
-/*
- * Roll from one trans in the sequence of PERMANENT transactions to
- * the next: permanent transactions are only flushed out when
- * committed with XFS_TRANS_RELEASE_LOG_RES, but we still want as soon
- * as possible to let chunks of it go to the log. So we commit the
- * chunk we've been working on and get a new transaction to continue.
- */
-int
-xfs_trans_roll(
-	struct xfs_trans	**tpp)
-{
-	struct xfs_trans	*trans = *tpp;
-	struct xfs_trans_res	tres;
-	int			error;
-
-	/*
-	 * Copy the critical parameters from one trans to the next.
-	 */
-	tres.tr_logres = trans->t_log_res;
-	tres.tr_logcount = trans->t_log_count;
-
-	*tpp = xfs_trans_dup(trans);
-
-	/*
-	 * Commit the current transaction.
-	 * If this commit failed, then it'd just unlock those items that
-	 * are marked to be released. That also means that a filesystem shutdown
-	 * is in progress. The caller takes the responsibility to cancel
-	 * the duplicate transaction that gets returned.
-	 */
-	error = __xfs_trans_commit(trans, true);
-	if (error)
-		return error;
-
-	/*
-	 * Reserve space in the log for the next transaction.
-	 * This also pushes items in the "AIL", the list of logged items,
-	 * out to disk if they are taking up space at the tail of the log
-	 * that we want to use.  This requires that either nothing be locked
-	 * across this call, or that anything that is locked be logged in
-	 * the prior and the next transactions.
-	 */
-	tres.tr_logflags = XFS_TRANS_PERM_LOG_RES;
-	return xfs_trans_reserve(*tpp, &tres, 0, 0);
-}
-
-/*
  * Free the transaction structure.  If there is more clean up
  * to do when the structure is freed, add it here.
  */
@@ -277,6 +195,21 @@ xfs_trans_alloc(
 }
 
 /*
+ * Allocate a transaction that can be rolled.  Since userspace doesn't have
+ * a need for log reservations, we really only tr_itruncate to get the
+ * permanent log reservation flag to avoid blowing asserts.
+ */
+int
+libxfs_trans_alloc_rollable(
+	struct xfs_mount	*mp,
+	unsigned int		blocks,
+	struct xfs_trans	**tpp)
+{
+	return xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, blocks,
+			0, 0, tpp);
+}
+
+/*
  * Create an empty transaction with no reservation.  This is a defensive
  * mechanism for routines that query metadata without actually modifying
  * them -- if the metadata being queried is somehow cross-linked (think a
@@ -299,44 +232,6 @@ xfs_trans_alloc_empty(
 }
 
 /*
- * Allocate a transaction that can be rolled.  Since userspace doesn't have
- * a need for log reservations, we really only tr_itruncate to get the
- * permanent log reservation flag to avoid blowing asserts.
- */
-int
-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,
-			0, 0, tpp);
-}
-
-void
-xfs_trans_cancel(
-	struct xfs_trans	*tp)
-{
-#ifdef XACT_DEBUG
-	struct xfs_trans	*otp = tp;
-#endif
-	if (tp == NULL)
-		goto out;
-
-	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;
-}
-
-/*
  * Record the indicated change to the given field for application
  * to the file system's superblock when the transaction commits.
  * For now, just store the change in the transaction structure.
@@ -383,19 +278,46 @@ _("Transaction block reservation exceeded! %u > %u\n"),
 	tp->t_flags |= (XFS_TRANS_SB_DIRTY | XFS_TRANS_DIRTY);
 }
 
+/*
+ * Add the given log item to the transaction's list of log items.
+ */
+void
+xfs_trans_add_item(
+	struct xfs_trans	*tp,
+	struct xfs_log_item	*lip)
+{
+	ASSERT(lip->li_mountp == tp->t_mountp);
+	ASSERT(lip->li_ailp == tp->t_mountp->m_ail);
+	ASSERT(list_empty(&lip->li_trans));
+	ASSERT(!test_bit(XFS_LI_DIRTY, &lip->li_flags));
+
+	list_add_tail(&lip->li_trans, &tp->t_items);
+}
+
+/*
+ * Unlink and free the given descriptor.
+ */
+void
+xfs_trans_del_item(
+	struct xfs_log_item	*lip)
+{
+	clear_bit(XFS_LI_DIRTY, &lip->li_flags);
+	list_del_init(&lip->li_trans);
+}
+
+/* Detach and unlock all of the items in a transaction */
 static void
-trans_committed(
-	xfs_trans_t		*tp)
+xfs_trans_free_items(
+	struct xfs_trans	*tp)
 {
 	struct xfs_log_item	*lip, *next;
 
 	list_for_each_entry_safe(lip, next, &tp->t_items, li_trans) {
 		xfs_trans_del_item(lip);
-
 		if (lip->li_type == XFS_LI_BUF)
-			buf_item_done((xfs_buf_log_item_t *)lip);
+			buf_item_unlock((xfs_buf_log_item_t *)lip);
 		else if (lip->li_type == XFS_LI_INODE)
-			inode_item_done((xfs_inode_log_item_t *)lip);
+			inode_item_unlock((xfs_inode_log_item_t *)lip);
 		else {
 			fprintf(stderr, _("%s: unrecognised log item type\n"),
 				progname);
@@ -404,19 +326,19 @@ trans_committed(
 	}
 }
 
-/* Detach and unlock all of the items in a transaction */
 static void
-xfs_trans_free_items(
-	struct xfs_trans	*tp)
+trans_committed(
+	xfs_trans_t		*tp)
 {
 	struct xfs_log_item	*lip, *next;
 
 	list_for_each_entry_safe(lip, next, &tp->t_items, li_trans) {
 		xfs_trans_del_item(lip);
+
 		if (lip->li_type == XFS_LI_BUF)
-			buf_item_unlock((xfs_buf_log_item_t *)lip);
+			buf_item_done((xfs_buf_log_item_t *)lip);
 		else if (lip->li_type == XFS_LI_INODE)
-			inode_item_unlock((xfs_inode_log_item_t *)lip);
+			inode_item_done((xfs_inode_log_item_t *)lip);
 		else {
 			fprintf(stderr, _("%s: unrecognised log item type\n"),
 				progname);
@@ -492,3 +414,72 @@ xfs_trans_commit(
 {
 	return __xfs_trans_commit(tp, false);
 }
+
+void
+xfs_trans_cancel(
+	struct xfs_trans	*tp)
+{
+#ifdef XACT_DEBUG
+	struct xfs_trans	*otp = tp;
+#endif
+	if (tp == NULL)
+		goto out;
+
+	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;
+}
+
+/*
+ * Roll from one trans in the sequence of PERMANENT transactions to
+ * the next: permanent transactions are only flushed out when
+ * committed with XFS_TRANS_RELEASE_LOG_RES, but we still want as soon
+ * as possible to let chunks of it go to the log. So we commit the
+ * chunk we've been working on and get a new transaction to continue.
+ */
+int
+xfs_trans_roll(
+	struct xfs_trans	**tpp)
+{
+	struct xfs_trans	*trans = *tpp;
+	struct xfs_trans_res	tres;
+	int			error;
+
+	/*
+	 * Copy the critical parameters from one trans to the next.
+	 */
+	tres.tr_logres = trans->t_log_res;
+	tres.tr_logcount = trans->t_log_count;
+
+	*tpp = xfs_trans_dup(trans);
+
+	/*
+	 * Commit the current transaction.
+	 * If this commit failed, then it'd just unlock those items that
+	 * are marked to be released. That also means that a filesystem shutdown
+	 * is in progress. The caller takes the responsibility to cancel
+	 * the duplicate transaction that gets returned.
+	 */
+	error = __xfs_trans_commit(trans, true);
+	if (error)
+		return error;
+
+	/*
+	 * Reserve space in the log for the next transaction.
+	 * This also pushes items in the "AIL", the list of logged items,
+	 * out to disk if they are taking up space at the tail of the log
+	 * that we want to use.  This requires that either nothing be locked
+	 * across this call, or that anything that is locked be logged in
+	 * the prior and the next transactions.
+	 */
+	tres.tr_logflags = XFS_TRANS_PERM_LOG_RES;
+	return xfs_trans_reserve(*tpp, &tres, 0, 0);
+}
-- 
1.8.3.1

[PATCH 11/11] libxfs: minor sync-ups to kernel-ish functions

[Eric Sandeen]

Change typedefs to structs, add comments, and other very
minor changes to userspace libxfs/ functions so that they
more closely match kernelspace.  Should be no functional
changes.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/buf_item.c    |  13 +---
 libxfs/inode_item.c  |   9 +--
 libxfs/libxfs_priv.h |   4 -
 libxfs/trans.c       |  80 ++++++++++++--------
 libxfs/trans_buf.c   | 205 ++++++++++++++++++++++++++++++++++++++-------------
 5 files changed, 211 insertions(+), 100 deletions(-)

diff --git a/libxfs/buf_item.c b/libxfs/buf_item.c
index 2e64c8c..30e9609 100644
--- a/libxfs/buf_item.c
+++ b/libxfs/buf_item.c
@@ -16,10 +16,6 @@
 
 kmem_zone_t	*xfs_buf_item_zone;
 
-/*
- * The following are from fs/xfs/xfs_buf_item.c
- */
-
 void
 xfs_buf_item_put(
 	struct xfs_buf_log_item	*bip)
@@ -56,8 +52,8 @@ buf_item_unlock(
  */
 void
 xfs_buf_item_init(
-	xfs_buf_t		*bp,
-	xfs_mount_t		*mp)
+	struct xfs_buf		*bp,
+	struct xfs_mount	*mp)
 {
 	xfs_log_item_t		*lip;
 	xfs_buf_log_item_t	*bip;
@@ -85,8 +81,7 @@ xfs_buf_item_init(
 		}
 	}
 
-	bip = (xfs_buf_log_item_t *)kmem_zone_zalloc(xfs_buf_item_zone,
-						    KM_SLEEP);
+	bip = kmem_zone_zalloc(xfs_buf_item_zone, KM_SLEEP);
 #ifdef LI_DEBUG
 	fprintf(stderr, "adding buf item %p for not-logged buffer %p\n",
 		bip, bp);
@@ -107,7 +102,7 @@ xfs_buf_item_init(
  */
 void
 xfs_buf_item_log(
-	xfs_buf_log_item_t	*bip,
+	struct xfs_buf_log_item	*bip,
 	uint			first,
 	uint			last)
 {
diff --git a/libxfs/inode_item.c b/libxfs/inode_item.c
index 4e9b1af..e3d4ff9 100644
--- a/libxfs/inode_item.c
+++ b/libxfs/inode_item.c
@@ -23,14 +23,13 @@ kmem_zone_t	*xfs_ili_zone;		/* inode log item zone */
  */
 void
 xfs_inode_item_init(
-	xfs_inode_t		*ip,
-	xfs_mount_t		*mp)
+	struct xfs_inode	*ip,
+	struct xfs_mount	*mp)
 {
-	xfs_inode_log_item_t	*iip;
+	struct xfs_inode_log_item *iip;
 
 	ASSERT(ip->i_itemp == NULL);
-	iip = ip->i_itemp = (xfs_inode_log_item_t *)
-			kmem_zone_zalloc(xfs_ili_zone, KM_SLEEP);
+	iip = ip->i_itemp = kmem_zone_zalloc(xfs_ili_zone, KM_SLEEP);
 #ifdef LI_DEBUG
 	fprintf(stderr, "inode_item_init for inode %llu, iip=%p\n",
 		ip->i_ino, iip);
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index cf808d3..2b7d8c5 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -526,10 +526,6 @@ void xfs_buf_item_put(struct xfs_buf_log_item *bip);
 void buf_item_unlock(struct xfs_buf_log_item *bip);
 void buf_item_done(struct xfs_buf_log_item *bip);
 
-/* xfs_trans_buf.c */
-struct xfs_buf *xfs_trans_buf_item_match(struct xfs_trans *,
-			struct xfs_buftarg *, struct xfs_buf_map *, int);
-
 /* local source files */
 #define xfs_mod_fdblocks(mp, delta, rsvd) \
 	libxfs_mod_incore_sb(mp, XFS_TRANS_SB_FDBLOCKS, delta, rsvd)
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 4d83ec5..831cfa1 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -16,10 +16,6 @@
 #include "xfs_sb.h"
 #include "xfs_defer.h"
 
-/*
- * Simple transaction interface
- */
-
 kmem_zone_t	*xfs_trans_zone;
 
 /*
@@ -30,14 +26,14 @@ void
 xfs_trans_init(
 	struct xfs_mount	*mp)
 {
-	xfs_trans_resv_calc(mp, &mp->m_resv);
+	xfs_trans_resv_calc(mp, M_RES(mp));
 }
 
 /*
  * Free the transaction structure.  If there is more clean up
  * to do when the structure is freed, add it here.
  */
-static void
+STATIC void
 xfs_trans_free(
 	struct xfs_trans	*tp)
 {
@@ -106,7 +102,7 @@ xfs_trans_reserve(
 	uint			blocks,
 	uint			rtextents)
 {
-	int			error = 0;
+	int		error = 0;
 
 	/*
 	 * Attempt to reserve the needed disk blocks by decrementing
@@ -114,8 +110,9 @@ xfs_trans_reserve(
 	 * fail if the count would go below zero.
 	 */
 	if (blocks > 0) {
-		if (tp->t_mountp->m_sb.sb_fdblocks < blocks)
+		if (tp->t_mountp->m_sb.sb_fdblocks < blocks) {
 			return -ENOSPC;
+		}
 		tp->t_blk_res += blocks;
 	}
 
@@ -128,10 +125,11 @@ xfs_trans_reserve(
 		ASSERT(tp->t_log_count == 0 ||
 		       tp->t_log_count == resp->tr_logcount);
 
-		if (resp->tr_logflags & XFS_TRANS_PERM_LOG_RES)
+		if (resp->tr_logflags & XFS_TRANS_PERM_LOG_RES) {
 			tp->t_flags |= XFS_TRANS_PERM_LOG_RES;
-		else
+		} else {
 			ASSERT(!(tp->t_flags & XFS_TRANS_PERM_LOG_RES));
+		}
 
 		tp->t_log_res = resp->tr_logres;
 		tp->t_log_count = resp->tr_logcount;
@@ -156,8 +154,9 @@ xfs_trans_reserve(
 	 * reservations which have already been performed.
 	 */
 undo_blocks:
-	if (blocks > 0)
+	if (blocks > 0) {
 		tp->t_blk_res = 0;
+	}
 
 	return error;
 }
@@ -166,11 +165,10 @@ int
 xfs_trans_alloc(
 	struct xfs_mount	*mp,
 	struct xfs_trans_res	*resp,
-	unsigned int		blocks,
-	unsigned int		rtextents,
-	unsigned int		flags,
+	uint			blocks,
+	uint			rtextents,
+	uint			flags,
 	struct xfs_trans	**tpp)
-
 {
 	struct xfs_trans	*tp;
 	int			error;
@@ -235,18 +233,24 @@ xfs_trans_alloc_empty(
  * Record the indicated change to the given field for application
  * to the file system's superblock when the transaction commits.
  * For now, just store the change in the transaction structure.
+ *
  * Mark the transaction structure to indicate that the superblock
  * needs to be updated before committing.
- *
- * Originally derived from xfs_trans_mod_sb().
  */
 void
 xfs_trans_mod_sb(
-	xfs_trans_t		*tp,
-	uint			field,
-	long			delta)
+	xfs_trans_t	*tp,
+	uint		field,
+	int64_t		delta)
 {
 	switch (field) {
+	case XFS_TRANS_SB_ICOUNT:
+		ASSERT(delta > 0);
+		tp->t_icount_delta += delta;
+		break;
+	case XFS_TRANS_SB_IFREE:
+		tp->t_ifree_delta += delta;
+		break;
 	case XFS_TRANS_SB_RES_FDBLOCKS:
 		return;
 	case XFS_TRANS_SB_FDBLOCKS:
@@ -261,13 +265,6 @@ _("Transaction block reservation exceeded! %u > %u\n"),
 		}
 		tp->t_fdblocks_delta += delta;
 		break;
-	case XFS_TRANS_SB_ICOUNT:
-		ASSERT(delta > 0);
-		tp->t_icount_delta += delta;
-		break;
-	case XFS_TRANS_SB_IFREE:
-		tp->t_ifree_delta += delta;
-		break;
 	case XFS_TRANS_SB_FREXTENTS:
 		tp->t_frextents_delta += delta;
 		break;
@@ -295,7 +292,9 @@ xfs_trans_add_item(
 }
 
 /*
- * Unlink and free the given descriptor.
+ * Unlink the log item from the transaction. the log item is no longer
+ * considered dirty in this transaction, as the linked transaction has
+ * finished, either by abort or commit completion.
  */
 void
 xfs_trans_del_item(
@@ -306,7 +305,7 @@ xfs_trans_del_item(
 }
 
 /* Detach and unlock all of the items in a transaction */
-static void
+void
 xfs_trans_free_items(
 	struct xfs_trans	*tp)
 {
@@ -373,6 +372,13 @@ __xfs_trans_commit(
 			goto out_unreserve;
 	}
 
+	/*
+	 * If there is nothing to be logged by the transaction,
+	 * then unlock all of the items associated with the
+	 * transaction and free the transaction structure.
+	 * Also make sure to return any reserved blocks to
+	 * the free pool.
+	 */
 	if (!(tp->t_flags & XFS_TRANS_DIRTY)) {
 #ifdef XACT_DEBUG
 		fprintf(stderr, "committed clean transaction %p\n", tp);
@@ -380,6 +386,9 @@ __xfs_trans_commit(
 		goto out_unreserve;
 	}
 
+	/*
+	 * If we need to update the superblock, then do it now.
+	 */
 	if (tp->t_flags & XFS_TRANS_SB_DIRTY) {
 		sbp = &(tp->t_mountp->m_sb);
 		if (tp->t_icount_delta)
@@ -405,6 +414,7 @@ __xfs_trans_commit(
 out_unreserve:
 	xfs_trans_free_items(tp);
 	xfs_trans_free(tp);
+
 	return error;
 }
 
@@ -415,6 +425,14 @@ xfs_trans_commit(
 	return __xfs_trans_commit(tp, false);
 }
 
+/*
+ * Unlock all of the transaction's items and free the transaction.
+ * The transaction must not have modified any of its items, because
+ * there is no way to restore them to their previous state.
+ *
+ * If the transaction has made a log reservation, make sure to release
+ * it as well.
+ */
 void
 xfs_trans_cancel(
 	struct xfs_trans	*tp)
@@ -441,7 +459,7 @@ out:
 /*
  * Roll from one trans in the sequence of PERMANENT transactions to
  * the next: permanent transactions are only flushed out when
- * committed with XFS_TRANS_RELEASE_LOG_RES, but we still want as soon
+ * committed with xfs_trans_commit(), but we still want as soon
  * as possible to let chunks of it go to the log. So we commit the
  * chunk we've been working on and get a new transaction to continue.
  */
@@ -464,7 +482,7 @@ xfs_trans_roll(
 	/*
 	 * Commit the current transaction.
 	 * If this commit failed, then it'd just unlock those items that
-	 * are marked to be released. That also means that a filesystem shutdown
+	 * are not marked ihold. That also means that a filesystem shutdown
 	 * is in progress. The caller takes the responsibility to cancel
 	 * the duplicate transaction that gets returned.
 	 */
diff --git a/libxfs/trans_buf.c b/libxfs/trans_buf.c
index 9c2c36e..6ce3b1d 100644
--- a/libxfs/trans_buf.c
+++ b/libxfs/trans_buf.c
@@ -4,7 +4,6 @@
  * Copyright (C) 2010 Red Hat, Inc.
  * All Rights Reserved.
  */
-
 #include "libxfs_priv.h"
 #include "xfs_fs.h"
 #include "xfs_shared.h"
@@ -17,22 +16,18 @@
 #include "xfs_sb.h"
 
 /*
- * Following functions from fs/xfs/xfs_trans_buf.c
- */
-
-/*
  * Check to see if a buffer matching the given parameters is already
  * a part of the given transaction.
  */
-xfs_buf_t *
+STATIC struct xfs_buf *
 xfs_trans_buf_item_match(
-	xfs_trans_t		*tp,
-	struct xfs_buftarg	*btp,
+	struct xfs_trans	*tp,
+	struct xfs_buftarg	*target,
 	struct xfs_buf_map	*map,
 	int			nmaps)
 {
 	struct xfs_log_item	*lip;
-	struct xfs_buf_log_item *blip;
+	struct xfs_buf_log_item	*blip;
 	int			len = 0;
 	int			i;
 
@@ -42,7 +37,7 @@ xfs_trans_buf_item_match(
 	list_for_each_entry(lip, &tp->t_items, li_trans) {
 		blip = (struct xfs_buf_log_item *)lip;
 		if (blip->bli_item.li_type == XFS_LI_BUF &&
-		    blip->bli_buf->b_target->dev == btp->dev &&
+		    blip->bli_buf->b_target->dev == target->dev &&
 		    XFS_BUF_ADDR(blip->bli_buf) == map[0].bm_bn &&
 		    blip->bli_buf->b_bcount == BBTOB(len)) {
 			ASSERT(blip->bli_buf->b_map_count == nmaps);
@@ -55,10 +50,10 @@ xfs_trans_buf_item_match(
 
 void
 xfs_trans_bjoin(
-	xfs_trans_t		*tp,
-	xfs_buf_t		*bp)
+	struct xfs_trans	*tp,
+	struct xfs_buf		*bp)
 {
-	xfs_buf_log_item_t	*bip;
+	struct xfs_buf_log_item	*bip;
 
 	ASSERT(bp->b_transp == NULL);
 #ifdef XACT_DEBUG
@@ -71,21 +66,36 @@ xfs_trans_bjoin(
 	bp->b_transp = tp;
 }
 
-xfs_buf_t *
+/*
+ * Get and lock the buffer for the caller if it is not already
+ * locked within the given transaction.  If it is already locked
+ * within the transaction, just increment its lock recursion count
+ * and return a pointer to it.
+ *
+ * If the transaction pointer is NULL, make this just a normal
+ * get_buf() call.
+ */
+struct xfs_buf *
 xfs_trans_get_buf_map(
-	xfs_trans_t		*tp,
-	struct xfs_buftarg	*btp,
+	struct xfs_trans	*tp,
+	struct xfs_buftarg	*target,
 	struct xfs_buf_map	*map,
 	int			nmaps,
-	uint			f)
+	uint			flags)
 {
 	xfs_buf_t		*bp;
-	xfs_buf_log_item_t	*bip;
-
-	if (tp == NULL)
-		return libxfs_getbuf_map(btp, map, nmaps, 0);
-
-	bp = xfs_trans_buf_item_match(tp, btp, map, nmaps);
+	struct xfs_buf_log_item	*bip;
+
+	if (!tp)
+		return libxfs_getbuf_map(target, map, nmaps, 0);
+
+	/*
+	 * If we find the buffer in the cache with this transaction
+	 * pointer in its b_fsprivate2 field, then we know we already
+	 * have it locked.  In this case we just increment the lock
+	 * recursion count and return the buffer to the caller.
+	 */
+	bp = xfs_trans_buf_item_match(tp, target, map, nmaps);
 	if (bp != NULL) {
 		ASSERT(bp->b_transp == tp);
 		bip = bp->b_log_item;
@@ -94,9 +104,10 @@ xfs_trans_get_buf_map(
 		return bp;
 	}
 
-	bp = libxfs_getbuf_map(btp, map, nmaps, 0);
-	if (bp == NULL)
+	bp = libxfs_getbuf_map(target, map, nmaps, 0);
+	if (bp == NULL) {
 		return NULL;
+	}
 #ifdef XACT_DEBUG
 	fprintf(stderr, "trans_get_buf buffer %p, transaction %p\n", bp, tp);
 #endif
@@ -107,14 +118,22 @@ xfs_trans_get_buf_map(
 	return bp;
 }
 
+/*
+ * Get and lock the superblock buffer of this file system for the
+ * given transaction.
+ *
+ * We don't need to use incore_match() here, because the superblock
+ * buffer is a private buffer which we keep a pointer to in the
+ * mount structure.
+ */
 xfs_buf_t *
 xfs_trans_getsb(
 	xfs_trans_t		*tp,
-	xfs_mount_t		*mp,
+	struct xfs_mount	*mp,
 	int			flags)
 {
 	xfs_buf_t		*bp;
-	xfs_buf_log_item_t	*bip;
+	struct xfs_buf_log_item	*bip;
 	int			len = XFS_FSS_TO_BB(mp, 1);
 	DEFINE_SINGLE_BUF_MAP(map, XFS_SB_DADDR, len);
 
@@ -141,25 +160,35 @@ xfs_trans_getsb(
 	return bp;
 }
 
+/*
+ * Get and lock the buffer for the caller if it is not already
+ * locked within the given transaction.  If it has not yet been
+ * read in, read it from disk. If it is already locked
+ * within the transaction and already read in, just increment its
+ * lock recursion count and return a pointer to it.
+ *
+ * If the transaction pointer is NULL, make this just a normal
+ * read_buf() call.
+ */
 int
 xfs_trans_read_buf_map(
-	xfs_mount_t		*mp,
-	xfs_trans_t		*tp,
-	struct xfs_buftarg	*btp,
+	struct xfs_mount	*mp,
+	struct xfs_trans	*tp,
+	struct xfs_buftarg	*target,
 	struct xfs_buf_map	*map,
 	int			nmaps,
 	uint			flags,
-	xfs_buf_t		**bpp,
+	struct xfs_buf		**bpp,
 	const struct xfs_buf_ops *ops)
 {
-	xfs_buf_t		*bp;
-	xfs_buf_log_item_t	*bip;
+	struct xfs_buf		*bp;
+	struct xfs_buf_log_item	*bip;
 	int			error;
 
 	*bpp = NULL;
 
 	if (tp == NULL) {
-		bp = libxfs_readbuf_map(btp, map, nmaps, flags, ops);
+		bp = libxfs_readbuf_map(target, map, nmaps, flags, ops);
 		if (!bp) {
 			return (flags & XBF_TRYLOCK) ?  -EAGAIN : -ENOMEM;
 		}
@@ -168,7 +197,7 @@ xfs_trans_read_buf_map(
 		goto done;
 	}
 
-	bp = xfs_trans_buf_item_match(tp, btp, map, nmaps);
+	bp = xfs_trans_buf_item_match(tp, target, map, nmaps);
 	if (bp != NULL) {
 		ASSERT(bp->b_transp == tp);
 		ASSERT(bp->b_log_item != NULL);
@@ -177,7 +206,7 @@ xfs_trans_read_buf_map(
 		goto done;
 	}
 
-	bp = libxfs_readbuf_map(btp, map, nmaps, flags, ops);
+	bp = libxfs_readbuf_map(target, map, nmaps, flags, ops);
 	if (!bp) {
 		return (flags & XBF_TRYLOCK) ?  -EAGAIN : -ENOMEM;
 	}
@@ -200,47 +229,80 @@ out_relse:
 	return error;
 }
 
+/*
+ * Release a buffer previously joined to the transaction. If the buffer is
+ * modified within this transaction, decrement the recursion count but do not
+ * release the buffer even if the count goes to 0. If the buffer is not modified
+ * within the transaction, decrement the recursion count and release the buffer
+ * if the recursion count goes to 0.
+ *
+ * If the buffer is to be released and it was not already dirty before this
+ * transaction began, then also free the buf_log_item associated with it.
+ *
+ * If the transaction pointer is NULL, this is a normal xfs_buf_relse() call.
+ */
 void
 xfs_trans_brelse(
-	xfs_trans_t		*tp,
-	xfs_buf_t		*bp)
+	struct xfs_trans	*tp,
+	struct xfs_buf		*bp)
 {
-	xfs_buf_log_item_t	*bip;
+	struct xfs_buf_log_item	*bip = bp->b_log_item;
 #ifdef XACT_DEBUG
 	fprintf(stderr, "released buffer %p, transaction %p\n", bp, tp);
 #endif
 
-	if (tp == NULL) {
+	if (!tp) {
 		ASSERT(bp->b_transp == NULL);
 		libxfs_putbuf(bp);
 		return;
 	}
 	ASSERT(bp->b_transp == tp);
-	bip = bp->b_log_item;
 	ASSERT(bip->bli_item.li_type == XFS_LI_BUF);
+
+	/*
+	 * If the release is for a recursive lookup, then decrement the count
+	 * and return.
+	 */
 	if (bip->bli_recur > 0) {
 		bip->bli_recur--;
 		return;
 	}
-	/* If dirty/stale, can't release till transaction committed */
-	if (bip->bli_flags & XFS_BLI_STALE)
-		return;
+
+	/*
+	 * If the buffer is invalidated or dirty in this transaction, we can't
+	 * release it until we commit.
+	 */
 	if (test_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags))
 		return;
+	if (bip->bli_flags & XFS_BLI_STALE)
+		return;
+
+	/*
+	 * Unlink the log item from the transaction and clear the hold flag, if
+	 * set. We wouldn't want the next user of the buffer to get confused.
+	 */
 	xfs_trans_del_item(&bip->bli_item);
-	if (bip->bli_flags & XFS_BLI_HOLD)
-		bip->bli_flags &= ~XFS_BLI_HOLD;
+	bip->bli_flags &= ~XFS_BLI_HOLD;
+
+	/* drop the reference to the bli */
 	xfs_buf_item_put(bip);
+
 	bp->b_transp = NULL;
 	libxfs_putbuf(bp);
 }
 
+/*
+ * Mark the buffer as not needing to be unlocked when the buf item's
+ * iop_unlock() routine is called.  The buffer msut already be locked
+ * and associated with the given transaction.
+ */
+/* ARGSUSED */
 void
 xfs_trans_bhold(
 	xfs_trans_t		*tp,
 	xfs_buf_t		*bp)
 {
-	xfs_buf_log_item_t	*bip = bp->b_log_item;
+	struct xfs_buf_log_item	*bip = bp->b_log_item;
 
 	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
@@ -289,18 +351,48 @@ xfs_trans_log_buf(
 {
 	struct xfs_buf_log_item	*bip = bp->b_log_item;
 
-	ASSERT((first <= last) && (last < bp->b_bcount));
+	ASSERT(first <= last && last < bp->b_bcount);
 
 	xfs_trans_dirty_buf(tp, bp);
 	xfs_buf_item_log(bip, first, last);
 }
 
+
+/*
+ * Invalidate a buffer that is being used within a transaction.
+ *
+ * Typically this is because the blocks in the buffer are being freed, so we
+ * need to prevent it from being written out when we're done.  Allowing it
+ * to be written again might overwrite data in the free blocks if they are
+ * reallocated to a file.
+ *
+ * We prevent the buffer from being written out by marking it stale.  We can't
+ * get rid of the buf log item at this point because the buffer may still be
+ * pinned by another transaction.  If that is the case, then we'll wait until
+ * the buffer is committed to disk for the last time (we can tell by the ref
+ * count) and free it in xfs_buf_item_unpin().  Until that happens we will
+ * keep the buffer locked so that the buffer and buf log item are not reused.
+ *
+ * We also set the XFS_BLF_CANCEL flag in the buf log format structure and log
+ * the buf item.  This will be used at recovery time to determine that copies
+ * of the buffer in the log before this should not be replayed.
+ *
+ * We mark the item descriptor and the transaction dirty so that we'll hold
+ * the buffer until after the commit.
+ *
+ * Since we're invalidating the buffer, we also clear the state about which
+ * parts of the buffer have been logged.  We also clear the flag indicating
+ * that this is an inode buffer since the data in the buffer will no longer
+ * be valid.
+ *
+ * We set the stale bit in the buffer as well since we're getting rid of it.
+ */
 void
 xfs_trans_binval(
 	xfs_trans_t		*tp,
 	xfs_buf_t		*bp)
 {
-	xfs_buf_log_item_t	*bip = bp->b_log_item;
+	struct xfs_buf_log_item	*bip = bp->b_log_item;
 #ifdef XACT_DEBUG
 	fprintf(stderr, "binval'd buffer %p, transaction %p\n", bp, tp);
 #endif
@@ -308,10 +400,12 @@ xfs_trans_binval(
 	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
 
-	if (bip->bli_flags & XFS_BLI_STALE)
+	if (bip->bli_flags & XFS_BLI_STALE) {
 		return;
+	}
 	XFS_BUF_UNDELAYWRITE(bp);
 	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;
@@ -320,12 +414,21 @@ xfs_trans_binval(
 	tp->t_flags |= XFS_TRANS_DIRTY;
 }
 
+/*
+ * Mark the buffer as being one which contains newly allocated
+ * inodes.  We need to make sure that even if this buffer is
+ * relogged as an 'inode buf' we still recover all of the inode
+ * images in the face of a crash.  This works in coordination with
+ * xfs_buf_item_committed() to ensure that the buffer remains in the
+ * AIL at its original location even after it has been relogged.
+ */
+/* ARGSUSED */
 void
 xfs_trans_inode_alloc_buf(
 	xfs_trans_t		*tp,
 	xfs_buf_t		*bp)
 {
-	xfs_buf_log_item_t	*bip = bp->b_log_item;
+	struct xfs_buf_log_item	*bip = bp->b_log_item;
 
 	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
-- 
1.8.3.1

Re: [PATCH 11/11] libxfs: minor sync-ups to kernel-ish functions

[Dave Chinner]

On Fri, May 10, 2019 at 03:18:30PM -0500, Eric Sandeen wrote:
> Change typedefs to structs, add comments, and other very
> minor changes to userspace libxfs/ functions so that they
> more closely match kernelspace.  Should be no functional
> changes.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Ah, there's the typedef removal....

....
>  /*
>   * Free the transaction structure.  If there is more clean up
>   * to do when the structure is freed, add it here.
>   */
> -static void
> +STATIC void
>  xfs_trans_free(
>  	struct xfs_trans	*tp)
>  {
> @@ -106,7 +102,7 @@ xfs_trans_reserve(
>  	uint			blocks,
>  	uint			rtextents)
>  {
> -	int			error = 0;
> +	int		error = 0;
>  
>  	/*
>  	 * Attempt to reserve the needed disk blocks by decrementing
> @@ -114,8 +110,9 @@ xfs_trans_reserve(
>  	 * fail if the count would go below zero.
>  	 */
>  	if (blocks > 0) {
> -		if (tp->t_mountp->m_sb.sb_fdblocks < blocks)
> +		if (tp->t_mountp->m_sb.sb_fdblocks < blocks) {
>  			return -ENOSPC;
> +		}
>  		tp->t_blk_res += blocks;
>  	}

These seem a bit weird by themselves. I know, the kernel code has
more code in the branches and this reduces the diff a bit, but
it does look a inconsistent now...

> @@ -235,18 +233,24 @@ xfs_trans_alloc_empty(
>   * Record the indicated change to the given field for application
>   * to the file system's superblock when the transaction commits.
>   * For now, just store the change in the transaction structure.
> + *
>   * Mark the transaction structure to indicate that the superblock
>   * needs to be updated before committing.
> - *
> - * Originally derived from xfs_trans_mod_sb().
>   */
>  void
>  xfs_trans_mod_sb(
> -	xfs_trans_t		*tp,
> -	uint			field,
> -	long			delta)
> +	xfs_trans_t	*tp,

typedef removal :)

Kernel side, too, I guess...

>  
> -xfs_buf_t *
> +/*
> + * Get and lock the buffer for the caller if it is not already
> + * locked within the given transaction.  If it is already locked
> + * within the transaction, just increment its lock recursion count
> + * and return a pointer to it.
> + *
> + * If the transaction pointer is NULL, make this just a normal
> + * get_buf() call.
> + */
> +struct xfs_buf *
>  xfs_trans_get_buf_map(
> -	xfs_trans_t		*tp,
> -	struct xfs_buftarg	*btp,
> +	struct xfs_trans	*tp,
> +	struct xfs_buftarg	*target,
>  	struct xfs_buf_map	*map,
>  	int			nmaps,
> -	uint			f)
> +	uint			flags)
>  {
>  	xfs_buf_t		*bp;

You missed a typedef.

>  
> +/*
> + * Get and lock the superblock buffer of this file system for the
> + * given transaction.
> + *
> + * We don't need to use incore_match() here, because the superblock
> + * buffer is a private buffer which we keep a pointer to in the
> + * mount structure.
> + */
>  xfs_buf_t *

typedef

>  xfs_trans_getsb(
>  	xfs_trans_t		*tp,

Another

> -	xfs_mount_t		*mp,
> +	struct xfs_mount	*mp,
>  	int			flags)
>  {
>  	xfs_buf_t		*bp;

And another.

yup, kernel code needs fixing, too.

.....
> + * transaction began, then also free the buf_log_item associated with it.
> + *
> + * If the transaction pointer is NULL, this is a normal xfs_buf_relse() call.
> + */
>  void
>  xfs_trans_brelse(
> -	xfs_trans_t		*tp,
> -	xfs_buf_t		*bp)
> +	struct xfs_trans	*tp,
> +	struct xfs_buf		*bp)
>  {
> -	xfs_buf_log_item_t	*bip;
> +	struct xfs_buf_log_item	*bip = bp->b_log_item;
>  #ifdef XACT_DEBUG
>  	fprintf(stderr, "released buffer %p, transaction %p\n", bp, tp);
>  #endif
>  
> -	if (tp == NULL) {
> +	if (!tp) {
>  		ASSERT(bp->b_transp == NULL);
>  		libxfs_putbuf(bp);
>  		return;
>  	}
>  	ASSERT(bp->b_transp == tp);
> -	bip = bp->b_log_item;
>  	ASSERT(bip->bli_item.li_type == XFS_LI_BUF);
> +
> +	/*
> +	 * If the release is for a recursive lookup, then decrement the count
> +	 * and return.
> +	 */
>  	if (bip->bli_recur > 0) {
>  		bip->bli_recur--;
>  		return;
>  	}
> -	/* If dirty/stale, can't release till transaction committed */
> -	if (bip->bli_flags & XFS_BLI_STALE)
> -		return;
> +
> +	/*
> +	 * If the buffer is invalidated or dirty in this transaction, we can't
> +	 * release it until we commit.
> +	 */
>  	if (test_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags))
>  		return;
> +	if (bip->bli_flags & XFS_BLI_STALE)
> +		return;

THis is a change of behaviour for userspace, right? What does
checking for a dirty log item do?

> +/*
> + * Invalidate a buffer that is being used within a transaction.
> + *
> + * Typically this is because the blocks in the buffer are being freed, so we
> + * need to prevent it from being written out when we're done.  Allowing it
> + * to be written again might overwrite data in the free blocks if they are
> + * reallocated to a file.
> + *
> + * We prevent the buffer from being written out by marking it stale.  We can't
> + * get rid of the buf log item at this point because the buffer may still be
> + * pinned by another transaction.  If that is the case, then we'll wait until
> + * the buffer is committed to disk for the last time (we can tell by the ref
> + * count) and free it in xfs_buf_item_unpin().  Until that happens we will
> + * keep the buffer locked so that the buffer and buf log item are not reused.
> + *
> + * We also set the XFS_BLF_CANCEL flag in the buf log format structure and log
> + * the buf item.  This will be used at recovery time to determine that copies
> + * of the buffer in the log before this should not be replayed.
> + *
> + * We mark the item descriptor and the transaction dirty so that we'll hold
> + * the buffer until after the commit.
> + *
> + * Since we're invalidating the buffer, we also clear the state about which
> + * parts of the buffer have been logged.  We also clear the flag indicating
> + * that this is an inode buffer since the data in the buffer will no longer
> + * be valid.
> + *
> + * We set the stale bit in the buffer as well since we're getting rid of it.
> + */
>  void
>  xfs_trans_binval(

Does userspace even have half of this functionality? Does marking
the buffer stale in userspace give the same guarantees as the kernel?
There's no point bringing across comments that don't reflect how
userspace works at this point in time...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 11/11] libxfs: minor sync-ups to kernel-ish functions

[Eric Sandeen]

On 5/15/19 1:25 AM, Dave Chinner wrote:
> On Fri, May 10, 2019 at 03:18:30PM -0500, Eric Sandeen wrote:
>> Change typedefs to structs, add comments, and other very
>> minor changes to userspace libxfs/ functions so that they
>> more closely match kernelspace.  Should be no functional
>> changes.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> 
> Ah, there's the typedef removal....
> 
> ....
>>  /*
>>   * Free the transaction structure.  If there is more clean up
>>   * to do when the structure is freed, add it here.
>>   */
>> -static void
>> +STATIC void
>>  xfs_trans_free(
>>  	struct xfs_trans	*tp)
>>  {
>> @@ -106,7 +102,7 @@ xfs_trans_reserve(
>>  	uint			blocks,
>>  	uint			rtextents)
>>  {
>> -	int			error = 0;
>> +	int		error = 0;
>>  
>>  	/*
>>  	 * Attempt to reserve the needed disk blocks by decrementing
>> @@ -114,8 +110,9 @@ xfs_trans_reserve(
>>  	 * fail if the count would go below zero.
>>  	 */
>>  	if (blocks > 0) {
>> -		if (tp->t_mountp->m_sb.sb_fdblocks < blocks)
>> +		if (tp->t_mountp->m_sb.sb_fdblocks < blocks) {
>>  			return -ENOSPC;
>> +		}
>>  		tp->t_blk_res += blocks;
>>  	}
> 
> These seem a bit weird by themselves. I know, the kernel code has
> more code in the branches and this reduces the diff a bit, but
> it does look a inconsistent now...

The goal would be to add the other bits back in later.  But doing it
in small steps like this just causes confusion, I guess?

>> @@ -235,18 +233,24 @@ xfs_trans_alloc_empty(
>>   * Record the indicated change to the given field for application
>>   * to the file system's superblock when the transaction commits.
>>   * For now, just store the change in the transaction structure.
>> + *
>>   * Mark the transaction structure to indicate that the superblock
>>   * needs to be updated before committing.
>> - *
>> - * Originally derived from xfs_trans_mod_sb().
>>   */
>>  void
>>  xfs_trans_mod_sb(
>> -	xfs_trans_t		*tp,
>> -	uint			field,
>> -	long			delta)
>> +	xfs_trans_t	*tp,
> 
> typedef removal :)
> 
> Kernel side, too, I guess...

Right, the goal here is to sync up with the kernel, not diverge more.

>>  
>> -xfs_buf_t *
>> +/*
>> + * Get and lock the buffer for the caller if it is not already
>> + * locked within the given transaction.  If it is already locked
>> + * within the transaction, just increment its lock recursion count
>> + * and return a pointer to it.
>> + *
>> + * If the transaction pointer is NULL, make this just a normal
>> + * get_buf() call.
>> + */
>> +struct xfs_buf *
>>  xfs_trans_get_buf_map(
>> -	xfs_trans_t		*tp,
>> -	struct xfs_buftarg	*btp,
>> +	struct xfs_trans	*tp,
>> +	struct xfs_buftarg	*target,
>>  	struct xfs_buf_map	*map,
>>  	int			nmaps,
>> -	uint			f)
>> +	uint			flags)
>>  {
>>  	xfs_buf_t		*bp;
> 
> You missed a typedef.

kernel missed a typedef ;)

[sandeen@sandeen linux-xfs]$ grep -A7 ^xfs_trans_get_buf_map fs/xfs/*.c fs/xfs/*/*.c
fs/xfs/xfs_trans_buf.c:xfs_trans_get_buf_map(
fs/xfs/xfs_trans_buf.c-	struct xfs_trans	*tp,
fs/xfs/xfs_trans_buf.c-	struct xfs_buftarg	*target,
fs/xfs/xfs_trans_buf.c-	struct xfs_buf_map	*map,
fs/xfs/xfs_trans_buf.c-	int			nmaps,
fs/xfs/xfs_trans_buf.c-	xfs_buf_flags_t		flags)
fs/xfs/xfs_trans_buf.c-{
fs/xfs/xfs_trans_buf.c-	xfs_buf_t		*bp;

>>  
>> +/*
>> + * Get and lock the superblock buffer of this file system for the
>> + * given transaction.
>> + *
>> + * We don't need to use incore_match() here, because the superblock
>> + * buffer is a private buffer which we keep a pointer to in the
>> + * mount structure.
>> + */
>>  xfs_buf_t *
> 
> typedef
> 
>>  xfs_trans_getsb(
>>  	xfs_trans_t		*tp,
> 
> Another
> 
>> -	xfs_mount_t		*mp,
>> +	struct xfs_mount	*mp,
>>  	int			flags)
>>  {
>>  	xfs_buf_t		*bp;
> 
> And another.
> 
> yup, kernel code needs fixing, too.

"you go first!" ;)

> .....
>> + * transaction began, then also free the buf_log_item associated with it.
>> + *
>> + * If the transaction pointer is NULL, this is a normal xfs_buf_relse() call.
>> + */
>>  void
>>  xfs_trans_brelse(
>> -	xfs_trans_t		*tp,
>> -	xfs_buf_t		*bp)
>> +	struct xfs_trans	*tp,
>> +	struct xfs_buf		*bp)
>>  {
>> -	xfs_buf_log_item_t	*bip;
>> +	struct xfs_buf_log_item	*bip = bp->b_log_item;
>>  #ifdef XACT_DEBUG
>>  	fprintf(stderr, "released buffer %p, transaction %p\n", bp, tp);
>>  #endif
>>  
>> -	if (tp == NULL) {
>> +	if (!tp) {
>>  		ASSERT(bp->b_transp == NULL);
>>  		libxfs_putbuf(bp);
>>  		return;
>>  	}
>>  	ASSERT(bp->b_transp == tp);
>> -	bip = bp->b_log_item;
>>  	ASSERT(bip->bli_item.li_type == XFS_LI_BUF);
>> +
>> +	/*
>> +	 * If the release is for a recursive lookup, then decrement the count
>> +	 * and return.
>> +	 */
>>  	if (bip->bli_recur > 0) {
>>  		bip->bli_recur--;
>>  		return;
>>  	}
>> -	/* If dirty/stale, can't release till transaction committed */
>> -	if (bip->bli_flags & XFS_BLI_STALE)
>> -		return;
>> +
>> +	/*
>> +	 * If the buffer is invalidated or dirty in this transaction, we can't
>> +	 * release it until we commit.
>> +	 */
>>  	if (test_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags))
>>  		return;
>> +	if (bip->bli_flags & XFS_BLI_STALE)
>> +		return;
> 
> THis is a change of behaviour for userspace, right? What does
> checking for a dirty log item do?

No, it just re-orders an existing test, look up a few lines.

>> +/*
>> + * Invalidate a buffer that is being used within a transaction.
>> + *
>> + * Typically this is because the blocks in the buffer are being freed, so we
>> + * need to prevent it from being written out when we're done.  Allowing it
>> + * to be written again might overwrite data in the free blocks if they are
>> + * reallocated to a file.
>> + *
>> + * We prevent the buffer from being written out by marking it stale.  We can't
>> + * get rid of the buf log item at this point because the buffer may still be
>> + * pinned by another transaction.  If that is the case, then we'll wait until
>> + * the buffer is committed to disk for the last time (we can tell by the ref
>> + * count) and free it in xfs_buf_item_unpin().  Until that happens we will
>> + * keep the buffer locked so that the buffer and buf log item are not reused.
>> + *
>> + * We also set the XFS_BLF_CANCEL flag in the buf log format structure and log
>> + * the buf item.  This will be used at recovery time to determine that copies
>> + * of the buffer in the log before this should not be replayed.
>> + *
>> + * We mark the item descriptor and the transaction dirty so that we'll hold
>> + * the buffer until after the commit.
>> + *
>> + * Since we're invalidating the buffer, we also clear the state about which
>> + * parts of the buffer have been logged.  We also clear the flag indicating
>> + * that this is an inode buffer since the data in the buffer will no longer
>> + * be valid.
>> + *
>> + * We set the stale bit in the buffer as well since we're getting rid of it.
>> + */
>>  void
>>  xfs_trans_binval(
> 
> Does userspace even have half of this functionality? Does marking
> the buffer stale in userspace give the same guarantees as the kernel?
> There's no point bringing across comments that don't reflect how
> userspace works at this point in time...

We have a lot of comments in userspace libxfs/* that reference locking that is 
defined away, etc.  IOWs we have this today, and I thought it was 
understood that shared kernelspace comments didn't always reflect userspace
reality...

*shrug*

Thanks for looking it over,

-Eric


> Cheers,
> 
> Dave.
>