Hi Bob,
On 04/05/2023 18:43, Bob Peterson wrote:
Before this patch function gfs2_dinode_dealloc would abort if it got a
bad return code from gfs2_rindex_update. The problem is that it left the
dinode in the unlinked (not free) state, which meant subsequent fsck
would clean it up and flag an error. That meant some of our QE tests
would fail.
As I understand it the test is an interrupted rename loop workload and
gfs2_grow at the same time, and the bad return code is -EINTR, right?
The sole purpose of gfs2_rindex_update, in this code path, is to read in
any newer rgrps added by gfs2_grow. But since this is a delete operation
it won't actually use any of those new rgrps. It can really only twiddle
the bits from "Unlinked" to "Free" in an existing rgrp. Therefore the
error should not prevent the transition from unlinked to free.
This patch makes gfs2_dinode_dealloc ignore the bad return code and
proceed with freeing the dinode so the QE tests will not be tripped up.
Is it really ok to ignore all potential errors here? I wonder if it
should just ignore -EINTR (or whichever error the test produces) so that
it can still fail well for errors like -EIO.
Cheers,
Andy
Signed-off-by: Bob Peterson <rpete...@redhat.com>
---
fs/gfs2/super.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index d3b5c6278be0..1f23d7845123 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -1131,9 +1131,7 @@ static int gfs2_dinode_dealloc(struct gfs2_inode *ip)
return -EIO;
}
- error = gfs2_rindex_update(sdp);
- if (error)
- return error;
+ gfs2_rindex_update(sdp);
error = gfs2_quota_hold(ip, NO_UID_QUOTA_CHANGE, NO_GID_QUOTA_CHANGE);
if (error)
