Skip to content

Commit 2056446

Browse files
committed
Merge: [xfstests xfs/017] xfs_repair fails and hit XFS: Assertion failed: 0, file: fs/xfs/xfs_icache.c, line: 1840
MR: https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-9/-/merge_requests/6047 JIRA: https://issues.redhat.com/browse/RHEL-56816 ``` xfs: fix freeing speculative preallocations for preallocated files xfs_can_free_eofblocks returns false for files that have persistent preallocations unless the force flag is passed and there are delayed blocks. This means it won't free delalloc reservations for files with persistent preallocations unless the force flag is set, and it will also free the persistent preallocations if the force flag is set and the file happens to have delayed allocations. Both of these are bad, so do away with the force flag and always free only post-EOF delayed allocations for files with the XFS_DIFLAG_PREALLOC or APPEND flags set. Signed-off-by: Christoph Hellwig <[email protected]> Reviewed-by: Darrick J. Wong <[email protected]> Signed-off-by: Chandan Babu R <[email protected]> (cherry picked from commit 610b291) ``` Signed-off-by: CKI Backport Bot <[email protected]> --- <small>Created 2024-12-17 14:02 UTC by backporter - [KWF FAQ](https://red.ht/kernel_workflow_doc) - [Slack #team-kernel-workflow](https://redhat-internal.slack.com/archives/C04LRUPMJQ5) - [Source](https://gitlab.com/cki-project/kernel-workflow/-/blob/main/webhook/utils/backporter.py) - [Documentation](https://gitlab.com/cki-project/kernel-workflow/-/blob/main/docs/README.backporter.md) - [Report an issue](https://gitlab.com/cki-project/kernel-workflow/-/issues/new?issue%5Btitle%5D=backporter%20webhook%20issue)</small> Approved-by: Brian Foster <[email protected]> Approved-by: Carlos Maiolino <[email protected]> Approved-by: CKI KWF Bot <[email protected]> Merged-by: Augusto Caringi <[email protected]>
2 parents 789aa3e + c549212 commit 2056446

File tree

4 files changed

+28
-20
lines changed

4 files changed

+28
-20
lines changed

