* [PATCH 1/8] libxfs: port kernel transaction code
2018-10-01 17:04 [PATCH v3 0/8] xfsprogs-4.19: transaction cleanups Darrick J. Wong
@ 2018-10-01 17:04 ` Darrick J. Wong
2018-10-01 17:04 ` [PATCH 2/8] libxfs: fix libxfs_trans_alloc callsite problems Darrick J. Wong
` (8 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2018-10-01 17:04 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Restructure the userspace transaction code to resemble the kernel code
more closely. This will make deferred operations behave the same (with
respect to transaction lifetimes) as the kernel.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
libxfs/trans.c | 238 ++++++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 187 insertions(+), 51 deletions(-)
diff --git a/libxfs/trans.c b/libxfs/trans.c
index b1543b0c..11a2098d 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -20,6 +20,10 @@
#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
@@ -76,24 +80,17 @@ int
libxfs_trans_roll(
struct xfs_trans **tpp)
{
- struct xfs_mount *mp;
struct xfs_trans *trans = *tpp;
struct xfs_trans_res tres;
- unsigned int old_blk_res;
- xfs_fsblock_t old_firstblock;
- struct list_head old_dfops;
int error;
/*
* Copy the critical parameters from one trans to the next.
*/
- mp = trans->t_mountp;
tres.tr_logres = trans->t_log_res;
tres.tr_logcount = trans->t_log_count;
- old_blk_res = trans->t_blk_res;
- old_firstblock = trans->t_firstblock;
- /* structure copy */
- old_dfops = trans->t_dfops;
+
+ *tpp = xfs_trans_dup(trans);
/*
* Commit the current transaction.
@@ -102,7 +99,7 @@ libxfs_trans_roll(
* is in progress. The caller takes the responsibility to cancel
* the duplicate transaction that gets returned.
*/
- error = xfs_trans_commit(trans);
+ error = __xfs_trans_commit(trans, true);
if (error)
return error;
@@ -115,14 +112,7 @@ libxfs_trans_roll(
* the prior and the next transactions.
*/
tres.tr_logflags = XFS_TRANS_PERM_LOG_RES;
- error = libxfs_trans_alloc(mp, &tres, 0, 0, 0, tpp);
- trans = *tpp;
- trans->t_blk_res = old_blk_res;
- trans->t_firstblock = old_firstblock;
- /* structure copy */
- trans->t_dfops = old_dfops;
-
- return 0;
+ return xfs_trans_reserve(*tpp, &tres, 0, 0);
}
/*
@@ -136,40 +126,150 @@ xfs_trans_free(
kmem_zone_free(xfs_trans_zone, tp);
}
-int
-libxfs_trans_alloc(
- struct xfs_mount *mp,
- struct xfs_trans_res *resp,
- unsigned int blocks,
- unsigned int rtextents,
- unsigned int flags,
- struct xfs_trans **tpp)
+/*
+ * This is called to create a new transaction which will share the
+ * permanent log reservation of the given transaction. The remaining
+ * unused block and rt extent reservations are also inherited. This
+ * implies that the original transaction is no longer allowed to allocate
+ * blocks. Locks and log items, however, are no inherited. They must
+ * be added to the new transaction explicitly.
+ */
+STATIC struct xfs_trans *
+xfs_trans_dup(
+ struct xfs_trans *tp)
+{
+ struct xfs_trans *ntp;
+ ntp = kmem_zone_zalloc(xfs_trans_zone, KM_SLEEP);
+
+ /*
+ * Initialize the new transaction structure.
+ */
+ ntp->t_mountp = tp->t_mountp;
+ INIT_LIST_HEAD(&ntp->t_items);
+ INIT_LIST_HEAD(&ntp->t_dfops);
+ ntp->t_firstblock = NULLFSBLOCK;
+
+ ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES);
+
+ ntp->t_flags = XFS_TRANS_PERM_LOG_RES |
+ (tp->t_flags & XFS_TRANS_RESERVE) |
+ (tp->t_flags & XFS_TRANS_NO_WRITECOUNT);
+ /* We gave our writer reference to the new transaction */
+ tp->t_flags |= XFS_TRANS_NO_WRITECOUNT;
+
+ /* move deferred ops over to the new tp */
+ xfs_defer_move(ntp, tp);
+
+ return ntp;
+}
+
+/*
+ * This is called to reserve free disk blocks and log space for the
+ * given transaction. This must be done before allocating any resources
+ * within the transaction.
+ *
+ * This will return ENOSPC if there are not enough blocks available.
+ * It will sleep waiting for available log space.
+ * The only valid value for the flags parameter is XFS_RES_LOG_PERM, which
+ * is used by long running transactions. If any one of the reservations
+ * fails then they will all be backed out.
+ *
+ * This does not do quota reservations. That typically is done by the
+ * caller afterwards.
+ */
+static int
+xfs_trans_reserve(
+ struct xfs_trans *tp,
+ struct xfs_trans_res *resp,
+ uint blocks,
+ uint rtextents)
{
- struct xfs_sb *sb = &mp->m_sb;
- struct xfs_trans *ptr;
+ int error = 0;
/*
* Attempt to reserve the needed disk blocks by decrementing
- * the number needed from the number available. This will
+ * the number needed from the number available. This will
* fail if the count would go below zero.
*/
if (blocks > 0) {
- if (sb->sb_fdblocks < blocks)
+ if (tp->t_mountp->m_sb.sb_fdblocks < blocks)
return -ENOSPC;
+ tp->t_blk_res += blocks;
+ }
+
+ /*
+ * Reserve the log space needed for this transaction.
+ */
+ if (resp->tr_logres > 0) {
+ ASSERT(tp->t_log_res == 0 ||
+ tp->t_log_res == resp->tr_logres);
+ ASSERT(tp->t_log_count == 0 ||
+ tp->t_log_count == resp->tr_logcount);
+
+ if (resp->tr_logflags & XFS_TRANS_PERM_LOG_RES)
+ tp->t_flags |= XFS_TRANS_PERM_LOG_RES;
+ else
+ ASSERT(!(tp->t_flags & XFS_TRANS_PERM_LOG_RES));
+
+ tp->t_log_res = resp->tr_logres;
+ tp->t_log_count = resp->tr_logcount;
}
- ptr = kmem_zone_zalloc(xfs_trans_zone,
+ /*
+ * Attempt to reserve the needed realtime extents by decrementing
+ * the number needed from the number available. This will
+ * fail if the count would go below zero.
+ */
+ if (rtextents > 0) {
+ if (tp->t_mountp->m_sb.sb_rextents < rtextents) {
+ error = -ENOSPC;
+ goto undo_blocks;
+ }
+ }
+
+ return 0;
+
+ /*
+ * Error cases jump to one of these labels to undo any
+ * reservations which have already been performed.
+ */
+undo_blocks:
+ if (blocks > 0)
+ tp->t_blk_res = 0;
+
+ return error;
+}
+
+int
+libxfs_trans_alloc(
+ struct xfs_mount *mp,
+ struct xfs_trans_res *resp,
+ unsigned int blocks,
+ unsigned int rtextents,
+ unsigned int flags,
+ struct xfs_trans **tpp)
+
+{
+ struct xfs_trans *tp;
+ int error;
+
+ tp = kmem_zone_zalloc(xfs_trans_zone,
(flags & XFS_TRANS_NOFS) ? KM_NOFS : KM_SLEEP);
- ptr->t_mountp = mp;
- ptr->t_blk_res = blocks;
- INIT_LIST_HEAD(&ptr->t_items);
- INIT_LIST_HEAD(&ptr->t_dfops);
- ptr->t_firstblock = NULLFSBLOCK;
+ tp->t_mountp = mp;
+ INIT_LIST_HEAD(&tp->t_items);
+ INIT_LIST_HEAD(&tp->t_dfops);
+ tp->t_firstblock = NULLFSBLOCK;
+
+ error = xfs_trans_reserve(tp, resp, blocks, rtextents);
+ if (error) {
+ xfs_trans_cancel(tp);
+ return error;
+ }
#ifdef XACT_DEBUG
- fprintf(stderr, "allocated new transaction %p\n", ptr);
+ fprintf(stderr, "allocated new transaction %p\n", tp);
#endif
- *tpp = ptr;
+ *tpp = tp;
return 0;
}
@@ -197,18 +297,25 @@ libxfs_trans_alloc_empty(
void
libxfs_trans_cancel(
- xfs_trans_t *tp)
+ struct xfs_trans *tp)
{
#ifdef XACT_DEBUG
- xfs_trans_t *otp = tp;
+ struct xfs_trans *otp = tp;
#endif
- if (tp != NULL) {
- xfs_trans_free_items(tp);
- xfs_trans_free(tp);
- }
+ 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;
}
int
@@ -261,6 +368,9 @@ libxfs_trans_ijoin(
ASSERT(iip->ili_flags == 0);
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));
ip->i_transp = tp;
@@ -839,22 +949,36 @@ xfs_trans_free_items(
/*
* Commit the changes represented by this transaction
*/
-int
-libxfs_trans_commit(
- xfs_trans_t *tp)
+static int
+__xfs_trans_commit(
+ struct xfs_trans *tp,
+ bool regrant)
{
- xfs_sb_t *sbp;
+ struct xfs_sb *sbp;
+ int error = 0;
if (tp == NULL)
return 0;
+ /*
+ * Finish deferred items on final commit. Only permanent transactions
+ * should ever have deferred ops.
+ */
+ WARN_ON_ONCE(!list_empty(&tp->t_dfops) &&
+ !(tp->t_flags & XFS_TRANS_PERM_LOG_RES));
+ if (!regrant && (tp->t_flags & XFS_TRANS_PERM_LOG_RES)) {
+ error = xfs_defer_finish_noroll(&tp);
+ if (error) {
+ xfs_defer_cancel(tp);
+ goto out_unreserve;
+ }
+ }
+
if (!(tp->t_flags & XFS_TRANS_DIRTY)) {
#ifdef XACT_DEBUG
fprintf(stderr, "committed clean transaction %p\n", tp);
#endif
- xfs_trans_free_items(tp);
- xfs_trans_free(tp);
- return 0;
+ goto out_unreserve;
}
if (tp->t_flags & XFS_TRANS_SB_DIRTY) {
@@ -878,4 +1002,16 @@ libxfs_trans_commit(
/* That's it for the transaction structure. Free it. */
xfs_trans_free(tp);
return 0;
+
+out_unreserve:
+ xfs_trans_free_items(tp);
+ xfs_trans_free(tp);
+ return error;
+}
+
+int
+libxfs_trans_commit(
+ struct xfs_trans *tp)
+{
+ return __xfs_trans_commit(tp, false);
}
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 2/8] libxfs: fix libxfs_trans_alloc callsite problems
2018-10-01 17:04 [PATCH v3 0/8] xfsprogs-4.19: transaction cleanups Darrick J. Wong
2018-10-01 17:04 ` [PATCH 1/8] libxfs: port kernel transaction code Darrick J. Wong
@ 2018-10-01 17:04 ` Darrick J. Wong
2018-10-01 17:04 ` [PATCH 3/8] xfs_repair: fix block reservation in mk_rsumino Darrick J. Wong
` (7 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2018-10-01 17:04 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Fix some incorrect libxfs_trans_alloc callers to check return values
correctly.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
mkfs/proto.c | 4 +++-
mkfs/xfs_mkfs.c | 2 +-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/mkfs/proto.c b/mkfs/proto.c
index c13e3644..07d019d6 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -192,7 +192,9 @@ rsvfile(
/*
* update the inode timestamp, mode, and prealloc flag bits
*/
- libxfs_trans_alloc(mp, &tres, 0, 0, 0, &tp);
+ error = -libxfs_trans_alloc(mp, &tres, 0, 0, 0, &tp);
+ if (error)
+ fail(_("allocating transaction for a file"), error);
libxfs_trans_ijoin(tp, ip, 0);
VFS_I(ip)->i_mode &= ~S_ISUID;
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 2e53c1e8..c6ef3a71 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3677,7 +3677,7 @@ initialise_ag_freespace(
struct xfs_trans_res tres = {0};
int c;
- c = libxfs_trans_alloc(mp, &tres, worst_freelist, 0, 0, &tp);
+ c = -libxfs_trans_alloc(mp, &tres, worst_freelist, 0, 0, &tp);
if (c)
res_failed(c);
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 3/8] xfs_repair: fix block reservation in mk_rsumino
2018-10-01 17:04 [PATCH v3 0/8] xfsprogs-4.19: transaction cleanups Darrick J. Wong
2018-10-01 17:04 ` [PATCH 1/8] libxfs: port kernel transaction code Darrick J. Wong
2018-10-01 17:04 ` [PATCH 2/8] libxfs: fix libxfs_trans_alloc callsite problems Darrick J. Wong
@ 2018-10-01 17:04 ` Darrick J. Wong
2018-10-01 17:04 ` [PATCH 4/8] libxfs: fix xfs_trans_alloc reservation abuse Darrick J. Wong
` (6 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2018-10-01 17:04 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
The functions mk_rsumino and rtinit both allocate transactions to create
the realtime summary inode. In order to allocate and map blocks to the
rtsummary file, these transactions require a block reservation.
However, despite the comments in mk_rsumino about lifting the code from
mkfs, it doesn't actually copy the same reservation calculation that
mkfs uses in rtinit(). Practically speaking this has no effect since
userspace doesn't care about transaction block reservations, but fix
this logic bomb anyway.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
repair/phase6.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/repair/phase6.c b/repair/phase6.c
index d4b6a5cf..afa65c51 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -815,7 +815,7 @@ mk_rsumino(xfs_mount_t *mp)
tres.tr_logcount = XFS_DEFAULT_PERM_LOG_COUNT;
tres.tr_logflags = XFS_TRANS_PERM_LOG_RES;
error = -libxfs_trans_alloc(mp, &tres,
- mp->m_sb.sb_rbmblocks + (XFS_BM_MAXLEVELS(mp,XFS_DATA_FORK) - 1),
+ nsumblocks + (XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) - 1),
0, 0, &tp);
if (error)
res_failed(error);
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 4/8] libxfs: fix xfs_trans_alloc reservation abuse
2018-10-01 17:04 [PATCH v3 0/8] xfsprogs-4.19: transaction cleanups Darrick J. Wong
` (2 preceding siblings ...)
2018-10-01 17:04 ` [PATCH 3/8] xfs_repair: fix block reservation in mk_rsumino Darrick J. Wong
@ 2018-10-01 17:04 ` Darrick J. Wong
2018-10-01 17:04 ` [PATCH 5/8] libxfs: check libxfs_trans_commit return values Darrick J. Wong
` (5 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2018-10-01 17:04 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Various xfsprogs tools have been abusing the transaction reservation
system by allocating the transaction with zero reservation. This has
always worked in the past because userspace transactions do not require
reservations. However, once we merge deferred ops into the transaction
structure, we will need to use a permanent reservation type to set up
any transaction that can roll. tr_itruncate has all we need, so use
that as the reservation dummy.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
include/xfs_trans.h | 2 ++
libxfs/trans.c | 15 +++++++++++++++
mkfs/proto.c | 25 +++++++++++--------------
mkfs/xfs_mkfs.c | 3 +--
repair/phase5.c | 3 +--
repair/phase6.c | 26 ++++++++++----------------
repair/rmap.c | 8 +++-----
7 files changed, 43 insertions(+), 39 deletions(-)
diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index 54546da8..eda2f894 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -86,6 +86,8 @@ int xfs_trans_roll(struct xfs_trans **);
int libxfs_trans_alloc(struct xfs_mount *mp, struct xfs_trans_res *resp,
uint blocks, uint rtextents, uint flags,
struct xfs_trans **tpp);
+int libxfs_trans_alloc_rollable(struct xfs_mount *mp, uint blocks,
+ struct xfs_trans **tpp);
int libxfs_trans_alloc_empty(struct xfs_mount *mp, struct xfs_trans **tpp);
int libxfs_trans_commit(struct xfs_trans *);
void libxfs_trans_cancel(struct xfs_trans *);
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 11a2098d..508d12d7 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -295,6 +295,21 @@ libxfs_trans_alloc_empty(
return xfs_trans_alloc(mp, &resv, 0, 0, XFS_TRANS_NO_WRITECOUNT, tpp);
}
+/*
+ * 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 libxfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, blocks,
+ 0, 0, tpp);
+}
+
void
libxfs_trans_cancel(
struct xfs_trans *tp)
diff --git a/mkfs/proto.c b/mkfs/proto.c
index 07d019d6..50a1bc54 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -123,9 +123,7 @@ getres(
uint r;
for (i = 0, r = MKFS_BLOCKRES(blocks); r >= blocks; r--) {
- struct xfs_trans_res tres = {0};
-
- i = -libxfs_trans_alloc(mp, &tres, r, 0, 0, &tp);
+ i = -libxfs_trans_alloc_rollable(mp, r, &tp);
if (i == 0)
return tp;
}
@@ -180,7 +178,6 @@ rsvfile(
{
int error;
xfs_trans_t *tp;
- struct xfs_trans_res tres = {0};
error = -libxfs_alloc_file_space(ip, 0, llen, 1, 0);
@@ -192,7 +189,7 @@ rsvfile(
/*
* update the inode timestamp, mode, and prealloc flag bits
*/
- error = -libxfs_trans_alloc(mp, &tres, 0, 0, 0, &tp);
+ error = -libxfs_trans_alloc_rollable(mp, 0, &tp);
if (error)
fail(_("allocating transaction for a file"), error);
libxfs_trans_ijoin(tp, ip, 0);
@@ -604,18 +601,18 @@ rtinit(
int i;
xfs_bmbt_irec_t map[XFS_BMAP_MAX_NMAP];
xfs_extlen_t nsumblocks;
+ uint blocks;
int nmap;
xfs_inode_t *rbmip;
xfs_inode_t *rsumip;
xfs_trans_t *tp;
struct cred creds;
struct fsxattr fsxattrs;
- struct xfs_trans_res tres = {0};
/*
* First, allocate the inodes.
*/
- i = -libxfs_trans_alloc(mp, &tres, MKFS_BLOCKRES_INODE, 0, 0, &tp);
+ i = -libxfs_trans_alloc_rollable(mp, MKFS_BLOCKRES_INODE, &tp);
if (i)
res_failed(i);
@@ -652,9 +649,9 @@ rtinit(
/*
* Next, give the bitmap file some zero-filled blocks.
*/
- i = -libxfs_trans_alloc(mp, &tres,
- mp->m_sb.sb_rbmblocks + (XFS_BM_MAXLEVELS(mp,XFS_DATA_FORK) - 1),
- 0, 0, &tp);
+ blocks = mp->m_sb.sb_rbmblocks +
+ XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) - 1;
+ i = -libxfs_trans_alloc_rollable(mp, blocks, &tp);
if (i)
res_failed(i);
@@ -683,9 +680,8 @@ rtinit(
* Give the summary file some zero-filled blocks.
*/
nsumblocks = mp->m_rsumsize >> mp->m_sb.sb_blocklog;
- i = -libxfs_trans_alloc(mp, &tres,
- nsumblocks + (XFS_BM_MAXLEVELS(mp,XFS_DATA_FORK) - 1),
- 0, 0, &tp);
+ blocks = nsumblocks + XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) - 1;
+ i = -libxfs_trans_alloc_rollable(mp, blocks, &tp);
if (i)
res_failed(i);
libxfs_trans_ijoin(tp, rsumip, 0);
@@ -713,7 +709,8 @@ rtinit(
* Do one transaction per bitmap block.
*/
for (bno = 0; bno < mp->m_sb.sb_rextents; bno = ebno) {
- i = -libxfs_trans_alloc(mp, &tres, 0, 0, 0, &tp);
+ i = -libxfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate,
+ 0, 0, 0, &tp);
if (i)
res_failed(i);
libxfs_trans_ijoin(tp, rbmip, 0);
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index c6ef3a71..982d3871 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3674,10 +3674,9 @@ initialise_ag_freespace(
{
struct xfs_alloc_arg args;
struct xfs_trans *tp;
- struct xfs_trans_res tres = {0};
int c;
- c = -libxfs_trans_alloc(mp, &tres, worst_freelist, 0, 0, &tp);
+ c = -libxfs_trans_alloc_rollable(mp, worst_freelist, &tp);
if (c)
res_failed(c);
diff --git a/repair/phase5.c b/repair/phase5.c
index 64f7b6e8..85d1f4fb 100644
--- a/repair/phase5.c
+++ b/repair/phase5.c
@@ -2421,7 +2421,6 @@ inject_lost_blocks(
struct xfs_trans *tp = NULL;
struct xfs_slab_cursor *cur = NULL;
xfs_fsblock_t *fsb;
- struct xfs_trans_res tres = {0};
struct xfs_owner_info oinfo;
int error;
@@ -2431,7 +2430,7 @@ inject_lost_blocks(
return error;
while ((fsb = pop_slab_cursor(cur)) != NULL) {
- error = -libxfs_trans_alloc(mp, &tres, 16, 0, 0, &tp);
+ error = -libxfs_trans_alloc_rollable(mp, 16, &tp);
if (error)
goto out_cancel;
diff --git a/repair/phase6.c b/repair/phase6.c
index afa65c51..d6cb0a5a 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -526,12 +526,12 @@ mk_rbmino(xfs_mount_t *mp)
xfs_bmbt_irec_t map[XFS_BMAP_MAX_NMAP];
int vers;
int times;
- struct xfs_trans_res tres = {0};
+ uint blocks;
/*
* first set up inode
*/
- i = -libxfs_trans_alloc(mp, &tres, 10, 0, 0, &tp);
+ i = -libxfs_trans_alloc_rollable(mp, 10, &tp);
if (i)
res_failed(i);
@@ -579,9 +579,9 @@ mk_rbmino(xfs_mount_t *mp)
* then allocate blocks for file and fill with zeroes (stolen
* from mkfs)
*/
- error = -libxfs_trans_alloc(mp, &tres,
- mp->m_sb.sb_rbmblocks + (XFS_BM_MAXLEVELS(mp,XFS_DATA_FORK) - 1),
- 0, 0, &tp);
+ blocks = mp->m_sb.sb_rbmblocks +
+ XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) - 1;
+ error = -libxfs_trans_alloc_rollable(mp, blocks, &tp);
if (error)
res_failed(error);
@@ -619,12 +619,11 @@ fill_rbmino(xfs_mount_t *mp)
int error;
xfs_fileoff_t bno;
xfs_bmbt_irec_t map;
- struct xfs_trans_res tres = {0};
bmp = btmcompute;
bno = 0;
- error = -libxfs_trans_alloc(mp, &tres, 10, 0, 0, &tp);
+ error = -libxfs_trans_alloc_rollable(mp, 10, &tp);
if (error)
res_failed(error);
@@ -686,13 +685,12 @@ fill_rsumino(xfs_mount_t *mp)
xfs_fileoff_t bno;
xfs_fileoff_t end_bno;
xfs_bmbt_irec_t map;
- struct xfs_trans_res tres = {0};
smp = sumcompute;
bno = 0;
end_bno = mp->m_rsumsize >> mp->m_sb.sb_blocklog;
- error = -libxfs_trans_alloc(mp, &tres, 10, 0, 0, &tp);
+ error = -libxfs_trans_alloc_rollable(mp, 10, &tp);
if (error)
res_failed(error);
@@ -757,7 +755,7 @@ mk_rsumino(xfs_mount_t *mp)
xfs_bmbt_irec_t map[XFS_BMAP_MAX_NMAP];
int vers;
int times;
- struct xfs_trans_res tres = {0};
+ uint blocks;
/*
* first set up inode
@@ -811,12 +809,8 @@ mk_rsumino(xfs_mount_t *mp)
* from mkfs)
*/
nsumblocks = mp->m_rsumsize >> mp->m_sb.sb_blocklog;
- tres.tr_logres = BBTOB(128);
- tres.tr_logcount = XFS_DEFAULT_PERM_LOG_COUNT;
- tres.tr_logflags = XFS_TRANS_PERM_LOG_RES;
- error = -libxfs_trans_alloc(mp, &tres,
- nsumblocks + (XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) - 1),
- 0, 0, &tp);
+ blocks = nsumblocks + XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) - 1;
+ error = -libxfs_trans_alloc_rollable(mp, blocks, &tp);
if (error)
res_failed(error);
diff --git a/repair/rmap.c b/repair/rmap.c
index f991144a..d596d0ff 100644
--- a/repair/rmap.c
+++ b/repair/rmap.c
@@ -449,7 +449,6 @@ rmap_store_ag_btree_rec(
struct xfs_buf *agbp = NULL;
struct xfs_buf *agflbp = NULL;
struct xfs_trans *tp;
- struct xfs_trans_res tres = {0};
__be32 *agfl_bno, *b;
int error = 0;
struct xfs_owner_info oinfo;
@@ -507,7 +506,7 @@ rmap_store_ag_btree_rec(
/* Insert rmaps into the btree one at a time */
rm_rec = pop_slab_cursor(rm_cur);
while (rm_rec) {
- error = -libxfs_trans_alloc(mp, &tres, 16, 0, 0, &tp);
+ error = -libxfs_trans_alloc_rollable(mp, 16, &tp);
if (error)
goto err_slab;
@@ -1366,7 +1365,6 @@ fix_freelist(
{
xfs_alloc_arg_t args;
xfs_trans_t *tp;
- struct xfs_trans_res tres = {0};
int flags;
int error;
@@ -1375,8 +1373,8 @@ fix_freelist(
args.agno = agno;
args.alignment = 1;
args.pag = libxfs_perag_get(mp, agno);
- error = -libxfs_trans_alloc(mp, &tres,
- libxfs_alloc_min_freelist(mp, args.pag), 0, 0, &tp);
+ error = -libxfs_trans_alloc_rollable(mp,
+ libxfs_alloc_min_freelist(mp, args.pag), &tp);
if (error)
do_error(_("failed to fix AGFL on AG %d, error %d\n"),
agno, error);
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 5/8] libxfs: check libxfs_trans_commit return values
2018-10-01 17:04 [PATCH v3 0/8] xfsprogs-4.19: transaction cleanups Darrick J. Wong
` (3 preceding siblings ...)
2018-10-01 17:04 ` [PATCH 4/8] libxfs: fix xfs_trans_alloc reservation abuse Darrick J. Wong
@ 2018-10-01 17:04 ` Darrick J. Wong
2018-10-01 17:04 ` [PATCH 6/8] libxfs: clean up IRELE/iput callsites Darrick J. Wong
` (4 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2018-10-01 17:04 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Check the return value from libxfs_trans_commit since it can now return
error codes.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
mkfs/proto.c | 38 ++++++++++++++++++-----
mkfs/xfs_mkfs.c | 4 ++
repair/phase6.c | 90 ++++++++++++++++++++++++++++++++++++++++++++-----------
repair/rmap.c | 4 ++
4 files changed, 108 insertions(+), 28 deletions(-)
diff --git a/mkfs/proto.c b/mkfs/proto.c
index 50a1bc54..dc82f093 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -211,7 +211,9 @@ rsvfile(
ip->i_d.di_flags |= XFS_DIFLAG_PREALLOC;
libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error)
+ fail(_("committing space for a file failed"), error);
}
static int
@@ -473,7 +475,9 @@ parseproto(
xname.type = XFS_DIR3_FT_REG_FILE;
newdirent(mp, tp, pip, &xname, ip->i_ino);
libxfs_trans_log_inode(tp, ip, flags);
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error)
+ fail(_("Space preallocation failed."), error);
rsvfile(mp, ip, llen);
IRELE(ip);
return;
@@ -551,7 +555,9 @@ parseproto(
}
newdirectory(mp, tp, ip, pip);
libxfs_trans_log_inode(tp, ip, flags);
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error)
+ fail(_("Directory inode allocation failed."), error);
/*
* RT initialization. Do this here to ensure that
* the RT inodes get placed after the root inode.
@@ -574,7 +580,11 @@ parseproto(
fail(_("Unknown format"), EINVAL);
}
libxfs_trans_log_inode(tp, ip, flags);
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error) {
+ fail(_("Error encountered creating file from prototype file"),
+ error);
+ }
IRELE(ip);
}
@@ -644,7 +654,10 @@ rtinit(
rsumip->i_d.di_size = mp->m_rsumsize;
libxfs_trans_log_inode(tp, rsumip, XFS_ILOG_CORE);
libxfs_log_sb(tp);
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error)
+ fail(_("Completion of the realtime summary inode failed"),
+ error);
mp->m_rsumip = rsumip;
/*
* Next, give the bitmap file some zero-filled blocks.
@@ -674,7 +687,10 @@ rtinit(
}
}
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error)
+ fail(_("Block allocation of the realtime bitmap inode failed"),
+ error);
/*
* Give the summary file some zero-filled blocks.
@@ -702,7 +718,10 @@ rtinit(
bno += ep->br_blockcount;
}
}
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error)
+ fail(_("Block allocation of the realtime summary inode failed"),
+ error);
/*
* Free the whole area using transactions.
@@ -721,7 +740,10 @@ rtinit(
fail(_("Error initializing the realtime space"),
error);
}
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error)
+ fail(_("Initialization of the realtime space failed"),
+ error);
}
}
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 982d3871..7c05c6f6 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3689,7 +3689,9 @@ initialise_ag_freespace(
libxfs_alloc_fix_freelist(&args, 0);
libxfs_perag_put(args.pag);
- libxfs_trans_commit(tp);
+ c = -libxfs_trans_commit(tp);
+ if (c)
+ res_failed(c);
}
/*
diff --git a/repair/phase6.c b/repair/phase6.c
index d6cb0a5a..8c0165ca 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -573,7 +573,9 @@ mk_rbmino(xfs_mount_t *mp)
* commit changes
*/
libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error)
+ do_error(_("%s: commit failed, error %d\n"), __func__, error);
/*
* then allocate blocks for file and fill with zeroes (stolen
@@ -604,7 +606,12 @@ mk_rbmino(xfs_mount_t *mp)
bno += ep->br_blockcount;
}
}
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error) {
+ do_error(
+ _("allocation of the realtime bitmap failed, error = %d\n"),
+ error);
+ }
IRELE(ip);
}
@@ -668,7 +675,9 @@ _("can't access block %" PRIu64 " (fsbno %" PRIu64 ") of realtime bitmap inode %
bno++;
}
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error)
+ do_error(_("%s: commit failed, error %d\n"), __func__, error);
IRELE(ip);
return(0);
}
@@ -736,7 +745,9 @@ _("can't access block %" PRIu64 " (fsbno %" PRIu64 ") of realtime summary inode
bno++;
}
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error)
+ do_error(_("%s: commit failed, error %d\n"), __func__, error);
IRELE(ip);
return(0);
}
@@ -802,7 +813,9 @@ mk_rsumino(xfs_mount_t *mp)
* commit changes
*/
libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error)
+ do_error(_("%s: commit failed, error %d\n"), __func__, error);
/*
* then allocate blocks for file and fill with zeroes (stolen
@@ -833,7 +846,12 @@ mk_rsumino(xfs_mount_t *mp)
bno += ep->br_blockcount;
}
}
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error) {
+ do_error(
+ _("allocation of the realtime summary ino failed, error = %d\n"),
+ error);
+ }
IRELE(ip);
}
@@ -898,7 +916,10 @@ mk_root_dir(xfs_mount_t *mp)
ip->d_ops = mp->m_dir_inode_ops;
libxfs_dir_init(tp, ip, ip);
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error)
+ do_error(_("%s: commit failed, error %d\n"), __func__, error);
+
IRELE(ip);
irec = find_inode_rec(mp, XFS_INO_TO_AGNO(mp, mp->m_sb.sb_rootino),
@@ -1028,7 +1049,11 @@ mk_orphanage(xfs_mount_t *mp)
libxfs_trans_log_inode(tp, pip, XFS_ILOG_CORE);
libxfs_dir_init(tp, ip, pip);
libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error) {
+ do_error(_("%s directory creation failed -- bmapf error %d\n"),
+ ORPHANAGE, error);
+ }
IRELE(ip);
IRELE(pip);
add_inode_reached(irec,ino_offset);
@@ -1126,7 +1151,10 @@ mv_orphanage(
inc_nlink(VFS_I(ino_p));
libxfs_trans_log_inode(tp, ino_p, XFS_ILOG_CORE);
- libxfs_trans_commit(tp);
+ err = -libxfs_trans_commit(tp);
+ if (err)
+ do_error(
+ _("creation of .. entry failed (%d)\n"), err);
} else {
err = -libxfs_trans_alloc(mp, &M_RES(mp)->tr_rename,
nres, 0, 0, &tp);
@@ -1166,7 +1194,10 @@ mv_orphanage(
err);
}
- libxfs_trans_commit(tp);
+ err = -libxfs_trans_commit(tp);
+ if (err)
+ do_error(
+ _("orphanage name replace op failed (%d)\n"), err);
}
} else {
@@ -1197,7 +1228,10 @@ mv_orphanage(
set_nlink(VFS_I(ino_p), 1);
libxfs_trans_log_inode(tp, ino_p, XFS_ILOG_CORE);
- libxfs_trans_commit(tp);
+ err = -libxfs_trans_commit(tp);
+ if (err)
+ do_error(
+ _("orphanage name create failed (%d)\n"), err);
}
IRELE(ino_p);
IRELE(orphanage_ip);
@@ -1333,7 +1367,10 @@ longform_dir2_rebuild(
goto out_bmap_cancel;
}
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error)
+ do_error(
+ _("dir init failed (%d)\n"), error);
if (ino == mp->m_sb.sb_rootino)
need_root_dotdot = 0;
@@ -1364,7 +1401,10 @@ _("name create failed in ino %" PRIu64 " (%d), filesystem may be out of space\n"
goto out_bmap_cancel;
}
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error)
+ do_error(
+_("name create failed (%d) during rebuild\n"), error);
}
return;
@@ -1410,7 +1450,10 @@ dir2_kill_block(
if (error)
do_error(_("shrink_inode failed inode %" PRIu64 " block %u\n"),
ip->i_ino, da_bno);
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error)
+ do_error(
+_("directory shrink failed (%d)\n"), error);
}
/*
@@ -1883,7 +1926,10 @@ _("entry \"%s\" in dir inode %" PRIu64 " inconsistent with .. value (%" PRIu64 "
d, &i);
if (needlog)
libxfs_dir2_data_log_header(&da, bp);
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error)
+ do_error(
+_("directory block fixing failed (%d)\n"), error);
/* record the largest free space in the freetab for later checking */
bf = M_DIROPS(mp)->data_bestfree_p(d);
@@ -2899,7 +2945,9 @@ process_dir_inode(
if (dirty) {
libxfs_trans_log_inode(tp, ip,
XFS_ILOG_CORE | XFS_ILOG_DDATA);
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error)
+ res_failed(error);
} else {
libxfs_trans_cancel(tp);
}
@@ -2940,7 +2988,10 @@ process_dir_inode(
_("can't make \"..\" entry in root inode %" PRIu64 ", createname error %d\n"), ino, error);
libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error)
+ do_error(
+ _("root inode \"..\" entry recreation failed (%d)\n"), error);
need_root_dotdot = 0;
} else if (need_root_dotdot && ino == mp->m_sb.sb_rootino) {
@@ -2993,7 +3044,10 @@ process_dir_inode(
ino, error);
libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error)
+ do_error(
+ _("root inode \".\" entry recreation failed (%d)\n"), error);
}
}
IRELE(ip);
diff --git a/repair/rmap.c b/repair/rmap.c
index d596d0ff..6de4a105 100644
--- a/repair/rmap.c
+++ b/repair/rmap.c
@@ -1410,7 +1410,9 @@ fix_freelist(
do_error(_("failed to fix AGFL on AG %d, error %d\n"),
agno, error);
}
- libxfs_trans_commit(tp);
+ error = -libxfs_trans_commit(tp);
+ if (error)
+ do_error(_("%s: commit failed, error %d\n"), __func__, error);
}
/*
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 6/8] libxfs: clean up IRELE/iput callsites
2018-10-01 17:04 [PATCH v3 0/8] xfsprogs-4.19: transaction cleanups Darrick J. Wong
` (4 preceding siblings ...)
2018-10-01 17:04 ` [PATCH 5/8] libxfs: check libxfs_trans_commit return values Darrick J. Wong
@ 2018-10-01 17:04 ` Darrick J. Wong
2018-10-01 17:05 ` [PATCH 7/8] libxfs: track transaction block reservation usage like the kernel Darrick J. Wong
` (3 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2018-10-01 17:04 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the IRELE macro with a proper function so that we can do proper
typechecking. This is the userspace cleanup in the same vein as the
kernel patch with the same subject.
---
db/attrset.c | 4 ++--
include/xfs_inode.h | 4 +---
libxfs/init.c | 4 ++--
libxfs/rdwr.c | 5 +++--
mkfs/proto.c | 6 +++---
repair/phase6.c | 22 +++++++++++-----------
repair/phase7.c | 2 +-
7 files changed, 23 insertions(+), 24 deletions(-)
diff --git a/db/attrset.c b/db/attrset.c
index dc82a6c6..56972506 100644
--- a/db/attrset.c
+++ b/db/attrset.c
@@ -159,7 +159,7 @@ attr_set_f(
out:
mp->m_flags &= ~LIBXFS_MOUNT_COMPAT_ATTR;
if (ip)
- IRELE(ip);
+ libxfs_irele(ip);
if (value)
free(value);
return 0;
@@ -234,6 +234,6 @@ attr_remove_f(
out:
mp->m_flags &= ~LIBXFS_MOUNT_COMPAT_ATTR;
if (ip)
- IRELE(ip);
+ libxfs_irele(ip);
return 0;
}
diff --git a/include/xfs_inode.h b/include/xfs_inode.h
index f573f23b..79ec3a2d 100644
--- a/include/xfs_inode.h
+++ b/include/xfs_inode.h
@@ -155,8 +155,6 @@ extern bool libxfs_inode_verify_forks(struct xfs_inode *ip,
extern int libxfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
uint, struct xfs_inode **,
struct xfs_ifork_ops *);
-extern void libxfs_iput(struct xfs_inode *);
-
-#define IRELE(ip) libxfs_iput(ip)
+extern void libxfs_irele(struct xfs_inode *ip);
#endif /* __XFS_INODE_H__ */
diff --git a/libxfs/init.c b/libxfs/init.c
index 36d637c5..d7543d4a 100644
--- a/libxfs/init.c
+++ b/libxfs/init.c
@@ -838,9 +838,9 @@ void
libxfs_rtmount_destroy(xfs_mount_t *mp)
{
if (mp->m_rsumip)
- IRELE(mp->m_rsumip);
+ libxfs_irele(mp->m_rsumip);
if (mp->m_rbmip)
- IRELE(mp->m_rbmip);
+ libxfs_irele(mp->m_rbmip);
mp->m_rsumip = mp->m_rbmip = NULL;
}
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 14a4633e..0ee3ba86 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -1402,7 +1402,7 @@ libxfs_iget(
}
if (!libxfs_inode_verify_forks(ip, ifork_ops)) {
- libxfs_iput(ip);
+ libxfs_irele(ip);
return -EFSCORRUPTED;
}
@@ -1435,7 +1435,8 @@ libxfs_idestroy(xfs_inode_t *ip)
}
void
-libxfs_iput(xfs_inode_t *ip)
+libxfs_irele(
+ struct xfs_inode *ip)
{
if (ip->i_itemp)
kmem_zone_free(xfs_ili_zone, ip->i_itemp);
diff --git a/mkfs/proto.c b/mkfs/proto.c
index dc82f093..687c53ab 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -479,7 +479,7 @@ parseproto(
if (error)
fail(_("Space preallocation failed."), error);
rsvfile(mp, ip, llen);
- IRELE(ip);
+ libxfs_irele(ip);
return;
case IF_BLOCK:
@@ -573,7 +573,7 @@ parseproto(
break;
parseproto(mp, ip, fsxp, pp, name);
}
- IRELE(ip);
+ libxfs_irele(ip);
return;
default:
ASSERT(0);
@@ -585,7 +585,7 @@ parseproto(
fail(_("Error encountered creating file from prototype file"),
error);
}
- IRELE(ip);
+ libxfs_irele(ip);
}
void
diff --git a/repair/phase6.c b/repair/phase6.c
index 8c0165ca..e2e4446a 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -612,7 +612,7 @@ mk_rbmino(xfs_mount_t *mp)
_("allocation of the realtime bitmap failed, error = %d\n"),
error);
}
- IRELE(ip);
+ libxfs_irele(ip);
}
static int
@@ -678,7 +678,7 @@ _("can't access block %" PRIu64 " (fsbno %" PRIu64 ") of realtime bitmap inode %
error = -libxfs_trans_commit(tp);
if (error)
do_error(_("%s: commit failed, error %d\n"), __func__, error);
- IRELE(ip);
+ libxfs_irele(ip);
return(0);
}
@@ -733,7 +733,7 @@ fill_rsumino(xfs_mount_t *mp)
do_warn(
_("can't access block %" PRIu64 " (fsbno %" PRIu64 ") of realtime summary inode %" PRIu64 "\n"),
bno, map.br_startblock, mp->m_sb.sb_rsumino);
- IRELE(ip);
+ libxfs_irele(ip);
return(1);
}
@@ -748,7 +748,7 @@ _("can't access block %" PRIu64 " (fsbno %" PRIu64 ") of realtime summary inode
error = -libxfs_trans_commit(tp);
if (error)
do_error(_("%s: commit failed, error %d\n"), __func__, error);
- IRELE(ip);
+ libxfs_irele(ip);
return(0);
}
@@ -852,7 +852,7 @@ mk_rsumino(xfs_mount_t *mp)
_("allocation of the realtime summary ino failed, error = %d\n"),
error);
}
- IRELE(ip);
+ libxfs_irele(ip);
}
/*
@@ -920,7 +920,7 @@ mk_root_dir(xfs_mount_t *mp)
if (error)
do_error(_("%s: commit failed, error %d\n"), __func__, error);
- IRELE(ip);
+ libxfs_irele(ip);
irec = find_inode_rec(mp, XFS_INO_TO_AGNO(mp, mp->m_sb.sb_rootino),
XFS_INO_TO_AGINO(mp, mp->m_sb.sb_rootino));
@@ -1054,8 +1054,8 @@ mk_orphanage(xfs_mount_t *mp)
do_error(_("%s directory creation failed -- bmapf error %d\n"),
ORPHANAGE, error);
}
- IRELE(ip);
- IRELE(pip);
+ libxfs_irele(ip);
+ libxfs_irele(pip);
add_inode_reached(irec,ino_offset);
return(ino);
@@ -1233,8 +1233,8 @@ mv_orphanage(
do_error(
_("orphanage name create failed (%d)\n"), err);
}
- IRELE(ino_p);
- IRELE(orphanage_ip);
+ libxfs_irele(ino_p);
+ libxfs_irele(orphanage_ip);
}
static int
@@ -3050,7 +3050,7 @@ process_dir_inode(
_("root inode \".\" entry recreation failed (%d)\n"), error);
}
}
- IRELE(ip);
+ libxfs_irele(ip);
}
/*
diff --git a/repair/phase7.c b/repair/phase7.c
index 09d11fcd..c2a60a93 100644
--- a/repair/phase7.c
+++ b/repair/phase7.c
@@ -77,7 +77,7 @@ update_inode_nlinks(
ASSERT(error == 0);
}
- IRELE(ip);
+ libxfs_irele(ip);
}
/*
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 7/8] libxfs: track transaction block reservation usage like the kernel
2018-10-01 17:04 [PATCH v3 0/8] xfsprogs-4.19: transaction cleanups Darrick J. Wong
` (5 preceding siblings ...)
2018-10-01 17:04 ` [PATCH 6/8] libxfs: clean up IRELE/iput callsites Darrick J. Wong
@ 2018-10-01 17:05 ` Darrick J. Wong
2018-10-01 17:05 ` [PATCH 8/8] xfs_scrub_all: fix systemd escaping again Darrick J. Wong
` (2 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2018-10-01 17:05 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Currently, block reservations in userspace transactions are not carried
over across transaction rolls. This will lead to ENOSPC failures inside
libxfs code which checks for reservation overruns in an upcoming patch
that borrows the bmbt repair code from the kernel because it makes
extensive use of transaction rolling.
Therefore, port t_blk_res_used from the kernel so that block
reservations work the same way in userspace.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
include/xfs_trans.h | 1 +
libxfs/trans.c | 12 ++++++++++++
2 files changed, 13 insertions(+)
diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index eda2f894..31df1172 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -71,6 +71,7 @@ typedef struct xfs_trans {
unsigned int t_blk_res; /* # of blocks resvd */
xfs_fsblock_t t_firstblock; /* first block allocated */
struct xfs_mount *t_mountp; /* ptr to fs mount struct */
+ unsigned int t_blk_res_used; /* # of resvd blocks used */
unsigned int t_flags; /* misc flags */
long t_icount_delta; /* superblock icount change */
long t_ifree_delta; /* superblock ifree change */
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 508d12d7..7058ecef 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -158,6 +158,9 @@ xfs_trans_dup(
/* We gave our writer reference to the new transaction */
tp->t_flags |= XFS_TRANS_NO_WRITECOUNT;
+ ntp->t_blk_res = tp->t_blk_res - tp->t_blk_res_used;
+ tp->t_blk_res = tp->t_blk_res_used;
+
/* move deferred ops over to the new tp */
xfs_defer_move(ntp, tp);
@@ -790,6 +793,15 @@ libxfs_trans_mod_sb(
case XFS_TRANS_SB_RES_FDBLOCKS:
return;
case XFS_TRANS_SB_FDBLOCKS:
+ if (delta < 0) {
+ tp->t_blk_res_used += (uint)-delta;
+ if (tp->t_blk_res_used > tp->t_blk_res) {
+ fprintf(stderr,
+_("Transaction block reservation exceeded! %u > %u\n"),
+ tp->t_blk_res_used, tp->t_blk_res);
+ ASSERT(0);
+ }
+ }
tp->t_fdblocks_delta += delta;
break;
case XFS_TRANS_SB_ICOUNT:
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 8/8] xfs_scrub_all: fix systemd escaping again
2018-10-01 17:04 [PATCH v3 0/8] xfsprogs-4.19: transaction cleanups Darrick J. Wong
` (6 preceding siblings ...)
2018-10-01 17:05 ` [PATCH 7/8] libxfs: track transaction block reservation usage like the kernel Darrick J. Wong
@ 2018-10-01 17:05 ` Darrick J. Wong
2018-10-04 19:13 ` [PATCH v3 0/8] xfsprogs-4.19: transaction cleanups Eric Sandeen
2018-10-04 22:27 ` [PATCH 9/8] xfs_io: dedupe command should only complain if we don't dedupe anything Darrick J. Wong
9 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2018-10-01 17:05 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs, Matthias Bodenbinder
From: Darrick J. Wong <darrick.wong@oracle.com>
Apparently newer versions of systemd than the one on this author's
laptop <cough> now complain about lack of (path) escaping in unit instance
variable contents:
# xfs_scrub_all
Scrubbing /home...
Invalid unit name "xfs_scrub@/home" was escaped as "xfs_scrub@-home"
(maybe you should use systemd-escape?)
Starting Online XFS Metadata Check for /home...
So change the systemd_escape() function to escape paths unconditionally
to make the warning go away.
Reported-by: Matthias Bodenbinder <matthias@bodenbinder.de>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
scrub/xfs_scrub_all.in | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/scrub/xfs_scrub_all.in b/scrub/xfs_scrub_all.in
index f7d9e6c7..c4e9899d 100644
--- a/scrub/xfs_scrub_all.in
+++ b/scrub/xfs_scrub_all.in
@@ -82,11 +82,16 @@ def run_killable(cmd, stdout, killfuncs, kill_fn):
# actually /can/ escape the dashes correctly if it is told that this is a path
# (and not a unit name), but it didn't do this prior to January 2017, so fix
# this for them.
+#
+# systemd path escaping also drops the initial slash so we add that back in so
+# that log messages from the service units preserve the full path and users can
+# look up log messages using full paths. However, for "/" the escaping rules
+# do /not/ drop the initial slash, so we have to special-case that here.
def systemd_escape(path):
'''Escape a path to avoid mangled systemd mangling.'''
- if '-' not in path:
- return path
+ if path == '/':
+ return '-'
cmd = ['systemd-escape', '--path', path]
try:
proc = subprocess.Popen(cmd, stdout = subprocess.PIPE)
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v3 0/8] xfsprogs-4.19: transaction cleanups
2018-10-01 17:04 [PATCH v3 0/8] xfsprogs-4.19: transaction cleanups Darrick J. Wong
` (7 preceding siblings ...)
2018-10-01 17:05 ` [PATCH 8/8] xfs_scrub_all: fix systemd escaping again Darrick J. Wong
@ 2018-10-04 19:13 ` Eric Sandeen
2018-10-04 19:43 ` Darrick J. Wong
2018-10-04 22:27 ` [PATCH 9/8] xfs_io: dedupe command should only complain if we don't dedupe anything Darrick J. Wong
9 siblings, 1 reply; 14+ messages in thread
From: Eric Sandeen @ 2018-10-04 19:13 UTC (permalink / raw)
To: Darrick J. Wong, sandeen; +Cc: linux-xfs
On 10/1/18 12:04 PM, Darrick J. Wong wrote:
> Here are seven cleanups to the userspace transaction code that make the
> commit and roll code more closely resemble their kernel counterparts,
> and fix a number of problems where client code did not check the return
> values of the transaction functions. The eighth patch fixes a bug in
> xfs_scrub_all. The patches should apply to Eric's libxfs-4.19-sync
> branch.
>
> --D
>
You're going to hate me (or maybe not) but in the end I decided to stop
being stubborn and bring in these patches before the libxfs-4.19 up[date,
as I should have from the start. You don't have to rebase them again tho,
I already got through it. I'll diff to your tree to make sure nothing went
haywire.
For the series with my out of order backporting tweaks,
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Thanks, this was great work.
-Eric
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v3 0/8] xfsprogs-4.19: transaction cleanups
2018-10-04 19:13 ` [PATCH v3 0/8] xfsprogs-4.19: transaction cleanups Eric Sandeen
@ 2018-10-04 19:43 ` Darrick J. Wong
0 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2018-10-04 19:43 UTC (permalink / raw)
To: Eric Sandeen; +Cc: sandeen, linux-xfs
On Thu, Oct 04, 2018 at 02:13:19PM -0500, Eric Sandeen wrote:
>
>
> On 10/1/18 12:04 PM, Darrick J. Wong wrote:
> > Here are seven cleanups to the userspace transaction code that make the
> > commit and roll code more closely resemble their kernel counterparts,
> > and fix a number of problems where client code did not check the return
> > values of the transaction functions. The eighth patch fixes a bug in
> > xfs_scrub_all. The patches should apply to Eric's libxfs-4.19-sync
> > branch.
> >
> > --D
> >
>
> You're going to hate me (or maybe not) but in the end I decided to stop
> being stubborn and bring in these patches before the libxfs-4.19 up[date,
> as I should have from the start. You don't have to rebase them again tho,
> I already got through it. I'll diff to your tree to make sure nothing went
> haywire.
But a lot of stuff went 'haywire'... most of which I think is in patch 3
and patch 4 of this series. Patch 8 is also new, though not related.
Oh well, see libxfs-4.19-sync-3 in my djwong/xfsprogs-dev.git tree on
korg.
(Oh and I found another annoying bug in xfs_io's dedupe command; will
send patch for that when the dust settles...)
--D
> For the series with my out of order backporting tweaks,
>
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
>
> Thanks, this was great work.
>
> -Eric
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 9/8] xfs_io: dedupe command should only complain if we don't dedupe anything
2018-10-01 17:04 [PATCH v3 0/8] xfsprogs-4.19: transaction cleanups Darrick J. Wong
` (8 preceding siblings ...)
2018-10-04 19:13 ` [PATCH v3 0/8] xfsprogs-4.19: transaction cleanups Eric Sandeen
@ 2018-10-04 22:27 ` Darrick J. Wong
2018-10-05 21:37 ` Eric Sandeen
9 siblings, 1 reply; 14+ messages in thread
From: Darrick J. Wong @ 2018-10-04 22:27 UTC (permalink / raw)
To: sandeen; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
The dedupe command should only complain about non-matching extents if
the kernel hasn't managed to dedupe /any/ of the input range.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
io/reflink.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/io/reflink.c b/io/reflink.c
index 26eb2e32..72dfe32d 100644
--- a/io/reflink.c
+++ b/io/reflink.c
@@ -70,7 +70,8 @@ dedupe_ioctl(
_(strerror(-info->status)));
goto done;
}
- if (info->status == XFS_EXTENT_DATA_DIFFERS) {
+ if (deduped == 0 &&
+ info->status == XFS_EXTENT_DATA_DIFFERS) {
fprintf(stderr, "XFS_IOC_FILE_EXTENT_SAME: %s\n",
_("Extents did not match."));
goto done;
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 9/8] xfs_io: dedupe command should only complain if we don't dedupe anything
2018-10-04 22:27 ` [PATCH 9/8] xfs_io: dedupe command should only complain if we don't dedupe anything Darrick J. Wong
@ 2018-10-05 21:37 ` Eric Sandeen
2018-10-05 22:23 ` Darrick J. Wong
0 siblings, 1 reply; 14+ messages in thread
From: Eric Sandeen @ 2018-10-05 21:37 UTC (permalink / raw)
To: Darrick J. Wong, sandeen; +Cc: linux-xfs
On 10/4/18 5:27 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> The dedupe command should only complain about non-matching extents if
> the kernel hasn't managed to dedupe /any/ of the input range.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Should it be "no extents matched" then perhaps?
I mean xfs_io is not exactly-quite a purpose-built dedupe tool, but should
we be a bit more specific if we're issuing a message at all?
if (info->status == XFS_EXTENT_DATA_DIFFERS) {
if (deduped == 0)
fprintf(stderr, "XFS_IOC_FILE_EXTENT_SAME: %s\n",
_("No extents matched."));
else
fprintf(stderr, "XFS_IOC_FILE_EXTENT_SAME: %s\n",
_("Some extents did not match."));
}
And the manpage implies that any difference will make it fail:
> map length bytes at offset dst_offset in the open file to the same physical
> blocks that are mapped at offset src_offset in the file src_file, but only
> if the contents of both ranges are identical.
now you're telling me it'll make a best effort? ;) I think the manpage
needs clarification too ...
> ---
> io/reflink.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/io/reflink.c b/io/reflink.c
> index 26eb2e32..72dfe32d 100644
> --- a/io/reflink.c
> +++ b/io/reflink.c
> @@ -70,7 +70,8 @@ dedupe_ioctl(
> _(strerror(-info->status)));
> goto done;
> }
> - if (info->status == XFS_EXTENT_DATA_DIFFERS) {
> + if (deduped == 0 &&
> + info->status == XFS_EXTENT_DATA_DIFFERS) {
> fprintf(stderr, "XFS_IOC_FILE_EXTENT_SAME: %s\n",
> _("Extents did not match."));
> goto done;
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 9/8] xfs_io: dedupe command should only complain if we don't dedupe anything
2018-10-05 21:37 ` Eric Sandeen
@ 2018-10-05 22:23 ` Darrick J. Wong
0 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2018-10-05 22:23 UTC (permalink / raw)
To: Eric Sandeen; +Cc: sandeen, linux-xfs
On Fri, Oct 05, 2018 at 04:37:14PM -0500, Eric Sandeen wrote:
> On 10/4/18 5:27 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > The dedupe command should only complain about non-matching extents if
> > the kernel hasn't managed to dedupe /any/ of the input range.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>
> Should it be "no extents matched" then perhaps?
>
> I mean xfs_io is not exactly-quite a purpose-built dedupe tool, but should
> we be a bit more specific if we're issuing a message at all?
>
> if (info->status == XFS_EXTENT_DATA_DIFFERS) {
> if (deduped == 0)
> fprintf(stderr, "XFS_IOC_FILE_EXTENT_SAME: %s\n",
> _("No extents matched."));
> else
> fprintf(stderr, "XFS_IOC_FILE_EXTENT_SAME: %s\n",
> _("Some extents did not match."));
We don't need to tell the user that /some/ extents did not match.
If we have two files containing "moo cow" and "moo bow", you'd agree
that the first four bytes match, right? So the output should be:
$ xfs_io -c 'dedupe /a 0 0 7' /b
linked 4/7 bytes at offset 0
4 bytes, 1 ops; 0.0000 sec
The "4/7" tells us that some extents didn't match, because we didn't
fulfill the entire range requested. Pretend that this fs can dedupe at
byte alignment.
Now if the files contained "boo cow" and "aaaaaaa", you'd expect:
$ xfs_io -c 'dedupe /a 0 0 7' /b
XFS_IOC_EXTENT_SAME: No extents matched.
And if the files contained "moo cow" and "moo cow":
$ xfs_io -c 'dedupe /a 0 0 7' /b
linked 7/7 bytes at offset 0
7 bytes, 1 ops; 0.0000 sec
> }
>
> And the manpage implies that any difference will make it fail:
>
> > map length bytes at offset dst_offset in the open file to the same physical
> > blocks that are mapped at offset src_offset in the file src_file, but only
> > if the contents of both ranges are identical.
It also says:
"Upon successful completion of this ioctl, the number of bytes
successfully deduplicated is returned in bytes_deduped...."
Which contradicts:
"If even a single byte in the range does not match, the deduplication
request will be ignored and status set to FILE_DEDUPE_RANGE_DIFFERS.
But that makes no sense because if our only choices were really "all the
bytes" or "none of the bytes" then there wouldn't be a bytes_deduped.
> now you're telling me it'll make a best effort? ;) I think the manpage
> needs clarification too ...
Yep.
--D
>
> > ---
>
>
> > io/reflink.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/io/reflink.c b/io/reflink.c
> > index 26eb2e32..72dfe32d 100644
> > --- a/io/reflink.c
> > +++ b/io/reflink.c
> > @@ -70,7 +70,8 @@ dedupe_ioctl(
> > _(strerror(-info->status)));
> > goto done;
> > }
> > - if (info->status == XFS_EXTENT_DATA_DIFFERS) {
> > + if (deduped == 0 &&
> > + info->status == XFS_EXTENT_DATA_DIFFERS) {
> > fprintf(stderr, "XFS_IOC_FILE_EXTENT_SAME: %s\n",
> > _("Extents did not match."));
> > goto done;
> >
^ permalink raw reply [flat|nested] 14+ messages in thread