Hi Greg,
This 5.10.y backport series contains fixes from v5.18 and v5.19-rc1.
Patches 1-5 in this series have already been applied to 5.15.y in Leah's latest update [1], so this 5.10.y is is mostly catching up with 5.15.y.
Patches 6-7 in this 5.10.y update have not been applied to 5.15.y yet. I pointed Leah's attention to these patches and she said she will include them in a following 5.15.y update.
Thanks, Amir.
Changes since v1: - Added ACKs - CC stable
[1] https://lore.kernel.org/linux-xfs/20220819181431.4113819-1-leah.rumancik@gma...
Amir Goldstein (1): xfs: remove infinite loop when reserving free block pool
Brian Foster (1): xfs: fix soft lockup via spinning in filestream ag selection loop
Darrick J. Wong (2): xfs: always succeed at setting the reserve pool size xfs: fix overfilling of reserve pool
Dave Chinner (2): xfs: reorder iunlink remove operation in xfs_ifree xfs: validate inode fork size against fork format
Eric Sandeen (1): xfs: revert "xfs: actually bump warning counts when we send warnings"
fs/xfs/libxfs/xfs_inode_buf.c | 35 +++++++++++++++++------ fs/xfs/xfs_filestream.c | 7 +++-- fs/xfs/xfs_fsops.c | 52 ++++++++++++++--------------------- fs/xfs/xfs_inode.c | 22 ++++++++------- fs/xfs/xfs_mount.h | 8 ++++++ fs/xfs/xfs_trans_dquot.c | 1 - 6 files changed, 71 insertions(+), 54 deletions(-)
commit 15f04fdc75aaaa1cccb0b8b3af1be290e118a7bc upstream.
[Added wrapper xfs_fdblocks_unavailable() for 5.10.y backport]
Infinite loops in kernel code are scary. Calls to xfs_reserve_blocks should be rare (people should just use the defaults!) so we really don't need to try so hard. Simplify the logic here by removing the infinite loop.
Cc: Brian Foster bfoster@redhat.com Signed-off-by: Darrick J. Wong djwong@kernel.org Reviewed-by: Dave Chinner dchinner@redhat.com Signed-off-by: Amir Goldstein amir73il@gmail.com Acked-by: Darrick J. Wong djwong@kernel.org --- fs/xfs/xfs_fsops.c | 52 +++++++++++++++++++--------------------------- fs/xfs/xfs_mount.h | 8 +++++++ 2 files changed, 29 insertions(+), 31 deletions(-)
diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c index ef1d5bb88b93..6d4f4271e7be 100644 --- a/fs/xfs/xfs_fsops.c +++ b/fs/xfs/xfs_fsops.c @@ -376,46 +376,36 @@ xfs_reserve_blocks( * If the request is larger than the current reservation, reserve the * blocks before we update the reserve counters. Sample m_fdblocks and * perform a partial reservation if the request exceeds free space. + * + * The code below estimates how many blocks it can request from + * fdblocks to stash in the reserve pool. This is a classic TOCTOU + * race since fdblocks updates are not always coordinated via + * m_sb_lock. */ - error = -ENOSPC; - do { - free = percpu_counter_sum(&mp->m_fdblocks) - - mp->m_alloc_set_aside; - if (free <= 0) - break; - - delta = request - mp->m_resblks; - lcounter = free - delta; - if (lcounter < 0) - /* We can't satisfy the request, just get what we can */ - fdblks_delta = free; - else - fdblks_delta = delta; - + free = percpu_counter_sum(&mp->m_fdblocks) - + xfs_fdblocks_unavailable(mp); + delta = request - mp->m_resblks; + if (delta > 0 && free > 0) { /* * We'll either succeed in getting space from the free block - * count or we'll get an ENOSPC. If we get a ENOSPC, it means - * things changed while we were calculating fdblks_delta and so - * we should try again to see if there is anything left to - * reserve. - * - * Don't set the reserved flag here - we don't want to reserve - * the extra reserve blocks from the reserve..... + * count or we'll get an ENOSPC. Don't set the reserved flag + * here - we don't want to reserve the extra reserve blocks + * from the reserve. */ + fdblks_delta = min(free, delta); spin_unlock(&mp->m_sb_lock); error = xfs_mod_fdblocks(mp, -fdblks_delta, 0); spin_lock(&mp->m_sb_lock); - } while (error == -ENOSPC);
- /* - * Update the reserve counters if blocks have been successfully - * allocated. - */ - if (!error && fdblks_delta) { - mp->m_resblks += fdblks_delta; - mp->m_resblks_avail += fdblks_delta; + /* + * Update the reserve counters if blocks have been successfully + * allocated. + */ + if (!error) { + mp->m_resblks += fdblks_delta; + mp->m_resblks_avail += fdblks_delta; + } } - out: if (outval) { outval->resblks = mp->m_resblks; diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h index dfa429b77ee2..3a6bc9dc11b5 100644 --- a/fs/xfs/xfs_mount.h +++ b/fs/xfs/xfs_mount.h @@ -406,6 +406,14 @@ extern int xfs_initialize_perag(xfs_mount_t *mp, xfs_agnumber_t agcount, xfs_agnumber_t *maxagi); extern void xfs_unmountfs(xfs_mount_t *);
+/* Accessor added for 5.10.y backport */ +static inline uint64_t +xfs_fdblocks_unavailable( + struct xfs_mount *mp) +{ + return mp->m_alloc_set_aside; +} + extern int xfs_mod_fdblocks(struct xfs_mount *mp, int64_t delta, bool reserved); extern int xfs_mod_frextents(struct xfs_mount *mp, int64_t delta);
From: "Darrick J. Wong" djwong@kernel.org
commit 0baa2657dc4d79202148be79a3dc36c35f425060 upstream.
Nowadays, xfs_mod_fdblocks will always choose to fill the reserve pool with freed blocks before adding to fdblocks. Therefore, we can change the behavior of xfs_reserve_blocks slightly -- setting the target size of the pool should always succeed, since a deficiency will eventually be made up as blocks get freed.
Signed-off-by: Darrick J. Wong djwong@kernel.org Reviewed-by: Dave Chinner dchinner@redhat.com Signed-off-by: Amir Goldstein amir73il@gmail.com Acked-by: Darrick J. Wong djwong@kernel.org --- fs/xfs/xfs_fsops.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c index 6d4f4271e7be..dacead0d0934 100644 --- a/fs/xfs/xfs_fsops.c +++ b/fs/xfs/xfs_fsops.c @@ -380,11 +380,14 @@ xfs_reserve_blocks( * The code below estimates how many blocks it can request from * fdblocks to stash in the reserve pool. This is a classic TOCTOU * race since fdblocks updates are not always coordinated via - * m_sb_lock. + * m_sb_lock. Set the reserve size even if there's not enough free + * space to fill it because mod_fdblocks will refill an undersized + * reserve when it can. */ free = percpu_counter_sum(&mp->m_fdblocks) - xfs_fdblocks_unavailable(mp); delta = request - mp->m_resblks; + mp->m_resblks = request; if (delta > 0 && free > 0) { /* * We'll either succeed in getting space from the free block @@ -401,10 +404,8 @@ xfs_reserve_blocks( * Update the reserve counters if blocks have been successfully * allocated. */ - if (!error) { - mp->m_resblks += fdblks_delta; + if (!error) mp->m_resblks_avail += fdblks_delta; - } } out: if (outval) {
From: "Darrick J. Wong" djwong@kernel.org
commit 82be38bcf8a2e056b4c99ce79a3827fa743df6ec upstream.
Due to cycling of m_sb_lock, it's possible for multiple callers of xfs_reserve_blocks to race at changing the pool size, subtracting blocks from fdblocks, and actually putting it in the pool. The result of all this is that we can overfill the reserve pool to hilarious levels.
xfs_mod_fdblocks, when called with a positive value, already knows how to take freed blocks and either fill the reserve until it's full, or put them in fdblocks. Use that instead of setting m_resblks_avail directly.
Signed-off-by: Darrick J. Wong djwong@kernel.org Reviewed-by: Dave Chinner dchinner@redhat.com Signed-off-by: Amir Goldstein amir73il@gmail.com Acked-by: Darrick J. Wong djwong@kernel.org --- fs/xfs/xfs_fsops.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c index dacead0d0934..775f833146e3 100644 --- a/fs/xfs/xfs_fsops.c +++ b/fs/xfs/xfs_fsops.c @@ -394,18 +394,17 @@ xfs_reserve_blocks( * count or we'll get an ENOSPC. Don't set the reserved flag * here - we don't want to reserve the extra reserve blocks * from the reserve. + * + * The desired reserve size can change after we drop the lock. + * Use mod_fdblocks to put the space into the reserve or into + * fdblocks as appropriate. */ fdblks_delta = min(free, delta); spin_unlock(&mp->m_sb_lock); error = xfs_mod_fdblocks(mp, -fdblks_delta, 0); - spin_lock(&mp->m_sb_lock); - - /* - * Update the reserve counters if blocks have been successfully - * allocated. - */ if (!error) - mp->m_resblks_avail += fdblks_delta; + xfs_mod_fdblocks(mp, fdblks_delta, 0); + spin_lock(&mp->m_sb_lock); } out: if (outval) {
From: Brian Foster bfoster@redhat.com
commit f650df7171b882dca737ddbbeb414100b31f16af upstream.
The filestream AG selection loop uses pagf data to aid in AG selection, which depends on pagf initialization. If the in-core structure is not initialized, the caller invokes the AGF read path to do so and carries on. If another task enters the loop and finds a pagf init already in progress, the AGF read returns -EAGAIN and the task continues the loop. This does not increment the current ag index, however, which means the task spins on the current AGF buffer until unlocked.
If the AGF read I/O submitted by the initial task happens to be delayed for whatever reason, this results in soft lockup warnings via the spinning task. This is reproduced by xfs/170. To avoid this problem, fix the AGF trylock failure path to properly iterate to the next AG. If a task iterates all AGs without making progress, the trylock behavior is dropped in favor of blocking locks and thus a soft lockup is no longer possible.
Fixes: f48e2df8a877ca1c ("xfs: make xfs_*read_agf return EAGAIN to ALLOC_FLAG_TRYLOCK callers") Signed-off-by: Brian Foster bfoster@redhat.com Reviewed-by: Darrick J. Wong djwong@kernel.org Reviewed-by: Christoph Hellwig hch@lst.de Signed-off-by: Dave Chinner david@fromorbit.com Signed-off-by: Amir Goldstein amir73il@gmail.com Acked-by: Darrick J. Wong djwong@kernel.org --- fs/xfs/xfs_filestream.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/fs/xfs/xfs_filestream.c b/fs/xfs/xfs_filestream.c index db23e455eb91..bc41ec0c483d 100644 --- a/fs/xfs/xfs_filestream.c +++ b/fs/xfs/xfs_filestream.c @@ -128,11 +128,12 @@ xfs_filestream_pick_ag( if (!pag->pagf_init) { err = xfs_alloc_pagf_init(mp, NULL, ag, trylock); if (err) { - xfs_perag_put(pag); - if (err != -EAGAIN) + if (err != -EAGAIN) { + xfs_perag_put(pag); return err; + } /* Couldn't lock the AGF, skip this AG. */ - continue; + goto next_ag; } }
From: Eric Sandeen sandeen@redhat.com
commit bc37e4fb5cac2925b2e286b1f1d4fc2b519f7d92 upstream.
This reverts commit 4b8628d57b725b32616965e66975fcdebe008fe7.
XFS quota has had the concept of a "quota warning limit" since the earliest Irix implementation, but a mechanism for incrementing the warning counter was never implemented, as documented in the xfs_quota(8) man page. We do know from the historical archive that it was never incremented at runtime during quota reservation operations.
With this commit, the warning counter quickly increments for every allocation attempt after the user has crossed a quote soft limit threshold, and this in turn transitions the user to hard quota failures, rendering soft quota thresholds and timers useless. This was reported as a regression by users.
Because the intended behavior of this warning counter has never been understood or documented, and the result of this change is a regression in soft quota functionality, revert this commit to make soft quota limits and timers operable again.
Fixes: 4b8628d57b72 ("xfs: actually bump warning counts when we send warnings) Signed-off-by: Eric Sandeen sandeen@redhat.com Reviewed-by: Darrick J. Wong djwong@kernel.org Reviewed-by: Dave Chinner dchinner@redhat.com Signed-off-by: Dave Chinner david@fromorbit.com Signed-off-by: Amir Goldstein amir73il@gmail.com Acked-by: Darrick J. Wong djwong@kernel.org --- fs/xfs/xfs_trans_dquot.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c index fe45b0c3970c..288ea38c43ad 100644 --- a/fs/xfs/xfs_trans_dquot.c +++ b/fs/xfs/xfs_trans_dquot.c @@ -615,7 +615,6 @@ xfs_dqresv_check( return QUOTA_NL_ISOFTLONGWARN; }
- res->warnings++; return QUOTA_NL_ISOFTWARN; }
From: Dave Chinner dchinner@redhat.com
commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream.
[backport for 5.10.y]
The O_TMPFILE creation implementation creates a specific order of operations for inode allocation/freeing and unlinked list modification. Currently both are serialised by the AGI, so the order doesn't strictly matter as long as the are both in the same transaction.
However, if we want to move the unlinked list insertions largely out from under the AGI lock, then we have to be concerned about the order in which we do unlinked list modification operations. O_TMPFILE creation tells us this order is inode allocation/free, then unlinked list modification.
Change xfs_ifree() to use this same ordering on unlinked list removal. This way we always guarantee that when we enter the iunlinked list removal code from this path, we already have the AGI locked and we don't have to worry about lock nesting AGI reads inside unlink list locks because it's already locked and attached to the transaction.
We can do this safely as the inode freeing and unlinked list removal are done in the same transaction and hence are atomic operations with respect to log recovery.
Reported-by: Frank Hofmann fhofmann@cloudflare.com Fixes: 298f7bec503f ("xfs: pin inode backing buffer to the inode log item") Signed-off-by: Dave Chinner dchinner@redhat.com Reviewed-by: Darrick J. Wong darrick.wong@oracle.com Signed-off-by: Dave Chinner david@fromorbit.com Signed-off-by: Amir Goldstein amir73il@gmail.com Acked-by: Darrick J. Wong djwong@kernel.org --- fs/xfs/xfs_inode.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index 1f61e085676b..929ed3bc5619 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -2669,14 +2669,13 @@ xfs_ifree_cluster( }
/* - * This is called to return an inode to the inode free list. - * The inode should already be truncated to 0 length and have - * no pages associated with it. This routine also assumes that - * the inode is already a part of the transaction. + * This is called to return an inode to the inode free list. The inode should + * already be truncated to 0 length and have no pages associated with it. This + * routine also assumes that the inode is already a part of the transaction. * - * The on-disk copy of the inode will have been added to the list - * of unlinked inodes in the AGI. We need to remove the inode from - * that list atomically with respect to freeing it here. + * The on-disk copy of the inode will have been added to the list of unlinked + * inodes in the AGI. We need to remove the inode from that list atomically with + * respect to freeing it here. */ int xfs_ifree( @@ -2694,13 +2693,16 @@ xfs_ifree( ASSERT(ip->i_d.di_nblocks == 0);
/* - * Pull the on-disk inode from the AGI unlinked list. + * Free the inode first so that we guarantee that the AGI lock is going + * to be taken before we remove the inode from the unlinked list. This + * makes the AGI lock -> unlinked list modification order the same as + * used in O_TMPFILE creation. */ - error = xfs_iunlink_remove(tp, ip); + error = xfs_difree(tp, ip->i_ino, &xic); if (error) return error;
- error = xfs_difree(tp, ip->i_ino, &xic); + error = xfs_iunlink_remove(tp, ip); if (error) return error;
On Thu, Sep 1, 2022 at 6:49 AM Amir Goldstein amir73il@gmail.com wrote:
From: Dave Chinner dchinner@redhat.com
commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream.
[backport for 5.10.y]
Hi Amir, hi Dave,
I've got no objections to backporting this change at all. We've been using the patch on our internal 5.15 tracker branch happily for several months now.
Would like to highlight though that it's currently not yet merged in linux-stable 5.15 branch either (it's in 5.19 and mainline alright). If this gets queued for 5.10 then maybe it also should be for 5.15 ?
Thank you, FrankH.
The O_TMPFILE creation implementation creates a specific order of operations for inode allocation/freeing and unlinked list modification. Currently both are serialised by the AGI, so the order doesn't strictly matter as long as the are both in the same transaction.
However, if we want to move the unlinked list insertions largely out from under the AGI lock, then we have to be concerned about the order in which we do unlinked list modification operations. O_TMPFILE creation tells us this order is inode allocation/free, then unlinked list modification.
Change xfs_ifree() to use this same ordering on unlinked list removal. This way we always guarantee that when we enter the iunlinked list removal code from this path, we already have the AGI locked and we don't have to worry about lock nesting AGI reads inside unlink list locks because it's already locked and attached to the transaction.
We can do this safely as the inode freeing and unlinked list removal are done in the same transaction and hence are atomic operations with respect to log recovery.
Reported-by: Frank Hofmann fhofmann@cloudflare.com Fixes: 298f7bec503f ("xfs: pin inode backing buffer to the inode log item") Signed-off-by: Dave Chinner dchinner@redhat.com Reviewed-by: Darrick J. Wong darrick.wong@oracle.com Signed-off-by: Dave Chinner david@fromorbit.com Signed-off-by: Amir Goldstein amir73il@gmail.com Acked-by: Darrick J. Wong djwong@kernel.org
fs/xfs/xfs_inode.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index 1f61e085676b..929ed3bc5619 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -2669,14 +2669,13 @@ xfs_ifree_cluster( }
/*
- This is called to return an inode to the inode free list.
- The inode should already be truncated to 0 length and have
- no pages associated with it. This routine also assumes that
- the inode is already a part of the transaction.
- This is called to return an inode to the inode free list. The inode should
- already be truncated to 0 length and have no pages associated with it. This
- routine also assumes that the inode is already a part of the transaction.
- The on-disk copy of the inode will have been added to the list
- of unlinked inodes in the AGI. We need to remove the inode from
- that list atomically with respect to freeing it here.
- The on-disk copy of the inode will have been added to the list of unlinked
- inodes in the AGI. We need to remove the inode from that list atomically with
*/
- respect to freeing it here.
int xfs_ifree( @@ -2694,13 +2693,16 @@ xfs_ifree( ASSERT(ip->i_d.di_nblocks == 0);
/*
* Pull the on-disk inode from the AGI unlinked list.
* Free the inode first so that we guarantee that the AGI lock is going* to be taken before we remove the inode from the unlinked list. This* makes the AGI lock -> unlinked list modification order the same as* used in O_TMPFILE creation. */
error = xfs_iunlink_remove(tp, ip);
error = xfs_difree(tp, ip->i_ino, &xic); if (error) return error;
error = xfs_difree(tp, ip->i_ino, &xic);
error = xfs_iunlink_remove(tp, ip); if (error) return error;-- 2.25.1
On Thu, Sep 1, 2022 at 12:04 PM Frank Hofmann fhofmann@cloudflare.com wrote:
On Thu, Sep 1, 2022 at 6:49 AM Amir Goldstein amir73il@gmail.com wrote:
From: Dave Chinner dchinner@redhat.com
commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream.
[backport for 5.10.y]
Hi Amir, hi Dave,
I've got no objections to backporting this change at all. We've been using the patch on our internal 5.15 tracker branch happily for several months now.
Would like to highlight though that it's currently not yet merged in linux-stable 5.15 branch either (it's in 5.19 and mainline alright). If this gets queued for 5.10 then maybe it also should be for 5.15 ?
Hi Frank,
Quoting from my cover letter:
Patches 6-7 in this 5.10.y update have not been applied to 5.15.y yet. I pointed Leah's attention to these patches and she said she will include them in a following 5.15.y update.
Thanks, Amir.
On Thu, Sep 01, 2022 at 12:30:13PM +0300, Amir Goldstein wrote:
On Thu, Sep 1, 2022 at 12:04 PM Frank Hofmann fhofmann@cloudflare.com wrote:
On Thu, Sep 1, 2022 at 6:49 AM Amir Goldstein amir73il@gmail.com wrote:
From: Dave Chinner dchinner@redhat.com
commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream.
[backport for 5.10.y]
Hi Amir, hi Dave,
I've got no objections to backporting this change at all. We've been using the patch on our internal 5.15 tracker branch happily for several months now.
Would like to highlight though that it's currently not yet merged in linux-stable 5.15 branch either (it's in 5.19 and mainline alright). If this gets queued for 5.10 then maybe it also should be for 5.15 ?
Hi Frank,
Quoting from my cover letter:
Patches 6-7 in this 5.10.y update have not been applied to 5.15.y yet. I pointed Leah's attention to these patches and she said she will include them in a following 5.15.y update.
And as you know, this means I can't take this series at all until that series is ready, so to help us out, in the future, just don't even send them until they are all ready together.
thanks,
greg k-h
On Thu, Sep 1, 2022 at 12:41 PM Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Thu, Sep 01, 2022 at 12:30:13PM +0300, Amir Goldstein wrote:
On Thu, Sep 1, 2022 at 12:04 PM Frank Hofmann fhofmann@cloudflare.com wrote:
On Thu, Sep 1, 2022 at 6:49 AM Amir Goldstein amir73il@gmail.com wrote:
From: Dave Chinner dchinner@redhat.com
commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream.
[backport for 5.10.y]
Hi Amir, hi Dave,
I've got no objections to backporting this change at all. We've been using the patch on our internal 5.15 tracker branch happily for several months now.
Would like to highlight though that it's currently not yet merged in linux-stable 5.15 branch either (it's in 5.19 and mainline alright). If this gets queued for 5.10 then maybe it also should be for 5.15 ?
Hi Frank,
Quoting from my cover letter:
Patches 6-7 in this 5.10.y update have not been applied to 5.15.y yet. I pointed Leah's attention to these patches and she said she will include them in a following 5.15.y update.
And as you know, this means I can't take this series at all until that series is ready, so to help us out, in the future, just don't even send them until they are all ready together.
What?
You cannot take backports to 5.10.y before they are applied to 5.15.y? Since when?
Thanks, Amir.
On Thu, Sep 01, 2022 at 01:16:33PM +0300, Amir Goldstein wrote:
On Thu, Sep 1, 2022 at 12:41 PM Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Thu, Sep 01, 2022 at 12:30:13PM +0300, Amir Goldstein wrote:
On Thu, Sep 1, 2022 at 12:04 PM Frank Hofmann fhofmann@cloudflare.com wrote:
On Thu, Sep 1, 2022 at 6:49 AM Amir Goldstein amir73il@gmail.com wrote:
From: Dave Chinner dchinner@redhat.com
commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream.
[backport for 5.10.y]
Hi Amir, hi Dave,
I've got no objections to backporting this change at all. We've been using the patch on our internal 5.15 tracker branch happily for several months now.
Would like to highlight though that it's currently not yet merged in linux-stable 5.15 branch either (it's in 5.19 and mainline alright). If this gets queued for 5.10 then maybe it also should be for 5.15 ?
Hi Frank,
Quoting from my cover letter:
Patches 6-7 in this 5.10.y update have not been applied to 5.15.y yet. I pointed Leah's attention to these patches and she said she will include them in a following 5.15.y update.
And as you know, this means I can't take this series at all until that series is ready, so to help us out, in the future, just don't even send them until they are all ready together.
What?
You cannot take backports to 5.10.y before they are applied to 5.15.y? Since when?
Since always.
Why would you ever want someone to upgrade from an older tree (like 5.10.y) to a newer one (5.15.y) and have a regression?
So we always try to make sure patches are always applied to newer trees first. Yes, sometimes we miss this and make mistakes, but it's always been this way and we fix that whenever it happens accidentally.
I'll drop this series from my review queue for now until the 5.15.y series shows up.
thanks,
greg k-h
On Thu, Sep 1, 2022 at 1:26 PM Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Thu, Sep 01, 2022 at 01:16:33PM +0300, Amir Goldstein wrote:
On Thu, Sep 1, 2022 at 12:41 PM Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Thu, Sep 01, 2022 at 12:30:13PM +0300, Amir Goldstein wrote:
On Thu, Sep 1, 2022 at 12:04 PM Frank Hofmann fhofmann@cloudflare.com wrote:
On Thu, Sep 1, 2022 at 6:49 AM Amir Goldstein amir73il@gmail.com wrote:
From: Dave Chinner dchinner@redhat.com
commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream.
[backport for 5.10.y]
Hi Amir, hi Dave,
I've got no objections to backporting this change at all. We've been using the patch on our internal 5.15 tracker branch happily for several months now.
Would like to highlight though that it's currently not yet merged in linux-stable 5.15 branch either (it's in 5.19 and mainline alright). If this gets queued for 5.10 then maybe it also should be for 5.15 ?
Hi Frank,
Quoting from my cover letter:
Patches 6-7 in this 5.10.y update have not been applied to 5.15.y yet. I pointed Leah's attention to these patches and she said she will include them in a following 5.15.y update.
And as you know, this means I can't take this series at all until that series is ready, so to help us out, in the future, just don't even send them until they are all ready together.
What?
You cannot take backports to 5.10.y before they are applied to 5.15.y? Since when?
Since always.
Why would you ever want someone to upgrade from an older tree (like 5.10.y) to a newer one (5.15.y) and have a regression?
That is certainly not a goal when backporting fixes to 5.10.y, but it can happen as a by-product of the decentralized nature of testing backports.
But it did not bother you when xfs patches were applied to 5.4.y and no xfs patches at all applied to 5.10.y for two years?
So we always try to make sure patches are always applied to newer trees first. Yes, sometimes we miss this and make mistakes, but it's always been this way and we fix that whenever it happens accidentally.
That is my intention. I will try to keep to that rule in the future. I would have waited for the patches to land in 5.15.y, but Leah got distracted by another task so I decided to not wait, knowing that the patches are already in her queue.
I'll drop this series from my review queue for now until the 5.15.y series shows up.
Please don't drop the series. Please drop patches 6-7 if you must Or if you insist I can re-post patches 1-5.
Thanks, Amir.
On Thu, Sep 01, 2022 at 03:37:59PM +0300, Amir Goldstein wrote:
On Thu, Sep 1, 2022 at 1:26 PM Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Thu, Sep 01, 2022 at 01:16:33PM +0300, Amir Goldstein wrote:
On Thu, Sep 1, 2022 at 12:41 PM Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Thu, Sep 01, 2022 at 12:30:13PM +0300, Amir Goldstein wrote:
On Thu, Sep 1, 2022 at 12:04 PM Frank Hofmann fhofmann@cloudflare.com wrote:
On Thu, Sep 1, 2022 at 6:49 AM Amir Goldstein amir73il@gmail.com wrote: > > From: Dave Chinner dchinner@redhat.com > > commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream. > > [backport for 5.10.y]
Hi Amir, hi Dave,
I've got no objections to backporting this change at all. We've been using the patch on our internal 5.15 tracker branch happily for several months now.
Would like to highlight though that it's currently not yet merged in linux-stable 5.15 branch either (it's in 5.19 and mainline alright). If this gets queued for 5.10 then maybe it also should be for 5.15 ?
Hi Frank,
Quoting from my cover letter:
Patches 6-7 in this 5.10.y update have not been applied to 5.15.y yet. I pointed Leah's attention to these patches and she said she will include them in a following 5.15.y update.
And as you know, this means I can't take this series at all until that series is ready, so to help us out, in the future, just don't even send them until they are all ready together.
What?
You cannot take backports to 5.10.y before they are applied to 5.15.y? Since when?
Since always.
Why would you ever want someone to upgrade from an older tree (like 5.10.y) to a newer one (5.15.y) and have a regression?
That is certainly not a goal when backporting fixes to 5.10.y, but it can happen as a by-product of the decentralized nature of testing backports.
But it did not bother you when xfs patches were applied to 5.4.y and no xfs patches at all applied to 5.10.y for two years?
I've been bothered by xfs patches for so long that really, I didn't care as the maintainers didn't seem bothered either :)
But now that everything is working properly, let's do it correctly please.
So we always try to make sure patches are always applied to newer trees first. Yes, sometimes we miss this and make mistakes, but it's always been this way and we fix that whenever it happens accidentally.
That is my intention. I will try to keep to that rule in the future. I would have waited for the patches to land in 5.15.y, but Leah got distracted by another task so I decided to not wait, knowing that the patches are already in her queue.
I'll drop this series from my review queue for now until the 5.15.y series shows up.
Please don't drop the series. Please drop patches 6-7 if you must Or if you insist I can re-post patches 1-5.
Please resend as just the correct ones that you want so I know what to pick exactly.
thanks,
greg k-h
On Thu, Sep 1, 2022 at 4:07 PM Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Thu, Sep 01, 2022 at 03:37:59PM +0300, Amir Goldstein wrote:
On Thu, Sep 1, 2022 at 1:26 PM Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Thu, Sep 01, 2022 at 01:16:33PM +0300, Amir Goldstein wrote:
On Thu, Sep 1, 2022 at 12:41 PM Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Thu, Sep 01, 2022 at 12:30:13PM +0300, Amir Goldstein wrote:
On Thu, Sep 1, 2022 at 12:04 PM Frank Hofmann fhofmann@cloudflare.com wrote: > > On Thu, Sep 1, 2022 at 6:49 AM Amir Goldstein amir73il@gmail.com wrote: > > > > From: Dave Chinner dchinner@redhat.com > > > > commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream. > > > > [backport for 5.10.y] > > Hi Amir, hi Dave, > > I've got no objections to backporting this change at all. We've been > using the patch on our internal 5.15 tracker branch happily for > several months now. > > Would like to highlight though that it's currently not yet merged in > linux-stable 5.15 branch either (it's in 5.19 and mainline alright). > If this gets queued for 5.10 then maybe it also should be for 5.15 ? >
Hi Frank,
Quoting from my cover letter:
Patches 6-7 in this 5.10.y update have not been applied to 5.15.y yet. I pointed Leah's attention to these patches and she said she will include them in a following 5.15.y update.
And as you know, this means I can't take this series at all until that series is ready, so to help us out, in the future, just don't even send them until they are all ready together.
What?
You cannot take backports to 5.10.y before they are applied to 5.15.y? Since when?
Since always.
Why would you ever want someone to upgrade from an older tree (like 5.10.y) to a newer one (5.15.y) and have a regression?
That is certainly not a goal when backporting fixes to 5.10.y, but it can happen as a by-product of the decentralized nature of testing backports.
But it did not bother you when xfs patches were applied to 5.4.y and no xfs patches at all applied to 5.10.y for two years?
I've been bothered by xfs patches for so long that really, I didn't care as the maintainers didn't seem bothered either :)
But now that everything is working properly, let's do it correctly please.
So we always try to make sure patches are always applied to newer trees first. Yes, sometimes we miss this and make mistakes, but it's always been this way and we fix that whenever it happens accidentally.
That is my intention. I will try to keep to that rule in the future. I would have waited for the patches to land in 5.15.y, but Leah got distracted by another task so I decided to not wait, knowing that the patches are already in her queue.
I'll drop this series from my review queue for now until the 5.15.y series shows up.
Please don't drop the series. Please drop patches 6-7 if you must Or if you insist I can re-post patches 1-5.
Please resend as just the correct ones that you want so I know what to pick exactly.
Posted v3 with patches 1-5.
Thanks, Amir.
On Thu, Sep 1, 2022 at 10:30 AM Amir Goldstein amir73il@gmail.com wrote:
On Thu, Sep 1, 2022 at 12:04 PM Frank Hofmann fhofmann@cloudflare.com wrote:
On Thu, Sep 1, 2022 at 6:49 AM Amir Goldstein amir73il@gmail.com wrote:
From: Dave Chinner dchinner@redhat.com
commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream.
[backport for 5.10.y]
Hi Amir, hi Dave,
I've got no objections to backporting this change at all. We've been using the patch on our internal 5.15 tracker branch happily for several months now.
Would like to highlight though that it's currently not yet merged in linux-stable 5.15 branch either (it's in 5.19 and mainline alright). If this gets queued for 5.10 then maybe it also should be for 5.15 ?
Hi Frank,
Quoting from my cover letter:
Patches 6-7 in this 5.10.y update have not been applied to 5.15.y yet. I pointed Leah's attention to these patches and she said she will include them in a following 5.15.y update.
Thanks, Amir.
Apologies missing that one ... I've only been directly cc:'ed on 6/7 though.
In any case, for 6/7, whether applied to 5.10.y or 5.15.y,
Acked-by: Frank Hofmann fhofmann@cloudflare.com
And yes, I hope to see it show up in both LTS branches.
FrankH.
From: Dave Chinner dchinner@redhat.com
commit 1eb70f54c445fcbb25817841e774adb3d912f3e8 upstream.
[backport for 5.10.y]
xfs_repair catches fork size/format mismatches, but the in-kernel verifier doesn't, leading to null pointer failures when attempting to perform operations on the fork. This can occur in the xfs_dir_is_empty() where the in-memory fork format does not match the size and so the fork data pointer is accessed incorrectly.
Note: this causes new failures in xfs/348 which is testing mode vs ftype mismatches. We now detect a regular file that has been changed to a directory or symlink mode as being corrupt because the data fork is for a symlink or directory should be in local form when there are only 3 bytes of data in the data fork. Hence the inode verify for the regular file now fires w/ -EFSCORRUPTED because the inode fork format does not match the format the corrupted mode says it should be in.
Signed-off-by: Dave Chinner dchinner@redhat.com Reviewed-by: Christoph Hellwig hch@lst.de Reviewed-by: Darrick J. Wong djwong@kernel.org Signed-off-by: Dave Chinner david@fromorbit.com Signed-off-by: Amir Goldstein amir73il@gmail.com Acked-by: Darrick J. Wong djwong@kernel.org --- fs/xfs/libxfs/xfs_inode_buf.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c index c667c63f2cb0..fa8aefe6b7ec 100644 --- a/fs/xfs/libxfs/xfs_inode_buf.c +++ b/fs/xfs/libxfs/xfs_inode_buf.c @@ -358,19 +358,36 @@ xfs_dinode_verify_fork( int whichfork) { uint32_t di_nextents = XFS_DFORK_NEXTENTS(dip, whichfork); + mode_t mode = be16_to_cpu(dip->di_mode); + uint32_t fork_size = XFS_DFORK_SIZE(dip, mp, whichfork); + uint32_t fork_format = XFS_DFORK_FORMAT(dip, whichfork);
- switch (XFS_DFORK_FORMAT(dip, whichfork)) { + /* + * For fork types that can contain local data, check that the fork + * format matches the size of local data contained within the fork. + * + * For all types, check that when the size says the should be in extent + * or btree format, the inode isn't claiming it is in local format. + */ + if (whichfork == XFS_DATA_FORK) { + if (S_ISDIR(mode) || S_ISLNK(mode)) { + if (be64_to_cpu(dip->di_size) <= fork_size && + fork_format != XFS_DINODE_FMT_LOCAL) + return __this_address; + } + + if (be64_to_cpu(dip->di_size) > fork_size && + fork_format == XFS_DINODE_FMT_LOCAL) + return __this_address; + } + + switch (fork_format) { case XFS_DINODE_FMT_LOCAL: /* - * no local regular files yet + * No local regular files yet. */ - if (whichfork == XFS_DATA_FORK) { - if (S_ISREG(be16_to_cpu(dip->di_mode))) - return __this_address; - if (be64_to_cpu(dip->di_size) > - XFS_DFORK_SIZE(dip, mp, whichfork)) - return __this_address; - } + if (S_ISREG(mode) && whichfork == XFS_DATA_FORK) + return __this_address; if (di_nextents) return __this_address; break;
linux-stable-mirror@lists.linaro.org
-
Amir Goldstein -
Frank Hofmann -
Greg Kroah-Hartman