fs/xfs/xfs_bmap_util.c

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -489,13 +489,11 @@ xfs_bmap_punch_delalloc_range(
489489

490490
/*
491491
* Test whether it is appropriate to check an inode for and free post EOF
492-
* blocks. The 'force' parameter determines whether we should also consider
493-
* regular files that are marked preallocated or append-only.
492+
* blocks.
494493
*/
495494
bool
496495
xfs_can_free_eofblocks(
497-
struct xfs_inode *ip,
498-
bool force)
496+
struct xfs_inode *ip)
499497
{
500498
struct xfs_bmbt_irec imap;
501499
struct xfs_mount *mp = ip->i_mount;
@@ -529,11 +527,11 @@ xfs_can_free_eofblocks(
529527
return false;
530528

531529
/*
532-
* Do not free real preallocated or append-only files unless the file
533-
* has delalloc blocks and we are forced to remove them.
530+
* Only free real extents for inodes with persistent preallocations or
531+
* the append-only flag.
534532
*/
535533
if (ip->i_diflags & (XFS_DIFLAG_PREALLOC | XFS_DIFLAG_APPEND))
536-
if (!force || ip->i_delayed_blks == 0)
534+
if (ip->i_delayed_blks == 0)
537535
return false;
538536

539537
/*
@@ -587,6 +585,22 @@ xfs_free_eofblocks(
587585
/* Wait on dio to ensure i_size has settled. */
588586
inode_dio_wait(VFS_I(ip));
589587

588+
/*
589+
* For preallocated files only free delayed allocations.
590+
*
591+
* Note that this means we also leave speculative preallocations in
592+
* place for preallocated files.
593+
*/
594+
if (ip->i_diflags & (XFS_DIFLAG_PREALLOC | XFS_DIFLAG_APPEND)) {
595+
if (ip->i_delayed_blks) {
596+
xfs_bmap_punch_delalloc_range(ip,
597+
round_up(XFS_ISIZE(ip), mp->m_sb.sb_blocksize),
598+
LLONG_MAX);
599+
}
600+
xfs_inode_clear_eofblocks_tag(ip);
601+
return 0;
602+
}
603+
590604
error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, 0, 0, 0, &tp);
591605
if (error) {
592606
ASSERT(xfs_is_shutdown(mp));
@@ -900,7 +914,7 @@ xfs_prepare_shift(
900914
* Trim eofblocks to avoid shifting uninitialized post-eof preallocation
901915
* into the accessible region of the file.
902916
*/
903-
if (xfs_can_free_eofblocks(ip, true)) {
917+
if (xfs_can_free_eofblocks(ip)) {
904918
error = xfs_free_eofblocks(ip);
905919
if (error)
906920
return error;

fs/xfs/xfs_bmap_util.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ int xfs_insert_file_space(struct xfs_inode *, xfs_off_t offset,
6363
xfs_off_t len);
6464

6565
/* EOF block manipulation functions */
66-
bool xfs_can_free_eofblocks(struct xfs_inode *ip, bool force);
66+
bool xfs_can_free_eofblocks(struct xfs_inode *ip);
6767
int xfs_free_eofblocks(struct xfs_inode *ip);
6868

6969
int xfs_swap_extents(struct xfs_inode *ip, struct xfs_inode *tip,

fs/xfs/xfs_icache.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1147,7 +1147,7 @@ xfs_inode_free_eofblocks(
11471147
}
11481148
*lockflags |= XFS_IOLOCK_EXCL;
11491149

1150-
if (xfs_can_free_eofblocks(ip, false))
1150+
if (xfs_can_free_eofblocks(ip))
11511151
return xfs_free_eofblocks(ip);
11521152

11531153
/* inode could be preallocated or append-only */

fs/xfs/xfs_inode.c

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1462,7 +1462,7 @@ xfs_release(
14621462
if (!xfs_ilock_nowait(ip, XFS_IOLOCK_EXCL))
14631463
return 0;
14641464

1465-
if (xfs_can_free_eofblocks(ip, false)) {
1465+
if (xfs_can_free_eofblocks(ip)) {
14661466
/*
14671467
* Check if the inode is being opened, written and closed
14681468
* frequently and we have delayed allocation blocks outstanding
@@ -1678,15 +1678,13 @@ xfs_inode_needs_inactive(
16781678

16791679
/*
16801680
* This file isn't being freed, so check if there are post-eof blocks
1681-
* to free. @force is true because we are evicting an inode from the
1682-
* cache. Post-eof blocks must be freed, lest we end up with broken
1683-
* free space accounting.
1681+
* to free.
16841682
*
16851683
* Note: don't bother with iolock here since lockdep complains about
16861684
* acquiring it in reclaim context. We have the only reference to the
16871685
* inode at this point anyways.
16881686
*/
1689-
return xfs_can_free_eofblocks(ip, true);
1687+
return xfs_can_free_eofblocks(ip);
16901688
}
16911689

16921690
/*
@@ -1734,15 +1732,11 @@ xfs_inactive(
17341732

17351733
if (VFS_I(ip)->i_nlink != 0) {
17361734
/*
1737-
* force is true because we are evicting an inode from the
1738-
* cache. Post-eof blocks must be freed, lest we end up with
1739-
* broken free space accounting.
1740-
*
17411735
* Note: don't bother with iolock here since lockdep complains
17421736
* about acquiring it in reclaim context. We have the only
17431737
* reference to the inode at this point anyways.
17441738
*/
1745-
if (xfs_can_free_eofblocks(ip, true))
1739+
if (xfs_can_free_eofblocks(ip))
17461740
error = xfs_free_eofblocks(ip);
17471741

17481742
goto out;

0 commit comments

Comments
 (0)