On Sun, Jun 26, 2022 at 08:57:25PM -0700, Darrick J. Wong wrote: > On Mon, Jun 27, 2022 at 11:37:31AM +1000, Dave Chinner wrote: > > On Sun, Jun 26, 2022 at 03:04:04PM -0700, Darrick J. Wong wrote: > > > From: Darrick J. Wong <djwong@xxxxxxxxxx> > > > > > > On a system with a realtime volume and a 28k realtime extent, > > > generic/491 fails because the test opens a file on a frozen filesystem > > > and closing it causes xfs_release -> xfs_can_free_eofblocks to > > > mistakenly think that the the blocks of the realtime extent beyond EOF > > > are posteof blocks to be freed. Realtime extents cannot be partially > > > unmapped, so this is pointless. Worse yet, this triggers posteof > > > cleanup, which stalls on a transaction allocation, which is why the test > > > fails. > > > > > > Teach the predicate to account for realtime extents properly. > > > > > > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx> > > > --- > > > fs/xfs/xfs_bmap_util.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > > > > diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c > > > index 52be58372c63..85e1a26c92e8 100644 > > > --- a/fs/xfs/xfs_bmap_util.c > > > +++ b/fs/xfs/xfs_bmap_util.c > > > @@ -686,6 +686,8 @@ xfs_can_free_eofblocks( > > > * forever. > > > */ > > > end_fsb = XFS_B_TO_FSB(mp, (xfs_ufsize_t)XFS_ISIZE(ip)); > > > + if (XFS_IS_REALTIME_INODE(ip) && mp->m_sb.sb_rextsize > 1) > > > + end_fsb = roundup_64(end_fsb, mp->m_sb.sb_rextsize); > > > last_fsb = XFS_B_TO_FSB(mp, mp->m_super->s_maxbytes); > > > if (last_fsb <= end_fsb) > > > return false; > > > > Ok, that works. Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx> > > > > However, I was looking at xfs_can_free_eofblocks() w.r.t. freeze a > > couple of days ago and wondering why there isn't a freeze/RO state > > check in xfs_can_free_eofblocks(). Shouldn't we have one here so > > that we never try to run xfs_free_eofblocks() on RO/frozen > > filesystems regardless of unexpected state/alignment issues? > > I asked myself that question too. I found three callers of this > predicate: > > 1. fallocate, which should have obtained freeze protection *nod* > 2. inodegc, which should never be running when we get to the innermost > freeze protection level So inodegc could still do IO here on a read-only fs? > 3. xfs_release, which doesn't take freeze protection at all. Either it > needs to take freeze protection so that xfs_free_eofblocks can't get > stuck in xfs_trans_alloc, or we'd need to modify xfs_trans_alloc to > sb_start_intwrite_trylock That looks to me like it is simply a case of replacing the !xfs_is_readonly() check in xfs_release() with a !xfs_fs_writeable(mp, SB_FREEZE_WRITE) check and we shouldn't have to touch anythign else, right? Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
