* [PATCH 1/6] libxfs: port kernel transaction code
2018-09-28 1:04 [PATCH v2 0/6] xfsprogs-4.19: transaction cleanups Darrick J. Wong
@ 2018-09-28 1:04 ` Darrick J. Wong
2018-09-28 1:04 ` [PATCH 2/6] libxfs: fix libxfs_trans_alloc callsite problems Darrick J. Wong
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2018-09-28 1: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] 9+ messages in thread* [PATCH 2/6] libxfs: fix libxfs_trans_alloc callsite problems
2018-09-28 1:04 [PATCH v2 0/6] xfsprogs-4.19: transaction cleanups Darrick J. Wong
2018-09-28 1:04 ` [PATCH 1/6] libxfs: port kernel transaction code Darrick J. Wong
@ 2018-09-28 1:04 ` Darrick J. Wong
2018-09-28 1:04 ` [PATCH 3/6] libxfs: fix xfs_trans_alloc reservation abuse Darrick J. Wong
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2018-09-28 1: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] 9+ messages in thread* [PATCH 3/6] libxfs: fix xfs_trans_alloc reservation abuse
2018-09-28 1:04 [PATCH v2 0/6] xfsprogs-4.19: transaction cleanups Darrick J. Wong
2018-09-28 1:04 ` [PATCH 1/6] libxfs: port kernel transaction code Darrick J. Wong
2018-09-28 1:04 ` [PATCH 2/6] libxfs: fix libxfs_trans_alloc callsite problems Darrick J. Wong
@ 2018-09-28 1:04 ` Darrick J. Wong
2018-09-29 22:31 ` Dave Chinner
2018-09-28 1:05 ` [PATCH 4/6] libxfs: check libxfs_trans_commit return values Darrick J. Wong
` (2 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Darrick J. Wong @ 2018-09-28 1: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>
---
mkfs/proto.c | 19 +++++++++----------
mkfs/xfs_mkfs.c | 4 ++--
repair/phase5.c | 4 ++--
repair/phase6.c | 20 ++++++++------------
repair/rmap.c | 7 +++----
5 files changed, 24 insertions(+), 30 deletions(-)
diff --git a/mkfs/proto.c b/mkfs/proto.c
index 07d019d6..9da0587e 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -123,9 +123,8 @@ 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(mp, &M_RES(mp)->tr_itruncate,
+ r, 0, 0, &tp);
if (i == 0)
return tp;
}
@@ -180,7 +179,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 +190,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(mp, &M_RES(mp)->tr_itruncate, 0, 0, 0, &tp);
if (error)
fail(_("allocating transaction for a file"), error);
libxfs_trans_ijoin(tp, ip, 0);
@@ -610,12 +608,12 @@ rtinit(
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(mp, &M_RES(mp)->tr_itruncate,
+ MKFS_BLOCKRES_INODE, 0, 0, &tp);
if (i)
res_failed(i);
@@ -652,7 +650,7 @@ rtinit(
/*
* Next, give the bitmap file some zero-filled blocks.
*/
- i = -libxfs_trans_alloc(mp, &tres,
+ i = -libxfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate,
mp->m_sb.sb_rbmblocks + (XFS_BM_MAXLEVELS(mp,XFS_DATA_FORK) - 1),
0, 0, &tp);
if (i)
@@ -683,7 +681,7 @@ rtinit(
* Give the summary file some zero-filled blocks.
*/
nsumblocks = mp->m_rsumsize >> mp->m_sb.sb_blocklog;
- i = -libxfs_trans_alloc(mp, &tres,
+ i = -libxfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate,
nsumblocks + (XFS_BM_MAXLEVELS(mp,XFS_DATA_FORK) - 1),
0, 0, &tp);
if (i)
@@ -713,7 +711,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..9ef6e84a 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3674,10 +3674,10 @@ 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(mp, &M_RES(mp)->tr_itruncate,
+ worst_freelist, 0, 0, &tp);
if (c)
res_failed(c);
diff --git a/repair/phase5.c b/repair/phase5.c
index 64f7b6e8..ac2eea54 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,8 @@ 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(mp, &M_RES(mp)->tr_itruncate,
+ 16, 0, 0, &tp);
if (error)
goto out_cancel;
diff --git a/repair/phase6.c b/repair/phase6.c
index d4b6a5cf..ab125d6c 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};
/*
* first set up inode
*/
- i = -libxfs_trans_alloc(mp, &tres, 10, 0, 0, &tp);
+ i = -libxfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate,
+ 10, 0, 0, &tp);
if (i)
res_failed(i);
@@ -579,7 +579,7 @@ mk_rbmino(xfs_mount_t *mp)
* then allocate blocks for file and fill with zeroes (stolen
* from mkfs)
*/
- error = -libxfs_trans_alloc(mp, &tres,
+ error = -libxfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate,
mp->m_sb.sb_rbmblocks + (XFS_BM_MAXLEVELS(mp,XFS_DATA_FORK) - 1),
0, 0, &tp);
if (error)
@@ -619,12 +619,12 @@ 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(mp, &M_RES(mp)->tr_itruncate,
+ 10, 0, 0, &tp);
if (error)
res_failed(error);
@@ -686,13 +686,13 @@ 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(mp, &M_RES(mp)->tr_itruncate,
+ 10, 0, 0, &tp);
if (error)
res_failed(error);
@@ -757,7 +757,6 @@ 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};
/*
* first set up inode
@@ -811,10 +810,7 @@ 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,
+ error = -libxfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate,
mp->m_sb.sb_rbmblocks + (XFS_BM_MAXLEVELS(mp,XFS_DATA_FORK) - 1),
0, 0, &tp);
if (error)
diff --git a/repair/rmap.c b/repair/rmap.c
index f991144a..dbe5b2c8 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,8 @@ 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(mp, &M_RES(mp)->tr_itruncate,
+ 16, 0, 0, &tp);
if (error)
goto err_slab;
@@ -1366,7 +1366,6 @@ fix_freelist(
{
xfs_alloc_arg_t args;
xfs_trans_t *tp;
- struct xfs_trans_res tres = {0};
int flags;
int error;
@@ -1375,7 +1374,7 @@ fix_freelist(
args.agno = agno;
args.alignment = 1;
args.pag = libxfs_perag_get(mp, agno);
- error = -libxfs_trans_alloc(mp, &tres,
+ error = -libxfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate,
libxfs_alloc_min_freelist(mp, args.pag), 0, 0, &tp);
if (error)
do_error(_("failed to fix AGFL on AG %d, error %d\n"),
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/6] libxfs: fix xfs_trans_alloc reservation abuse
2018-09-28 1:04 ` [PATCH 3/6] libxfs: fix xfs_trans_alloc reservation abuse Darrick J. Wong
@ 2018-09-29 22:31 ` Dave Chinner
2018-10-01 16:04 ` Darrick J. Wong
0 siblings, 1 reply; 9+ messages in thread
From: Dave Chinner @ 2018-09-29 22:31 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: sandeen, linux-xfs
On Thu, Sep 27, 2018 at 06:04:49PM -0700, Darrick J. Wong wrote:
> 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>
> ---
> mkfs/proto.c | 19 +++++++++----------
> mkfs/xfs_mkfs.c | 4 ++--
> repair/phase5.c | 4 ++--
> repair/phase6.c | 20 ++++++++------------
> repair/rmap.c | 7 +++----
> 5 files changed, 24 insertions(+), 30 deletions(-)
>
>
> diff --git a/mkfs/proto.c b/mkfs/proto.c
> index 07d019d6..9da0587e 100644
> --- a/mkfs/proto.c
> +++ b/mkfs/proto.c
> @@ -123,9 +123,8 @@ 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(mp, &M_RES(mp)->tr_itruncate,
I'm wondering if this should explicitly call out that it's a dummy
reservation rather than using the itruncate reservation? e.g. these
places use:
i = -libxfs_trans_alloc_perm(mp, blks, rtblks, flags, &tp);
And the implementation of this function then goes and uses the
itruncate reservation with a comment explaining what thay is used
(open to a better name - "dummy" doesn't seem right - perm, rolling,
deferred, etc all seem appropriate to indicate that it's an
allocation for a permanent transaction type for rolling/defered
transactions).
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/6] libxfs: fix xfs_trans_alloc reservation abuse
2018-09-29 22:31 ` Dave Chinner
@ 2018-10-01 16:04 ` Darrick J. Wong
0 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2018-10-01 16:04 UTC (permalink / raw)
To: Dave Chinner; +Cc: sandeen, linux-xfs
On Sun, Sep 30, 2018 at 08:31:31AM +1000, Dave Chinner wrote:
> On Thu, Sep 27, 2018 at 06:04:49PM -0700, Darrick J. Wong wrote:
> > 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>
> > ---
> > mkfs/proto.c | 19 +++++++++----------
> > mkfs/xfs_mkfs.c | 4 ++--
> > repair/phase5.c | 4 ++--
> > repair/phase6.c | 20 ++++++++------------
> > repair/rmap.c | 7 +++----
> > 5 files changed, 24 insertions(+), 30 deletions(-)
> >
> >
> > diff --git a/mkfs/proto.c b/mkfs/proto.c
> > index 07d019d6..9da0587e 100644
> > --- a/mkfs/proto.c
> > +++ b/mkfs/proto.c
> > @@ -123,9 +123,8 @@ 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(mp, &M_RES(mp)->tr_itruncate,
>
> I'm wondering if this should explicitly call out that it's a dummy
> reservation rather than using the itruncate reservation? e.g. these
> places use:
>
> i = -libxfs_trans_alloc_perm(mp, blks, rtblks, flags, &tp);
>
> And the implementation of this function then goes and uses the
> itruncate reservation with a comment explaining what thay is used
>
> (open to a better name - "dummy" doesn't seem right - perm, rolling,
> deferred, etc all seem appropriate to indicate that it's an
> allocation for a permanent transaction type for rolling/defered
> transactions).
I don't necessarily like the long name, but
"libxfs_trans_alloc_rollable" seems the most descriptive to me.
I do like using a helper instead of opencoding tr_itruncate
everywhere.
--D
>
> Cheers,
>
> Dave.
>
> --
> Dave Chinner
> david@fromorbit.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/6] libxfs: check libxfs_trans_commit return values
2018-09-28 1:04 [PATCH v2 0/6] xfsprogs-4.19: transaction cleanups Darrick J. Wong
` (2 preceding siblings ...)
2018-09-28 1:04 ` [PATCH 3/6] libxfs: fix xfs_trans_alloc reservation abuse Darrick J. Wong
@ 2018-09-28 1:05 ` Darrick J. Wong
2018-09-28 1:05 ` [PATCH 5/6] libxfs: clean up IRELE/iput callsites Darrick J. Wong
2018-09-28 1:05 ` [PATCH 6/6] libxfs: track transaction block reservation usage like the kernel Darrick J. Wong
5 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2018-09-28 1:05 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 9da0587e..5999fd2e 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -212,7 +212,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
@@ -474,7 +476,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;
@@ -552,7 +556,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.
@@ -575,7 +581,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);
}
@@ -645,7 +655,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.
@@ -675,7 +688,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.
@@ -704,7 +720,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.
@@ -723,7 +742,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 9ef6e84a..b04a705f 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3690,7 +3690,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 ab125d6c..0a52cf96 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);
}
@@ -669,7 +676,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);
}
@@ -738,7 +747,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);
}
@@ -803,7 +814,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
@@ -835,7 +848,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);
}
@@ -900,7 +918,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),
@@ -1030,7 +1051,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);
@@ -1128,7 +1153,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);
@@ -1168,7 +1196,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 {
@@ -1199,7 +1230,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);
@@ -1335,7 +1369,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;
@@ -1366,7 +1403,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;
@@ -1412,7 +1452,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);
}
/*
@@ -1885,7 +1928,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);
@@ -2901,7 +2947,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);
}
@@ -2942,7 +2990,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) {
@@ -2995,7 +3046,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 dbe5b2c8..bd24b527 100644
--- a/repair/rmap.c
+++ b/repair/rmap.c
@@ -1411,7 +1411,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] 9+ messages in thread* [PATCH 5/6] libxfs: clean up IRELE/iput callsites
2018-09-28 1:04 [PATCH v2 0/6] xfsprogs-4.19: transaction cleanups Darrick J. Wong
` (3 preceding siblings ...)
2018-09-28 1:05 ` [PATCH 4/6] libxfs: check libxfs_trans_commit return values Darrick J. Wong
@ 2018-09-28 1:05 ` Darrick J. Wong
2018-09-28 1:05 ` [PATCH 6/6] libxfs: track transaction block reservation usage like the kernel Darrick J. Wong
5 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2018-09-28 1:05 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 5999fd2e..d5ffe969 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -480,7 +480,7 @@ parseproto(
if (error)
fail(_("Space preallocation failed."), error);
rsvfile(mp, ip, llen);
- IRELE(ip);
+ libxfs_irele(ip);
return;
case IF_BLOCK:
@@ -574,7 +574,7 @@ parseproto(
break;
parseproto(mp, ip, fsxp, pp, name);
}
- IRELE(ip);
+ libxfs_irele(ip);
return;
default:
ASSERT(0);
@@ -586,7 +586,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 0a52cf96..3d43935c 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
@@ -679,7 +679,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);
}
@@ -735,7 +735,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);
}
@@ -750,7 +750,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);
}
@@ -854,7 +854,7 @@ mk_rsumino(xfs_mount_t *mp)
_("allocation of the realtime summary ino failed, error = %d\n"),
error);
}
- IRELE(ip);
+ libxfs_irele(ip);
}
/*
@@ -922,7 +922,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));
@@ -1056,8 +1056,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);
@@ -1235,8 +1235,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
@@ -3052,7 +3052,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] 9+ messages in thread* [PATCH 6/6] libxfs: track transaction block reservation usage like the kernel
2018-09-28 1:04 [PATCH v2 0/6] xfsprogs-4.19: transaction cleanups Darrick J. Wong
` (4 preceding siblings ...)
2018-09-28 1:05 ` [PATCH 5/6] libxfs: clean up IRELE/iput callsites Darrick J. Wong
@ 2018-09-28 1:05 ` Darrick J. Wong
5 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2018-09-28 1: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 54546da8..d0fc2a84 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 11a2098d..5ebca844 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);
@@ -775,6 +778,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] 9+ messages in thread