All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Luis Henriques (SUSE)" <luis.henriques@linux.dev>
To: Theodore Ts'o <tytso@mit.edu>, Andreas Dilger <adilger@dilger.ca>
Cc: linux-ext4@vger.kernel.org,
	"Luis Henriques (SUSE)" <luis.henriques@linux.dev>
Subject: [PATCH v3 2/4] e2fsck: update quota when deallocating a bad inode
Date: Fri,  5 Apr 2024 15:24:03 +0100	[thread overview]
Message-ID: <20240405142405.12312-3-luis.henriques@linux.dev> (raw)
In-Reply-To: <20240405142405.12312-1-luis.henriques@linux.dev>

If a bad inode is found it will be deallocated.  However, if the filesystem has
quota enabled, the quota information isn't being updated accordingly.  This
issue was detected by running fstest ext4/019.

This patch fixes the issue by decreasing the inode count from the
quota and, if blocks are also being released, also subtract them as well.

While there, and as suggested by Andreas Dilger, the deallocate_inode()
function documentation is also updated by this patch to make it clear what
that function really does.

Signed-off-by: Luis Henriques (SUSE) <luis.henriques@linux.dev>
---
 e2fsck/pass2.c | 43 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 11 deletions(-)

diff --git a/e2fsck/pass2.c b/e2fsck/pass2.c
index b91628567a7f..08ab40fa8ba1 100644
--- a/e2fsck/pass2.c
+++ b/e2fsck/pass2.c
@@ -1854,17 +1854,26 @@ static int deallocate_inode_block(ext2_filsys fs,
 }
 
 /*
- * This function deallocates an inode
+ * This function reverts various counters and bitmaps incremented in
+ * pass1 for the inode, blocks, and quotas before it was decided the
+ * inode was corrupt and needed to be cleared.  This avoids the need
+ * to run e2fsck a second time (or have it restart itself) to repair
+ * these counters.
+ *
+ * It does not modify any on-disk state, so even if the inode is bad
+ * it _should_ reset in-memory state to before the inode was first
+ * processed.
  */
 static void deallocate_inode(e2fsck_t ctx, ext2_ino_t ino, char* block_buf)
 {
 	ext2_filsys fs = ctx->fs;
-	struct ext2_inode	inode;
+	struct ext2_inode_large	inode;
 	struct problem_context	pctx;
 	__u32			count;
 	struct del_block	del_block;
 
-	e2fsck_read_inode(ctx, ino, &inode, "deallocate_inode");
+	e2fsck_read_inode_full(ctx, ino, EXT2_INODE(&inode),
+			       sizeof(inode), "deallocate_inode");
 	clear_problem_context(&pctx);
 	pctx.ino = ino;
 
@@ -1874,29 +1883,29 @@ static void deallocate_inode(e2fsck_t ctx, ext2_ino_t ino, char* block_buf)
 	e2fsck_read_bitmaps(ctx);
 	ext2fs_inode_alloc_stats2(fs, ino, -1, LINUX_S_ISDIR(inode.i_mode));
 
-	if (ext2fs_file_acl_block(fs, &inode) &&
+	if (ext2fs_file_acl_block(fs, EXT2_INODE(&inode)) &&
 	    ext2fs_has_feature_xattr(fs->super)) {
 		pctx.errcode = ext2fs_adjust_ea_refcount3(fs,
-				ext2fs_file_acl_block(fs, &inode),
+				ext2fs_file_acl_block(fs, EXT2_INODE(&inode)),
 				block_buf, -1, &count, ino);
 		if (pctx.errcode == EXT2_ET_BAD_EA_BLOCK_NUM) {
 			pctx.errcode = 0;
 			count = 1;
 		}
 		if (pctx.errcode) {
-			pctx.blk = ext2fs_file_acl_block(fs, &inode);
+			pctx.blk = ext2fs_file_acl_block(fs, EXT2_INODE(&inode));
 			fix_problem(ctx, PR_2_ADJ_EA_REFCOUNT, &pctx);
 			ctx->flags |= E2F_FLAG_ABORT;
 			return;
 		}
 		if (count == 0) {
 			ext2fs_block_alloc_stats2(fs,
-				  ext2fs_file_acl_block(fs, &inode), -1);
+				  ext2fs_file_acl_block(fs, EXT2_INODE(&inode)), -1);
 		}
-		ext2fs_file_acl_block_set(fs, &inode, 0);
+		ext2fs_file_acl_block_set(fs, EXT2_INODE(&inode), 0);
 	}
 
-	if (!ext2fs_inode_has_valid_blocks2(fs, &inode))
+	if (!ext2fs_inode_has_valid_blocks2(fs, EXT2_INODE(&inode)))
 		goto clear_inode;
 
 	/* Inline data inodes don't have blocks to iterate */
@@ -1921,10 +1930,22 @@ static void deallocate_inode(e2fsck_t ctx, ext2_ino_t ino, char* block_buf)
 		ctx->flags |= E2F_FLAG_ABORT;
 		return;
 	}
+
+	if ((ino != quota_type2inum(PRJQUOTA, fs->super)) &&
+	    (ino != fs->super->s_orphan_file_inum) &&
+	    (ino == EXT2_ROOT_INO || ino >= EXT2_FIRST_INODE(ctx->fs->super)) &&
+	    !(inode.i_flags & EXT4_EA_INODE_FL)) {
+		if (del_block.num > 0)
+			quota_data_sub(ctx->qctx, &inode, ino,
+				       del_block.num * EXT2_CLUSTER_SIZE(fs->super));
+		quota_data_inodes(ctx->qctx, (struct ext2_inode_large *)&inode,
+				  ino, -1);
+	}
+
 clear_inode:
 	/* Inode may have changed by block_iterate, so reread it */
-	e2fsck_read_inode(ctx, ino, &inode, "deallocate_inode");
-	e2fsck_clear_inode(ctx, ino, &inode, 0, "deallocate_inode");
+	e2fsck_read_inode(ctx, ino, EXT2_INODE(&inode), "deallocate_inode");
+	e2fsck_clear_inode(ctx, ino, EXT2_INODE(&inode), 0, "deallocate_inode");
 }
 
 /*

  parent reply	other threads:[~2024-04-05 14:24 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-05 14:24 [PATCH e2fsprogs v3 0/4] quota-related e2fsck fixes and tests Luis Henriques (SUSE)
2024-04-05 14:24 ` [PATCH v3 1/4] e2fsck: update quota accounting after directory optimization Luis Henriques (SUSE)
2024-04-05 14:24 ` Luis Henriques (SUSE) [this message]
2024-04-05 14:24 ` [PATCH v3 3/4] tests: new test to check quota " Luis Henriques (SUSE)
2024-04-05 14:24 ` [PATCH v3 4/4] tests: new test to check quota after a bad inode deallocation Luis Henriques (SUSE)
2024-04-17  2:03 ` [PATCH e2fsprogs v3 0/4] quota-related e2fsck fixes and tests Theodore Ts'o

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240405142405.12312-3-luis.henriques@linux.dev \
    --to=luis.henriques@linux.dev \
    --cc=adilger@dilger.ca \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.