From 3a1af6c317d0a55524f39079183be107be4c1f39 Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Tue, 26 Jan 2021 16:33:29 -0800 Subject: xfs: refactor common transaction/inode/quota allocation idiom Create a new helper xfs_trans_alloc_inode that allocates a transaction, locks and joins an inode to it, and then reserves the appropriate amount of quota against that transction. Then replace all the open-coded idioms with a single call to this helper. Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig Reviewed-by: Brian Foster --- fs/xfs/xfs_trans.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) (limited to 'fs/xfs/xfs_trans.c') diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index e72730f85af1..156b9ed8534f 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -20,6 +20,7 @@ #include "xfs_trace.h" #include "xfs_error.h" #include "xfs_defer.h" +#include "xfs_inode.h" kmem_zone_t *xfs_trans_zone; @@ -1024,3 +1025,50 @@ xfs_trans_roll( tres.tr_logflags = XFS_TRANS_PERM_LOG_RES; return xfs_trans_reserve(*tpp, &tres, 0, 0); } + +/* + * Allocate an transaction, lock and join the inode to it, and reserve quota. + * + * The caller must ensure that the on-disk dquots attached to this inode have + * already been allocated and initialized. The caller is responsible for + * releasing ILOCK_EXCL if a new transaction is returned. + */ +int +xfs_trans_alloc_inode( + struct xfs_inode *ip, + struct xfs_trans_res *resv, + unsigned int dblocks, + bool force, + struct xfs_trans **tpp) +{ + struct xfs_trans *tp; + struct xfs_mount *mp = ip->i_mount; + int error; + + error = xfs_trans_alloc(mp, resv, dblocks, 0, + force ? XFS_TRANS_RESERVE : 0, &tp); + if (error) + return error; + + xfs_ilock(ip, XFS_ILOCK_EXCL); + xfs_trans_ijoin(tp, ip, 0); + + error = xfs_qm_dqattach_locked(ip, false); + if (error) { + /* Caller should have allocated the dquots! */ + ASSERT(error != -ENOENT); + goto out_cancel; + } + + error = xfs_trans_reserve_quota_nblks(tp, ip, dblocks, 0, force); + if (error) + goto out_cancel; + + *tpp = tp; + return 0; + +out_cancel: + xfs_trans_cancel(tp); + xfs_iunlock(ip, XFS_ILOCK_EXCL); + return error; +} -- cgit v1.2.3 From 3de4eb106fcc97f086b78bd17a0c3529691e8259 Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Tue, 26 Jan 2021 16:44:07 -0800 Subject: xfs: allow reservation of rtblocks with xfs_trans_alloc_inode Make it so that we can reserve rt blocks with the xfs_trans_alloc_inode wrapper function, then convert a few more callsites. Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig Reviewed-by: Brian Foster --- fs/xfs/xfs_trans.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'fs/xfs/xfs_trans.c') diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index 156b9ed8534f..151f274eee43 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -1038,6 +1038,7 @@ xfs_trans_alloc_inode( struct xfs_inode *ip, struct xfs_trans_res *resv, unsigned int dblocks, + unsigned int rblocks, bool force, struct xfs_trans **tpp) { @@ -1045,7 +1046,8 @@ xfs_trans_alloc_inode( struct xfs_mount *mp = ip->i_mount; int error; - error = xfs_trans_alloc(mp, resv, dblocks, 0, + error = xfs_trans_alloc(mp, resv, dblocks, + rblocks / mp->m_sb.sb_rextsize, force ? XFS_TRANS_RESERVE : 0, &tp); if (error) return error; @@ -1060,7 +1062,7 @@ xfs_trans_alloc_inode( goto out_cancel; } - error = xfs_trans_reserve_quota_nblks(tp, ip, dblocks, 0, force); + error = xfs_trans_reserve_quota_nblks(tp, ip, dblocks, rblocks, force); if (error) goto out_cancel; -- cgit v1.2.3 From f2f7b9ff62a28928f6fe2bd55cdb4d4b02ab7477 Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Wed, 27 Jan 2021 12:07:57 -0800 Subject: xfs: refactor inode creation transaction/inode/quota allocation idiom For file creation, create a new helper xfs_trans_alloc_icreate that allocates a transaction and reserves the appropriate amount of quota against that transction. Replace all the open-coded idioms with a single call to this helper so that we can contain the retry loops in the next patchset. This changes the locking behavior for non-tempfile creation slightly, in that we now make the quota reservation without holding the directory ILOCK. While the dquots chosen for inode creation are based on the directory state at a given point in time, the directory ILOCK was released as soon as the dquot references are picked up. Hence it was never necessary to hold the directory ILOCK for the quota reservation. Signed-off-by: Darrick J. Wong Reviewed-by: Brian Foster Reviewed-by: Christoph Hellwig --- fs/xfs/xfs_trans.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) (limited to 'fs/xfs/xfs_trans.c') diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index 151f274eee43..6c68635cc6ac 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -21,6 +21,8 @@ #include "xfs_error.h" #include "xfs_defer.h" #include "xfs_inode.h" +#include "xfs_dquot_item.h" +#include "xfs_dquot.h" kmem_zone_t *xfs_trans_zone; @@ -1074,3 +1076,34 @@ out_cancel: xfs_iunlock(ip, XFS_ILOCK_EXCL); return error; } + +/* + * Allocate an transaction in preparation for inode creation by reserving quota + * against the given dquots. Callers are not required to hold any inode locks. + */ +int +xfs_trans_alloc_icreate( + struct xfs_mount *mp, + struct xfs_trans_res *resv, + struct xfs_dquot *udqp, + struct xfs_dquot *gdqp, + struct xfs_dquot *pdqp, + unsigned int dblocks, + struct xfs_trans **tpp) +{ + struct xfs_trans *tp; + int error; + + error = xfs_trans_alloc(mp, resv, dblocks, 0, 0, &tp); + if (error) + return error; + + error = xfs_trans_reserve_quota_icreate(tp, udqp, gdqp, pdqp, dblocks); + if (error) { + xfs_trans_cancel(tp); + return error; + } + + *tpp = tp; + return 0; +} -- cgit v1.2.3 From 7317a03df703f7cdae3ae9e9635a0ef45849fe09 Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Fri, 29 Jan 2021 11:32:09 -0800 Subject: xfs: refactor inode ownership change transaction/inode/quota allocation idiom For file ownership (uid, gid, prid) changes, create a new helper xfs_trans_alloc_ichange that allocates a transaction and reserves the appropriate amount of quota against that transction in preparation for a change of user, group, or project id. Replace all the open-coded idioms with a single call to this helper so that we can contain the retry loops in the next patchset. This changes the locking behavior for ichange transactions slightly. Since tr_ichange does not have a permanent reservation and cannot roll, we pass XFS_ILOCK_EXCL to ijoin so that the inode will be unlocked automatically at commit time. Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig Reviewed-by: Brian Foster --- fs/xfs/xfs_trans.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) (limited to 'fs/xfs/xfs_trans.c') diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index 6c68635cc6ac..60672b5545c9 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -1107,3 +1107,65 @@ xfs_trans_alloc_icreate( *tpp = tp; return 0; } + +/* + * Allocate an transaction, lock and join the inode to it, and reserve quota + * in preparation for inode attribute changes that include uid, gid, or prid + * changes. + * + * The caller must ensure that the on-disk dquots attached to this inode have + * already been allocated and initialized. The ILOCK will be dropped when the + * transaction is committed or cancelled. + */ +int +xfs_trans_alloc_ichange( + struct xfs_inode *ip, + struct xfs_dquot *udqp, + struct xfs_dquot *gdqp, + struct xfs_dquot *pdqp, + bool force, + struct xfs_trans **tpp) +{ + struct xfs_trans *tp; + struct xfs_mount *mp = ip->i_mount; + int error; + + error = xfs_trans_alloc(mp, &M_RES(mp)->tr_ichange, 0, 0, 0, &tp); + if (error) + return error; + + xfs_ilock(ip, XFS_ILOCK_EXCL); + xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL); + + error = xfs_qm_dqattach_locked(ip, false); + if (error) { + /* Caller should have allocated the dquots! */ + ASSERT(error != -ENOENT); + goto out_cancel; + } + + /* + * For each quota type, skip quota reservations if the inode's dquots + * now match the ones that came from the caller, or the caller didn't + * pass one in. + */ + if (udqp == ip->i_udquot) + udqp = NULL; + if (gdqp == ip->i_gdquot) + gdqp = NULL; + if (pdqp == ip->i_pdquot) + pdqp = NULL; + if (udqp || gdqp || pdqp) { + error = xfs_qm_vop_chown_reserve(tp, ip, udqp, gdqp, pdqp, + force ? XFS_QMOPT_FORCE_RES : 0); + if (error) + goto out_cancel; + } + + *tpp = tp; + return 0; + +out_cancel: + xfs_trans_cancel(tp); + return error; +} -- cgit v1.2.3 From 5c615f0feb9a559abd08da0842d6fcfee105b7e3 Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Mon, 1 Feb 2021 10:38:51 -0800 Subject: xfs: remove xfs_qm_vop_chown_reserve Now that the only caller of this function is xfs_trans_alloc_ichange, just open-code the meat of _chown_reserve in that caller. Drop the (redundant) [ugp]id checks because xfs has a 1:1 relationship between quota ids and incore dquots. Signed-off-by: Darrick J. Wong Reviewed-by: Brian Foster Reviewed-by: Christoph Hellwig --- fs/xfs/xfs_trans.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'fs/xfs/xfs_trans.c') diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index 60672b5545c9..29dca1bc4c1a 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -1156,8 +1156,20 @@ xfs_trans_alloc_ichange( if (pdqp == ip->i_pdquot) pdqp = NULL; if (udqp || gdqp || pdqp) { - error = xfs_qm_vop_chown_reserve(tp, ip, udqp, gdqp, pdqp, - force ? XFS_QMOPT_FORCE_RES : 0); + unsigned int qflags = XFS_QMOPT_RES_REGBLKS; + + if (force) + qflags |= XFS_QMOPT_FORCE_RES; + + /* + * Reserve enough quota to handle blocks on disk and reserved + * for a delayed allocation. We'll actually transfer the + * delalloc reservation between dquots at chown time, even + * though that part is only semi-transactional. + */ + error = xfs_trans_reserve_quota_bydquots(tp, mp, udqp, gdqp, + pdqp, ip->i_d.di_nblocks + ip->i_delayed_blks, + 1, qflags); if (error) goto out_cancel; } -- cgit v1.2.3 From 766aabd59929cd05fc1a249f376e4395bed93d30 Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Fri, 22 Jan 2021 16:48:37 -0800 Subject: xfs: flush eof/cowblocks if we can't reserve quota for file blocks If a fs modification (data write, reflink, xattr set, fallocate, etc.) is unable to reserve enough quota to handle the modification, try clearing whatever space the filesystem might have been hanging onto in the hopes of speeding up the filesystem. The flushing behavior will become particularly important when we add deferred inode inactivation because that will increase the amount of space that isn't actively tied to user data. Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig Reviewed-by: Brian Foster --- fs/xfs/xfs_trans.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'fs/xfs/xfs_trans.c') diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index 29dca1bc4c1a..4071bbed2d48 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -23,6 +23,7 @@ #include "xfs_inode.h" #include "xfs_dquot_item.h" #include "xfs_dquot.h" +#include "xfs_icache.h" kmem_zone_t *xfs_trans_zone; @@ -1046,8 +1047,10 @@ xfs_trans_alloc_inode( { struct xfs_trans *tp; struct xfs_mount *mp = ip->i_mount; + bool retried = false; int error; +retry: error = xfs_trans_alloc(mp, resv, dblocks, rblocks / mp->m_sb.sb_rextsize, force ? XFS_TRANS_RESERVE : 0, &tp); @@ -1065,6 +1068,13 @@ xfs_trans_alloc_inode( } error = xfs_trans_reserve_quota_nblks(tp, ip, dblocks, rblocks, force); + if ((error == -EDQUOT || error == -ENOSPC) && !retried) { + xfs_trans_cancel(tp); + xfs_iunlock(ip, XFS_ILOCK_EXCL); + xfs_blockgc_free_quota(ip, 0); + retried = true; + goto retry; + } if (error) goto out_cancel; -- cgit v1.2.3 From c237dd7c709432611a7642ca10c2a0c8c48ea313 Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Fri, 22 Jan 2021 16:48:37 -0800 Subject: xfs: flush eof/cowblocks if we can't reserve quota for inode creation If an inode creation is unable to reserve enough quota to handle the modification, try clearing whatever space the filesystem might have been hanging onto in the hopes of speeding up the filesystem. The flushing behavior will become particularly important when we add deferred inode inactivation because that will increase the amount of space that isn't actively tied to user data. Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig Reviewed-by: Brian Foster --- fs/xfs/xfs_trans.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'fs/xfs/xfs_trans.c') diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index 4071bbed2d48..b29434199079 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -1102,13 +1102,21 @@ xfs_trans_alloc_icreate( struct xfs_trans **tpp) { struct xfs_trans *tp; + bool retried = false; int error; +retry: error = xfs_trans_alloc(mp, resv, dblocks, 0, 0, &tp); if (error) return error; error = xfs_trans_reserve_quota_icreate(tp, udqp, gdqp, pdqp, dblocks); + if ((error == -EDQUOT || error == -ENOSPC) && !retried) { + xfs_trans_cancel(tp); + xfs_blockgc_free_dquots(mp, udqp, gdqp, pdqp, 0); + retried = true; + goto retry; + } if (error) { xfs_trans_cancel(tp); return error; -- cgit v1.2.3 From 758303d1449965819661048e9e31f32d61888f70 Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Fri, 22 Jan 2021 16:48:38 -0800 Subject: xfs: flush eof/cowblocks if we can't reserve quota for chown If a file user, group, or project change is unable to reserve enough quota to handle the modification, try clearing whatever space the filesystem might have been hanging onto in the hopes of speeding up the filesystem. The flushing behavior will become particularly important when we add deferred inode inactivation because that will increase the amount of space that isn't actively tied to user data. Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig Reviewed-by: Brian Foster --- fs/xfs/xfs_trans.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) (limited to 'fs/xfs/xfs_trans.c') diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index b29434199079..a23400f59e7d 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -1138,16 +1138,21 @@ retry: int xfs_trans_alloc_ichange( struct xfs_inode *ip, - struct xfs_dquot *udqp, - struct xfs_dquot *gdqp, - struct xfs_dquot *pdqp, + struct xfs_dquot *new_udqp, + struct xfs_dquot *new_gdqp, + struct xfs_dquot *new_pdqp, bool force, struct xfs_trans **tpp) { struct xfs_trans *tp; struct xfs_mount *mp = ip->i_mount; + struct xfs_dquot *udqp; + struct xfs_dquot *gdqp; + struct xfs_dquot *pdqp; + bool retried = false; int error; +retry: error = xfs_trans_alloc(mp, &M_RES(mp)->tr_ichange, 0, 0, 0, &tp); if (error) return error; @@ -1165,14 +1170,12 @@ xfs_trans_alloc_ichange( /* * For each quota type, skip quota reservations if the inode's dquots * now match the ones that came from the caller, or the caller didn't - * pass one in. + * pass one in. The inode's dquots can change if we drop the ILOCK to + * perform a blockgc scan, so we must preserve the caller's arguments. */ - if (udqp == ip->i_udquot) - udqp = NULL; - if (gdqp == ip->i_gdquot) - gdqp = NULL; - if (pdqp == ip->i_pdquot) - pdqp = NULL; + udqp = (new_udqp != ip->i_udquot) ? new_udqp : NULL; + gdqp = (new_gdqp != ip->i_gdquot) ? new_gdqp : NULL; + pdqp = (new_pdqp != ip->i_pdquot) ? new_pdqp : NULL; if (udqp || gdqp || pdqp) { unsigned int qflags = XFS_QMOPT_RES_REGBLKS; @@ -1188,6 +1191,12 @@ xfs_trans_alloc_ichange( error = xfs_trans_reserve_quota_bydquots(tp, mp, udqp, gdqp, pdqp, ip->i_d.di_nblocks + ip->i_delayed_blks, 1, qflags); + if ((error == -EDQUOT || error == -ENOSPC) && !retried) { + xfs_trans_cancel(tp); + xfs_blockgc_free_dquots(mp, udqp, gdqp, pdqp, 0); + retried = true; + goto retry; + } if (error) goto out_cancel; } -- cgit v1.2.3 From a1a7d05a05765eec042942a5c360e909c0dd0131 Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Fri, 22 Jan 2021 16:48:39 -0800 Subject: xfs: flush speculative space allocations when we run out of space If a fs modification (creation, file write, reflink, etc.) is unable to reserve enough space to handle the modification, try clearing whatever space the filesystem might have been hanging onto in the hopes of speeding up the filesystem. The flushing behavior will become particularly important when we add deferred inode inactivation because that will increase the amount of space that isn't actively tied to user data. Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig Reviewed-by: Brian Foster --- fs/xfs/xfs_trans.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'fs/xfs/xfs_trans.c') diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index a23400f59e7d..44f72c09c203 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -289,6 +289,17 @@ xfs_trans_alloc( tp->t_firstblock = NULLFSBLOCK; error = xfs_trans_reserve(tp, resp, blocks, rtextents); + if (error == -ENOSPC) { + /* + * We weren't able to reserve enough space for the transaction. + * Flush the other speculative space allocations to free space. + * Do not perform a synchronous scan because callers can hold + * other locks. + */ + error = xfs_blockgc_free_space(mp, NULL); + if (!error) + error = xfs_trans_reserve(tp, resp, blocks, rtextents); + } if (error) { xfs_trans_cancel(tp); return error; -- cgit v1.2.3 From 9febcda6f8d1db9f922945d026bb838864b1b6d5 Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Fri, 19 Feb 2021 09:18:06 -0800 Subject: xfs: don't nest transactions when scanning for eofblocks Brian Foster reported a lockdep warning on xfs/167: ============================================ WARNING: possible recursive locking detected 5.11.0-rc4 #35 Tainted: G W I -------------------------------------------- fsstress/17733 is trying to acquire lock: ffff8e0fd1d90650 (sb_internal){++++}-{0:0}, at: xfs_free_eofblocks+0x104/0x1d0 [xfs] but task is already holding lock: ffff8e0fd1d90650 (sb_internal){++++}-{0:0}, at: xfs_trans_alloc_inode+0x5f/0x160 [xfs] stack backtrace: CPU: 38 PID: 17733 Comm: fsstress Tainted: G W I 5.11.0-rc4 #35 Hardware name: Dell Inc. PowerEdge R740/01KPX8, BIOS 1.6.11 11/20/2018 Call Trace: dump_stack+0x8b/0xb0 __lock_acquire.cold+0x159/0x2ab lock_acquire+0x116/0x370 xfs_trans_alloc+0x1ad/0x310 [xfs] xfs_free_eofblocks+0x104/0x1d0 [xfs] xfs_blockgc_scan_inode+0x24/0x60 [xfs] xfs_inode_walk_ag+0x202/0x4b0 [xfs] xfs_inode_walk+0x66/0xc0 [xfs] xfs_trans_alloc+0x160/0x310 [xfs] xfs_trans_alloc_inode+0x5f/0x160 [xfs] xfs_alloc_file_space+0x105/0x300 [xfs] xfs_file_fallocate+0x270/0x460 [xfs] vfs_fallocate+0x14d/0x3d0 __x64_sys_fallocate+0x3e/0x70 do_syscall_64+0x33/0x40 entry_SYSCALL_64_after_hwframe+0x44/0xa9 The cause of this is the new code that spurs a scan to garbage collect speculative preallocations if we fail to reserve enough blocks while allocating a transaction. While the warning itself is a fairly benign lockdep complaint, it does expose a potential livelock if the rwsem behavior ever changes with regards to nesting read locks when someone's waiting for a write lock. Fix this by freeing the transaction and jumping back to xfs_trans_alloc like this patch in the V4 submission[1]. [1] https://lore.kernel.org/linux-xfs/161142798066.2171939.9311024588681972086.stgit@magnolia/ Fixes: a1a7d05a0576 ("xfs: flush speculative space allocations when we run out of space") Reported-by: Brian Foster Signed-off-by: Darrick J. Wong Reviewed-by: Brian Foster Reviewed-by: Allison Henderson Reviewed-by: Christoph Hellwig --- fs/xfs/xfs_trans.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'fs/xfs/xfs_trans.c') diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index 44f72c09c203..377f3961d7ed 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -260,6 +260,7 @@ xfs_trans_alloc( struct xfs_trans **tpp) { struct xfs_trans *tp; + bool want_retry = true; int error; /* @@ -267,6 +268,7 @@ xfs_trans_alloc( * GFP_NOFS allocation context so that we avoid lockdep false positives * by doing GFP_KERNEL allocations inside sb_start_intwrite(). */ +retry: tp = kmem_cache_zalloc(xfs_trans_zone, GFP_KERNEL | __GFP_NOFAIL); if (!(flags & XFS_TRANS_NO_WRITECOUNT)) sb_start_intwrite(mp->m_super); @@ -289,7 +291,9 @@ xfs_trans_alloc( tp->t_firstblock = NULLFSBLOCK; error = xfs_trans_reserve(tp, resp, blocks, rtextents); - if (error == -ENOSPC) { + if (error == -ENOSPC && want_retry) { + xfs_trans_cancel(tp); + /* * We weren't able to reserve enough space for the transaction. * Flush the other speculative space allocations to free space. @@ -297,8 +301,11 @@ xfs_trans_alloc( * other locks. */ error = xfs_blockgc_free_space(mp, NULL); - if (!error) - error = xfs_trans_reserve(tp, resp, blocks, rtextents); + if (error) + return error; + + want_retry = false; + goto retry; } if (error) { xfs_trans_cancel(tp); -- cgit v1.2.3 From 756b1c343333a5aefcc26b0409f3fd16f72281bf Mon Sep 17 00:00:00 2001 From: Dave Chinner Date: Tue, 23 Feb 2021 10:26:06 -0800 Subject: xfs: use current->journal_info for detecting transaction recursion Because the iomap code using PF_MEMALLOC_NOFS to detect transaction recursion in XFS is just wrong. Remove it from the iomap code and replace it with XFS specific internal checks using current->journal_info instead. [djwong: This change also realigns the lifetime of NOFS flag changes to match the incore transaction, instead of the inconsistent scheme we have now.] Fixes: 9070733b4efa ("xfs: abstract PF_FSTRANS to PF_MEMALLOC_NOFS") Signed-off-by: Dave Chinner Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig --- fs/xfs/xfs_trans.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) (limited to 'fs/xfs/xfs_trans.c') diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index 377f3961d7ed..b22a09e9daee 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -72,6 +72,7 @@ xfs_trans_free( xfs_extent_busy_clear(tp->t_mountp, &tp->t_busy, false); trace_xfs_trans_free(tp, _RET_IP_); + xfs_trans_clear_context(tp); if (!(tp->t_flags & XFS_TRANS_NO_WRITECOUNT)) sb_end_intwrite(tp->t_mountp->m_super); xfs_trans_free_dqinfo(tp); @@ -123,7 +124,8 @@ xfs_trans_dup( ntp->t_rtx_res = tp->t_rtx_res - tp->t_rtx_res_used; tp->t_rtx_res = tp->t_rtx_res_used; - ntp->t_pflags = tp->t_pflags; + + xfs_trans_switch_context(tp, ntp); /* move deferred ops over to the new tp */ xfs_defer_move(ntp, tp); @@ -157,9 +159,6 @@ xfs_trans_reserve( int error = 0; bool rsvd = (tp->t_flags & XFS_TRANS_RESERVE) != 0; - /* Mark this thread as being in a transaction */ - current_set_flags_nested(&tp->t_pflags, PF_MEMALLOC_NOFS); - /* * Attempt to reserve the needed disk blocks by decrementing * the number needed from the number available. This will @@ -167,10 +166,8 @@ xfs_trans_reserve( */ if (blocks > 0) { error = xfs_mod_fdblocks(mp, -((int64_t)blocks), rsvd); - if (error != 0) { - current_restore_flags_nested(&tp->t_pflags, PF_MEMALLOC_NOFS); + if (error != 0) return -ENOSPC; - } tp->t_blk_res += blocks; } @@ -244,9 +241,6 @@ undo_blocks: xfs_mod_fdblocks(mp, (int64_t)blocks, rsvd); tp->t_blk_res = 0; } - - current_restore_flags_nested(&tp->t_pflags, PF_MEMALLOC_NOFS); - return error; } @@ -272,6 +266,7 @@ retry: tp = kmem_cache_zalloc(xfs_trans_zone, GFP_KERNEL | __GFP_NOFAIL); if (!(flags & XFS_TRANS_NO_WRITECOUNT)) sb_start_intwrite(mp->m_super); + xfs_trans_set_context(tp); /* * Zero-reservation ("empty") transactions can't modify anything, so @@ -900,7 +895,6 @@ __xfs_trans_commit( xfs_log_commit_cil(mp, tp, &commit_lsn, regrant); - current_restore_flags_nested(&tp->t_pflags, PF_MEMALLOC_NOFS); xfs_trans_free(tp); /* @@ -932,7 +926,6 @@ out_unreserve: xfs_log_ticket_ungrant(mp->m_log, tp->t_ticket); tp->t_ticket = NULL; } - current_restore_flags_nested(&tp->t_pflags, PF_MEMALLOC_NOFS); xfs_trans_free_items(tp, !!error); xfs_trans_free(tp); @@ -992,9 +985,6 @@ xfs_trans_cancel( tp->t_ticket = NULL; } - /* mark this thread as no longer being in a transaction */ - current_restore_flags_nested(&tp->t_pflags, PF_MEMALLOC_NOFS); - xfs_trans_free_items(tp, dirty); xfs_trans_free(tp); } -- cgit v1.2.3