xfs: don't assert when XFS_SCRUB_TYPE_HEALTHY scans return corruption
Subsystems (2)
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 2/26 xfs: don't assert when XFS_SCRUB_TYPE_HEALTHY scans return corruption
From: "Darrick J. Wong" <[email protected]>
XFS_SCRUB_TYPE_HEALTHY is a synthentic scrub type so that xfs_scrub can tell the kernel "Hey, I finished a scan and saw no problems" and have the kernel forget that it saw indirect evidence of corruption.
Unfortunately, as LOLLM points out, it's possible for the health system to record a new corruption just before xfs_scrub gets to XFS_SCRUB_TYPE_HEALTHY. In this case, the existing logic doesn't return early and instead wanders into unknown regions of type_to_health_flag and trips the assert because HEALTHY doesn't have a group assignment.
Fix the logic so that we always return early for a HEALTHY scrub type, even if we decide not to call xchk_mark_all_healthy.
--- fs/xfs/scrub/health.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
fs/xfs/scrub/health.c +3 -3
@@ -202,9 +202,9 @@ xchk_update_health(* there's no sick flag defined for it, so we branch here ahead of the * mask check. */ -if (sc->sm->sm_type ==XFS_SCRUB_TYPE_HEALTHY && - !(sc->sm->sm_flags &XFS_SCRUB_OFLAG_CORRUPT )) { -xchk_mark_all_healthy (sc->mp); +if (sc->sm->sm_type ==XFS_SCRUB_TYPE_HEALTHY ) { +if (!(sc->sm->sm_flags &XFS_SCRUB_OFLAG_CORRUPT )) +xchk_mark_all_healthy (sc->mp);return ; }
Signature · 1 line
2.55.0
PATCH 3/26 xfs: fix attr fork block count checks in xrep_inode_blockcounts
In reply to a message not in the archiveFrom: "Darrick J. Wong" <[email protected]>
LOLLM points out that a file has an attr fork, it will call xchk_inode_count_blocks to set @ablocks to the number of fsblocks mapped by the attr fork; but then it'll compare @blocks (aka the count of fsblocks mapped by the data fork). We already checked that and we never do anything with @acount, so I think this is clearly a bug. Fix the comparison.
--- fs/xfs/scrub/inode_repair.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/xfs/scrub/inode_repair.c +1 -1
@@ -1702,7 +1702,7 @@ xrep_inode_blockcounts( &acount);if (error)return error; -if (count >= sc->mp->m_sb.sb_dblocks) +if (acount >= sc->mp->m_sb.sb_dblocks)return -EFSCORRUPTED ; error =xrep_ino_ensure_extent_count (sc,XFS_ATTR_FORK , nextents);
Signature · 1 line
2.55.0
PATCH 4/26 xfs: release orphanage dir inode if chown fails
In reply to a message not in the archiveFrom: "Darrick J. Wong" <[email protected]>
LOLLM points out that we leak the igrab'd reference to the orphanage directory inode if chowning it fails. Fix that.
--- fs/xfs/scrub/orphanage.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
fs/xfs/scrub/orphanage.c +5 -1
@@ -192,12 +192,16 @@ xrep_orphanage_create(/* Make sure the orphanage is owned by root. */ error =xrep_chown_orphanage (sc,XFS_I (orphanage_inode));if (error) - goto out_dput_orphanage; + goto out_rele_orphanage;/* Stash the reference for later and bail out. */ sc->orphanage =XFS_I (orphanage_inode); sc->orphanage_ilock_flags =0 ; + orphanage_inode =NULL ; +out_rele_orphanage: +if (orphanage_inode) +xchk_irele (sc,XFS_I (orphanage_inode)); out_dput_orphanage:end_creating (orphanage_dentry); out_dput_root:
Signature · 1 line
2.55.0
PATCH 6/26 xfs: use correct jiffies comparison function in xchk_maybe_relax
In reply to a message not in the archiveFrom: "Darrick J. Wong" <[email protected]>
LOLLM points out that we're supposed to use time_after_eq, not a raw >= operation here, or else jiffies wraps can go unnoticed. Fix this.
--- fs/xfs/scrub/scrub.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/xfs/scrub/scrub.h +1 -1
@@ -40,7 +40,7 @@ static inline int xchk_maybe_relax(struct xchk_relax *widget)return 0 ; widget->resched_nr =0 ; -if (unlikely (widget->next_resched <= jiffies)) { +if (unlikely (time_after_eq (jiffies, widget->next_resched))) {cond_resched (); widget->next_resched =XCHK_RELAX_NEXT ; }
Signature · 1 line
2.55.0
PATCH 7/26 xfs: check padding field in xfs_ioc_commit_range
In reply to a message not in the archiveFrom: "Darrick J. Wong" <[email protected]>
LOLLM points out that we don't check the ioctl padding field here, so let's do that. I don't think there are many users yet since exchrange requires a new feature flag, so it's a good time to try to plug this hole.
--- fs/xfs/xfs_exchrange.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/xfs/xfs_exchrange.c +1 -1
@@ -902,7 +902,7 @@ xfs_ioc_commit_range(if (copy_from_user (&args, argp,sizeof (args)))return -EFAULT ; -if (args.flags & ~XFS_EXCHANGE_RANGE_ALL_FLAGS ) +if (args.pad || (args.flags & ~XFS_EXCHANGE_RANGE_ALL_FLAGS ))return -EINVAL ;if (kern_f->magic !=XCR_FRESH_MAGIC )return -EBUSY ;
Signature · 1 line
2.55.0
PATCH 8/26 xfs: don't call xfs_exchange_range_finish for a dry run
In reply to a message not in the archiveFrom: "Darrick J. Wong" <[email protected]>
LOLLM noticed that we strip file privileges and whatnot even for a dry run. We also shouldn't flush dirty data to disk or trim COW staging events for a dry run. Neither of those behaviors are allowed by the manpage, so fix that by exiting early on DRY_RUN in various functions.
--- fs/xfs/xfs_exchrange.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
fs/xfs/xfs_exchrange.c +8 -3
@@ -633,6 +633,9 @@ xfs_exchrange_prep(if (error)return error; +if (fxr->flags &XFS_EXCHANGE_RANGE_DRY_RUN ) +return 0 ; +trace_xfs_exchrange_flush (fxr, ip1, ip2);/* Flush the relevant ranges of both files. */
@@ -709,9 +712,11 @@ xfs_exchrange_contents(* other file write would do. This may involve turning on support for * logged xattrs if either file has security capabilities. */ - error =xfs_exchange_range_finish (fxr); -if (error) - goto out_unlock; +if (!(fxr->flags &XFS_EXCHANGE_RANGE_DRY_RUN )) { + error =xfs_exchange_range_finish (fxr); +if (error) + goto out_unlock; + } out_unlock:xfs_iunlock2_io_mmap (ip1, ip2);
Signature · 1 line
2.55.0
PATCH 9/26 xfs: use the correct reservations for rtrmap/refcount recovery
In reply to a message not in the archiveFrom: "Darrick J. Wong" <[email protected]>
LOLLM noticed that we might reserve the wrong number of blocks for recovering rtrmap and rtrefcount updates after a crash. Fix that.
--- fs/xfs/xfs_refcount_item.c | 8 ++++++-- fs/xfs/xfs_rmap_item.c | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-)
fs/xfs/xfs_refcount_item.c +6 -2
@@ -508,6 +508,7 @@ xfs_refcount_recover_work(struct xfs_cui_log_item *cuip =CUI_ITEM (lip);struct xfs_trans *tp;struct xfs_mount *mp = lip->li_log->l_mp; +unsigned int dblocks;bool isrt =xfs_cui_item_isrt (lip);int i;int error =0 ;
@@ -543,8 +544,11 @@ xfs_refcount_recover_work(* full btree split on either end of the refcount range. */ resv =xlog_recover_resv (&M_RES (mp)->tr_itruncate); - error =xfs_trans_alloc (mp, &resv, mp->m_refc_maxlevels *2 ,0 , -XFS_TRANS_RESERVE , &tp); +if (isrt) + dblocks = mp->m_rtrefc_maxlevels *2 ; +else + dblocks = mp->m_refc_maxlevels *2 ; + error =xfs_trans_alloc (mp, &resv, dblocks,0 ,XFS_TRANS_RESERVE , &tp);if (error)return error;
fs/xfs/xfs_rmap_item.c +6 -2
@@ -573,6 +573,7 @@ xfs_rmap_recover_work(struct xfs_rui_log_item *ruip =RUI_ITEM (lip);struct xfs_trans *tp;struct xfs_mount *mp = lip->li_log->l_mp; +unsigned int dblocks;bool isrt =xfs_rui_item_isrt (lip);int i;int error =0 ;
@@ -596,8 +597,11 @@ xfs_rmap_recover_work( } resv =xlog_recover_resv (&M_RES (mp)->tr_itruncate); - error =xfs_trans_alloc (mp, &resv, mp->m_rmap_maxlevels,0 , -XFS_TRANS_RESERVE , &tp); +if (isrt) + dblocks = mp->m_rtrmap_maxlevels; +else + dblocks = mp->m_rmap_maxlevels; + error =xfs_trans_alloc (mp, &resv, dblocks,0 ,XFS_TRANS_RESERVE , &tp);if (error)return error;
Signature · 1 line
2.55.0
PATCH 11/26 xfs: only flag zero padding for dir3 data blocks, not dir3 block blocks
In reply to a message not in the archiveFrom: "Darrick J. Wong" <[email protected]>
LOLLM complains that xchk_directory_data_bestfree can be passed a directory block that is either in "block" or "data" format, but the check here unconditionally treats the dir3_block and dir3_data blocks as if they have the same header format (they don't). Consequently, we can incorrectly set the preen state on dir3_block blocks, which of course we can't preen away because dir3_block blocks do not have a padding field. Fix this.
--- fs/xfs/scrub/dir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/xfs/scrub/dir.c +1 -1
@@ -492,7 +492,7 @@ xchk_directory_data_bestfree( goto out;xchk_buffer_recheck (sc, bp); -if (xfs_has_crc (sc->mp)) { +if (!is_block &&xfs_has_crc (sc->mp)) {struct xfs_dir3_data_hdr *hdr3 = bp->b_addr;if (hdr3->pad)
Signature · 1 line
2.55.0
PATCH 12/26 xfs: check di_forkoff correctly in scrub
In reply to a message not in the archiveFrom: "Darrick J. Wong" <[email protected]>
The di_forkoff check in xchk_dinode is incorrect, according to LOLLM. XFS_DFORK_BOFF returns a byte count relative to the start of the literal area, not the start of the inode. Therefore, this check won't flag di_forkoff values that are larger than the literal area but not the inode size itself. Fix this check; sadly the old APTR code was correct.
--- fs/xfs/scrub/inode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/xfs/scrub/inode.c +1 -1
@@ -607,7 +607,7 @@ xchk_dinode( }/* di_forkoff */ -if (XFS_DFORK_BOFF (dip) >= mp->m_sb.sb_inodesize) +if (dip->di_forkoff >= (XFS_LITINO (mp) >>3 ))xchk_ino_set_corrupt (sc, ino);if (naextents !=0 && dip->di_forkoff ==0 )xchk_ino_set_corrupt (sc, ino);
Signature · 1 line
2.55.0
PATCH 17/26 xfs: call xfs_dquot_set_prealloc_limits if we installed default rtb limits
In reply to a message not in the archiveFrom: "Darrick J. Wong" <[email protected]>
Now that we have quotas for the realtime volume, we also have precomputed watermark limits for the realtime block counts. These precomputations should be done any time we change the rtb limits, which means that xfs_qm_adjust_dqlimits needs to ensure that if we installed a default rtb limit.
--- fs/xfs/xfs_dquot.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
fs/xfs/xfs_dquot.c +6 -2
@@ -139,10 +139,14 @@ xfs_qm_adjust_dqlimits( dq->q_ino.softlimit = defq->ino.soft;if (!dq->q_ino.hardlimit) dq->q_ino.hardlimit = defq->ino.hard; -if (!dq->q_rtb.softlimit) +if (!dq->q_rtb.softlimit) { dq->q_rtb.softlimit = defq->rtb.soft; -if (!dq->q_rtb.hardlimit) + prealloc =1 ; + } +if (!dq->q_rtb.hardlimit) { dq->q_rtb.hardlimit = defq->rtb.hard; + prealloc =1 ; + }if (prealloc)xfs_dquot_set_prealloc_limits (dq);
Signature · 1 line
2.55.0
PATCH 18/26 xfs: fix rtgroup repair estimations
In reply to a message not in the archiveFrom: "Darrick J. Wong" <[email protected]>
When I added online fsck for realtime reflink, I forgot to update xrep_calc_rtgroup_resblks to factor in the size of the refcount btree when it guesses how much space we need to start a repair. This hasn't been a huge problem in practice because there are few filesystems with
(a) realtime, (b) rtgroups, (c) reflink, and (d) no rmap. But let's fix this before someone stumbles upon it, especially since LOLLM flagged this for me.
--- fs/xfs/scrub/repair.c | 19 +++++++++++++++++-- fs/xfs/scrub/trace.h | 12 ++++++++---- 2 files changed, 25 insertions(+), 6 deletions(-)
fs/xfs/scrub/repair.c +17 -2
@@ -399,6 +399,7 @@ xrep_calc_rtgroup_resblks(struct xfs_mount *mp = sc->mp;struct xfs_scrub_metadata *sm = sc->sm;uint64_t usedlen; +xfs_extlen_t refcbt_sz =0 ;xfs_extlen_t rmapbt_sz =0 ;if (!(sm->sm_flags &XFS_SCRUB_IFLAG_REPAIR ))
@@ -411,13 +412,27 @@ xrep_calc_rtgroup_resblks( usedlen =xfs_rtbxlen_to_blen (mp,xfs_rtgroup_extents (mp, sm->sm_agno));ASSERT (usedlen <=XFS_MAX_RGBLOCKS ); +if (xfs_has_reflink (mp)) + refcbt_sz =xfs_rtrefcountbt_calc_size (mp, usedlen); +if (xfs_has_rmapbt (mp)) rmapbt_sz =xfs_rtrmapbt_calc_size (mp, usedlen); +/* +* Guess how many blocks we need to rebuild the rmapbt. For +* non-reflink filesystems we can't have more records than used blocks. +* However, with reflink it's possible to have more than one rmap +* record per rtgroup block. We don't know how many rmaps there could +* be in the rtgroup, so we start off with what we hope is an generous +* over-estimation. +*/ +if (refcbt_sz >0 && rmapbt_sz >0 ) + rmapbt_sz *=2 ; +trace_xrep_calc_rtgroup_resblks_btsize (mp, sm->sm_agno, usedlen, - rmapbt_sz); + rmapbt_sz, refcbt_sz); - return rmapbt_sz; + return max(rmapbt_sz, refcbt_sz); }#endif /* CONFIG_XFS_RT */
fs/xfs/scrub/trace.h +8 -4
@@ -2376,25 +2376,29 @@ TRACE_EVENT(xrep_calc_ag_resblks_btsize,#ifdef CONFIG_XFS_RT TRACE_EVENT (xrep_calc_rtgroup_resblks_btsize, TP_PROTO(struct xfs_mount *mp, xfs_rgnumber_t rgno, - xfs_rgblock_t usedlen, xfs_rgblock_t rmapbt_sz), -TP_ARGS (mp, rgno, usedlen, rmapbt_sz), + xfs_rgblock_t usedlen, xfs_rgblock_t rmapbt_sz, + xfs_rgblock_t refcbt_sz), +TP_ARGS (mp, rgno, usedlen, rmapbt_sz, refcbt_sz),TP_STRUCT__entry ( __field(dev_t, dev) __field(xfs_rgnumber_t, rgno) __field(xfs_rgblock_t, usedlen) __field(xfs_rgblock_t, rmapbt_sz) +__field (xfs_rgblock_t, refcbt_sz) ),TP_fast_assign ( __entry->dev = mp->m_super->s_dev; __entry->rgno = rgno; __entry->usedlen = usedlen; __entry->rmapbt_sz = rmapbt_sz; + __entry->refcbt_sz = refcbt_sz; ), -TP_printk ("dev %d:%d rgno 0x%x usedlen %u rmapbt %u" , +TP_printk ("dev %d:%d rgno 0x%x usedlen %u rmapbt %u refcountbt %u" ,MAJOR (__entry->dev),MINOR (__entry->dev), __entry->rgno, __entry->usedlen, - __entry->rmapbt_sz) + __entry->rmapbt_sz, + __entry->refcbt_sz) );#endif /* CONFIG_XFS_RT */
Signature · 1 line
2.55.0
PATCH 19/26 xfs: don't cross reference rmapbt with bitmaps if they're incomplete
In reply to a message not in the archiveFrom: "Darrick J. Wong" <[email protected]>
LOLLM points out that runtime errors (e.g. ENOMEM) when we're trying to compute space usag bitmaps are silently dropped by the rmapbt scrubber. We ought to flag that as an incomplete scrub instead of reporting cross-referencing errors based on faulty data.
--- fs/xfs/scrub/rmap.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)
fs/xfs/scrub/rmap.c +14 -6
@@ -493,11 +493,18 @@ xchk_rmapbt_walk_ag_metadata(* If there's an error, set XFAIL and disable the bitmap * cross-referencing checks, but proceed with the scrub anyway. */ -if (error) -xchk_btree_xref_process_error (sc, sc->sa.rmap_cur, - sc->sa.rmap_cur->bc_nlevels -1 , &error); -else - cr->bitmaps_complete = true; +if (error) { +if (!xchk_btree_xref_process_error (sc, sc->sa.rmap_cur, + sc->sa.rmap_cur->bc_nlevels -1 , &error)) { +/* only set incomplete if we didn't set xfail */ +if (error) +xchk_set_incomplete (sc); + } + +return 0 ; + } + + cr->bitmaps_complete = true;return 0 ; }
@@ -567,7 +574,8 @@ xchk_rmapbt(if (error) goto out; -xchk_rmapbt_check_bitmaps (sc, cr); +if (cr->bitmaps_complete) +xchk_rmapbt_check_bitmaps (sc, cr); out:xagb_bitmap_destroy (&cr->refcbt_owned);
Signature · 1 line
2.55.0
PATCH 20/26 xfs: don't let memory failures leak blocks and kill repairs
In reply to a message not in the archiveFrom: "Darrick J. Wong" <[email protected]>
LOLLM complains that a memory allocation failure in xrep_newbt_add_blocks results in online repair leaking blocks that were previously allocated to write a new btree, but the problem is worse than that -- a limitation of the codebase is that the callers cannot undo the transaction /and/ return the error -- either you undo all changes and commit the transaction, or you error out and the filesystem goes down.
However, the new btree space reservation object isn't that big (~48 bytes). Let's just do a NOFAIL allocation and the problem goes away.
--- fs/xfs/scrub/newbt.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
fs/xfs/scrub/newbt.c +5 -3
@@ -193,9 +193,11 @@ xrep_newbt_add_blocks(struct xrep_newbt_resv *resv;int error; - resv = kmalloc_obj(struct xrep_newbt_resv, XCHK_GFP_FLAGS); -if (!resv) -return -ENOMEM ; +/* +* We have no way to clean up the allocated space *and* return an +* ENOMEM if we fail to allocate this control structure. +*/ + resv = kmalloc_obj(struct xrep_newbt_resv, GFP_KERNEL | __GFP_NOFAIL);INIT_LIST_HEAD (&resv->list); resv->agbno =XFS_FSB_TO_AGBNO (mp, args->fsbno);
Signature · 1 line
2.55.0
PATCH 21/26 xfs: don't merge different file IO error types
In reply to a message not in the archiveFrom: "Darrick J. Wong" <[email protected]>
LOLLM noticed that we can accidentally merge file range health monitoring events even if they have different errors. We shouldn't do that.
--- fs/xfs/xfs_healthmon.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
fs/xfs/xfs_healthmon.c +3 -1
@@ -247,7 +247,9 @@ xfs_healthmon_merge_events(case XFS_HEALTHMON_DIOWRITE :case XFS_HEALTHMON_DATALOST :/* logically adjacent file ranges can merge */ -if (existing->fino != new->fino || existing->fgen != new->fgen) +if (existing->fino != new->fino || + existing->fgen != new->fgen || + existing->error != new->error)return false;if (existing->fpos + existing->flen == new->fpos) {
Signature · 1 line
2.55.0
PATCH 22/26 xfs: fix blockgc group quota scanning when usrquota isn't enforced
In reply to a message not in the archiveFrom: "Darrick J. Wong" <[email protected]>
LOLLM noticed the copy-paste error here -- if user quotas aren't enforced but we're near the group quota limit, we fail to set FLAG_GID and hence we might not actually free any preallocations, causing unnecessary EDQUOT. Fix that.
--- fs/xfs/xfs_icache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/xfs/xfs_icache.c +1 -1
@@ -1653,7 +1653,7 @@ xfs_blockgc_free_dquots( do_work = true; } -if (XFS_IS_UQUOTA_ENFORCED (mp) && gdqp &&xfs_dquot_lowsp (gdqp)) { +if (XFS_IS_GQUOTA_ENFORCED (mp) && gdqp &&xfs_dquot_lowsp (gdqp)) { icw.icw_gid =make_kgid (mp->m_super->s_user_ns, gdqp->q_id); icw.icw_flags |=XFS_ICWALK_FLAG_GID ; do_work = true;
Signature · 1 line
2.55.0
PATCH 23/26 xfs: fix cursor and pointer handling when recovering iunlink buckets
In reply to a message not in the archiveFrom: "Darrick J. Wong" <[email protected]>
LOLLM pointed out a bug in xlog_recover_iunlink_bucket:
1. We don't null out prev_ip after releasing it, which can lead to UAF problems if the inodegc flush call in the loop fails.
at which point I noticed even more bugs:
2. If the inodegc flush inside the loop fails, we also leak @ip.
3. We set prev_agino to agino having already advanced agino, which results in inodes with i_prev_unlinked set to itself.
4. If we exit the bottom of the loop with prev_ip set, then prev_ip aliases ip and we also set its i_prev_unlinked to itself.
Bugs 3 and 4 introduce loops into the unlinked list, though these loops don't surface because we immediately flush each unlinked inode after loading it.
Fix all of these issues.
--- fs/xfs/xfs_log_recover.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
fs/xfs/xfs_log_recover.c +8 -5
@@ -2736,12 +2736,13 @@ xlog_recover_iunlink_bucket( {struct xfs_mount *mp =pag_mount (pag);struct xfs_inode *prev_ip =NULL ; -struct xfs_inode *ip;xfs_agino_t prev_agino, agino;int error =0 ; agino =be32_to_cpu (agi->agi_unlinked[bucket]);while (agino !=NULLAGINO ) { +struct xfs_inode *ip; + error =xfs_iget (mp,NULL ,xfs_agino_to_ino (pag, agino),0 ,0 , &ip);if (error)
@@ -2750,11 +2751,11 @@ xlog_recover_iunlink_bucket(ASSERT (VFS_I (ip)->i_nlink ==0 );ASSERT (VFS_I (ip)->i_mode !=0 );xfs_iflags_clear (ip,XFS_IRECOVERY ); - agino = ip->i_next_unlinked;if (prev_ip) { ip->i_prev_unlinked = prev_agino;xfs_irele (prev_ip); + prev_ip =NULL ;/* * Ensure the inode is removed from the unlinked list
@@ -2766,18 +2767,20 @@ xlog_recover_iunlink_bucket(* complete. */ error =xfs_inodegc_flush (mp); -if (error) -break ; +if (error) { +xfs_irele (ip); +return error; + } } prev_agino = agino; + agino = ip->i_next_unlinked; prev_ip = ip; }if (prev_ip) {int error2; - ip->i_prev_unlinked = prev_agino;xfs_irele (prev_ip); error2 =xfs_inodegc_flush (mp);
Signature · 1 line
2.55.0
PATCH 24/26 xfs: drop dquot flush lock when we can't find a buffer to flush
In reply to a message not in the archiveFrom: "Darrick J. Wong" <[email protected]>
LOLLM noticed that xfs_qm_flush_one fails to drop the dquot flush lock if it can't grab the buffer associated with the dquot. Since there's no buffer, nobody else is going to drop the dqflock, so we need to do it ourselves.
--- fs/xfs/xfs_qm.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
fs/xfs/xfs_qm.c +8 -2
@@ -1432,16 +1432,22 @@ xfs_qm_flush_one( error =xfs_dquot_use_attached_buf (dqp, &bp);if (error) - goto out_unlock; + goto out_dqflock;if (!bp) { error = -EFSCORRUPTED ; - goto out_unlock; + goto out_dqflock; } error =xfs_qm_dqflush (dqp, bp);if (!error)xfs_buf_delwri_queue (bp, buffer_list);xfs_buf_relse (bp); +mutex_unlock (&dqp->q_qlock); +xfs_qm_dqrele (dqp); +return error; + +out_dqflock: +xfs_dqfunlock (dqp); out_unlock:mutex_unlock (&dqp->q_qlock);xfs_qm_dqrele (dqp);
Signature · 1 line
2.55.0
PATCH 25/26 xfs: don't let hidden_space go negative in xfs_metafile_resv_init
In reply to a message not in the archiveFrom: "Darrick J. Wong" <[email protected]>
LOLLM points out that if the amount of fdblocks that we can reserve for a metadata btree file goes below the space already used by that file, then the hidden_space subtraction can underflow, causing xfs_dec_fdblocks to subtract a huge amount of space. We never want the target to be less than the used sapce, so fix the logic that adjusts dblocks_avail downwards.
Also fix an error in the adjacent comment.
--- fs/xfs/libxfs/xfs_metafile.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
fs/xfs/libxfs/xfs_metafile.c +4 -4
@@ -297,14 +297,14 @@ xfs_metafile_resv_init( goto out_unlock;/* -* Space taken by the per-AG metadata btrees are accounted on-disk as -* used space. We therefore only hide the space that is reserved but -* not used by the trees. +* Space taken by metadata btrees are accounted on-disk as used space. +* We therefore only hide the space that is reserved but not used by +* the trees. */ if (used > target) target = used;else if (target > dblocks_avail) - target = dblocks_avail; + target =max (dblocks_avail, used); hidden_space = target - used; error =xfs_dec_fdblocks (mp, hidden_space, true);
Signature · 1 line
2.55.0
PATCH 26/26 xfs: fix wild memcpy access when formatting ondisk rtrefcount btree roots
In reply to a message not in the archiveFrom: "Darrick J. Wong" <[email protected]>
LOLLM noticed that the inode btree root formatting methods copy too many bytes -- there's only one set of keys in node blocks, not two. This causes memory corruption of whatever's beyond the buffers.
--- fs/xfs/libxfs/xfs_rtrefcount_btree.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
fs/xfs/libxfs/xfs_rtrefcount_btree.c +2 -2
@@ -617,7 +617,7 @@ xfs_rtrefcountbt_from_disk( fpp =xfs_rtrefcount_droot_ptr_addr (dblock,1 , maxrecs); tpp =xfs_rtrefcount_broot_ptr_addr (mp, rblock,1 , rblocklen); numrecs =be16_to_cpu (dblock->bb_numrecs); -memcpy (tkp, fkp,2 *sizeof (*fkp) * numrecs); +memcpy (tkp, fkp,sizeof (*fkp) * numrecs);memcpy (tpp, fpp,sizeof (*fpp) * numrecs); }else { frp =xfs_rtrefcount_droot_rec_addr (dblock,1 );
@@ -703,7 +703,7 @@ xfs_rtrefcountbt_to_disk( fpp =xfs_rtrefcount_broot_ptr_addr (mp, rblock,1 , rblocklen); tpp =xfs_rtrefcount_droot_ptr_addr (dblock,1 , maxrecs); numrecs =be16_to_cpu (rblock->bb_numrecs); -memcpy (tkp, fkp,2 *sizeof (*fkp) * numrecs); +memcpy (tkp, fkp,sizeof (*fkp) * numrecs);memcpy (tpp, fpp,sizeof (*fpp) * numrecs); }else { frp =xfs_rtrefcount_rec_addr (rblock,1 );
Signature · 1 line
2.55.0