linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Theodore Ts'o <tytso@mit.edu>
To: stable@vger.kernel.org
Cc: Ext4 Developers List <linux-ext4@vger.kernel.org>,
	Dmitry Monakhov <dmonakhov@openvz.org>,
	"Theodore Ts'o" <tytso@mit.edu>
Subject: [PATCH v2.6.34.y 12/28] ext4: clean up inode bitmaps manipulation in ext4_free_inode
Date: Tue,  1 Jun 2010 12:12:59 -0400	[thread overview]
Message-ID: <1275408795-17487-12-git-send-email-tytso@mit.edu> (raw)
In-Reply-To: <1275408795-17487-1-git-send-email-tytso@mit.edu>

From: Dmitry Monakhov <dmonakhov@openvz.org>

commit d17413c08cd2b1dd2bf2cfdbb0f7b736b2b2b15c upstrea (as of v2..34-git13)

- Reorganize locking scheme to batch two atomic operation in to one.
  This also allow us to state what healthy group must obey following rule
  ext4_free_inodes_count(sb, gdp) == ext4_count_free(inode_bitmap, NUM);
- Fix possible undefined pointer dereference.
- Even if group descriptor stats aren't accessible we have to update
  inode bitmaps.
- Move non-group members update out of group_lock.

Note: this commit has been observed to fix fs corruption problems
under heavy fs load

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/ialloc.c |   81 ++++++++++++++++++++++++-----------------------------
 1 files changed, 37 insertions(+), 44 deletions(-)

diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 57f6eef..52618d5 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -240,56 +240,49 @@ void ext4_free_inode(handle_t *handle, struct inode *inode)
 	if (fatal)
 		goto error_return;
 
-	/* Ok, now we can actually update the inode bitmaps.. */
-	cleared = ext4_clear_bit_atomic(ext4_group_lock_ptr(sb, block_group),
-					bit, bitmap_bh->b_data);
-	if (!cleared)
-		ext4_error(sb, "bit already cleared for inode %lu", ino);
-	else {
-		gdp = ext4_get_group_desc(sb, block_group, &bh2);
-
+	fatal = -ESRCH;
+	gdp = ext4_get_group_desc(sb, block_group, &bh2);
+	if (gdp) {
 		BUFFER_TRACE(bh2, "get_write_access");
 		fatal = ext4_journal_get_write_access(handle, bh2);
-		if (fatal) goto error_return;
-
-		if (gdp) {
-			ext4_lock_group(sb, block_group);
-			count = ext4_free_inodes_count(sb, gdp) + 1;
-			ext4_free_inodes_set(sb, gdp, count);
-			if (is_directory) {
-				count = ext4_used_dirs_count(sb, gdp) - 1;
-				ext4_used_dirs_set(sb, gdp, count);
-				if (sbi->s_log_groups_per_flex) {
-					ext4_group_t f;
-
-					f = ext4_flex_group(sbi, block_group);
-					atomic_dec(&sbi->s_flex_groups[f].used_dirs);
-				}
+	}
+	ext4_lock_group(sb, block_group);
+	cleared = ext4_clear_bit(bit, bitmap_bh->b_data);
+	if (fatal || !cleared) {
+		ext4_unlock_group(sb, block_group);
+		goto out;
+	}
 
-			}
-			gdp->bg_checksum = ext4_group_desc_csum(sbi,
-							block_group, gdp);
-			ext4_unlock_group(sb, block_group);
-			percpu_counter_inc(&sbi->s_freeinodes_counter);
-			if (is_directory)
-				percpu_counter_dec(&sbi->s_dirs_counter);
-
-			if (sbi->s_log_groups_per_flex) {
-				ext4_group_t f;
-
-				f = ext4_flex_group(sbi, block_group);
-				atomic_inc(&sbi->s_flex_groups[f].free_inodes);
-			}
-		}
-		BUFFER_TRACE(bh2, "call ext4_handle_dirty_metadata");
-		err = ext4_handle_dirty_metadata(handle, NULL, bh2);
-		if (!fatal) fatal = err;
+	count = ext4_free_inodes_count(sb, gdp) + 1;
+	ext4_free_inodes_set(sb, gdp, count);
+	if (is_directory) {
+		count = ext4_used_dirs_count(sb, gdp) - 1;
+		ext4_used_dirs_set(sb, gdp, count);
+		percpu_counter_dec(&sbi->s_dirs_counter);
 	}
-	BUFFER_TRACE(bitmap_bh, "call ext4_handle_dirty_metadata");
-	err = ext4_handle_dirty_metadata(handle, NULL, bitmap_bh);
-	if (!fatal)
-		fatal = err;
-	sb->s_dirt = 1;
+	gdp->bg_checksum = ext4_group_desc_csum(sbi, block_group, gdp);
+	ext4_unlock_group(sb, block_group);
+
+	percpu_counter_inc(&sbi->s_freeinodes_counter);
+	if (sbi->s_log_groups_per_flex) {
+		ext4_group_t f = ext4_flex_group(sbi, block_group);
+
+		atomic_inc(&sbi->s_flex_groups[f].free_inodes);
+		if (is_directory)
+			atomic_dec(&sbi->s_flex_groups[f].used_dirs);
+	}
+	BUFFER_TRACE(bh2, "call ext4_handle_dirty_metadata");
+	fatal = ext4_handle_dirty_metadata(handle, NULL, bh2);
+out:
+	if (cleared) {
+		BUFFER_TRACE(bitmap_bh, "call ext4_handle_dirty_metadata");
+		err = ext4_handle_dirty_metadata(handle, NULL, bitmap_bh);
+		if (!fatal)
+			fatal = err;
+		sb->s_dirt = 1;
+	} else
+		ext4_error(sb, "bit already cleared for inode %lu", ino);
+
 error_return:
 	brelse(bitmap_bh);
 	ext4_std_error(sb, fatal);
-- 
1.6.6.1.1.g974db.dirty


  parent reply	other threads:[~2010-06-01 16:13 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-01 16:12 [PATCH v2.6.34.y 01/28] ext4: check missed return value in ext4_sync_file() Theodore Ts'o
2010-06-01 16:12 ` [PATCH v2.6.34.y 02/28] ext4: fix memory leaks in error path handling of ext4_ext_zeroout() Theodore Ts'o
2010-06-01 16:12 ` [PATCH v2.6.34.y 03/28] ext4: Remove unnecessary call to ext4_get_group_desc() in mballoc Theodore Ts'o
2010-06-01 16:12 ` [PATCH v2.6.34.y 04/28] ext4: rename ext4_mb_release_desc() to ext4_mb_unload_buddy() Theodore Ts'o
2010-06-01 16:12 ` [PATCH v2.6.34.y 05/28] ext4: allow defrag (EXT4_IOC_MOVE_EXT) in 32bit compat mode Theodore Ts'o
2010-06-01 16:12 ` [PATCH v2.6.34.y 06/28] ext4: fix quota accounting in case of fallocate Theodore Ts'o
2010-06-01 16:12 ` [PATCH v2.6.34.y 07/28] ext4: check s_log_groups_per_flex in online resize code Theodore Ts'o
2010-06-01 16:12 ` [PATCH v2.6.34.y 08/28] ext4: don't return to userspace after freezing the fs with a mutex held Theodore Ts'o
2010-06-01 16:12 ` [PATCH v2.6.34.y 09/28] ext4: stop issuing discards if not supported by device Theodore Ts'o
2010-06-01 16:12 ` [PATCH v2.6.34.y 10/28] ext4: don't scan/accumulate more pages than mballoc will allocate Theodore Ts'o
2010-06-01 16:12 ` [PATCH v2.6.34.y 11/28] ext4: Do not zero out uninitialized extents beyond i_size Theodore Ts'o
2010-06-01 16:12 ` Theodore Ts'o [this message]
2010-06-01 16:13 ` [PATCH v2.6.34.y 13/28] ext4: init statistics after journal recovery Theodore Ts'o
2010-06-01 16:13 ` [PATCH v2.6.34.y 14/28] quota: use flags interface for dquot alloc/free space Theodore Ts'o
2010-06-01 16:13 ` [PATCH v2.6.34.y 15/28] quota: add the option to not fail with EDQUOT in block Theodore Ts'o
2010-06-01 16:13 ` [PATCH v2.6.34.y 16/28] ext4: don't use quota reservation for speculative metadata Theodore Ts'o
2010-06-01 16:13 ` [PATCH v2.6.34.y 17/28] ext4: Remove extraneous newlines in ext4_msg() calls Theodore Ts'o
2010-06-01 16:13 ` [PATCH v2.6.34.y 18/28] ext4: Prevent creation of files larger than RLIMIT_FSIZE using fallocate Theodore Ts'o
2010-06-01 16:13 ` [PATCH v2.6.34.y 19/28] ext4: check for a good block group before loading buddy pages Theodore Ts'o
2010-06-01 16:13 ` [PATCH v2.6.34.y 20/28] ext4: Show journal_checksum option Theodore Ts'o
2010-06-01 16:13 ` [PATCH v2.6.34.y 21/28] ext4: Use our own write_cache_pages() Theodore Ts'o
2010-06-01 16:13 ` [PATCH v2.6.34.y 22/28] ext4: Use bitops to read/modify i_flags in struct ext4_inode_info Theodore Ts'o
2010-06-01 16:13 ` [PATCH v2.6.34.y 23/28] ext4: Avoid crashing on NULL ptr dereference on a filesystem error Theodore Ts'o
2010-06-01 16:13 ` [PATCH v2.6.34.y 24/28] ext4: Clear the EXT4_EOFBLOCKS_FL flag only when warranted Theodore Ts'o
2010-06-01 16:13 ` [PATCH v2.6.34.y 25/28] ext4: restart ext4_ext_remove_space() after transaction restart Theodore Ts'o
2010-06-01 16:13 ` [PATCH v2.6.34.y 26/28] ext4: Conditionally define compat ioctl numbers Theodore Ts'o
2010-06-01 16:13 ` [PATCH v2.6.34.y 27/28] ext4: Fix compat EXT4_IOC_ADD_GROUP Theodore Ts'o
2010-06-01 16:13 ` [PATCH v2.6.34.y 28/28] ext4: Make fsync sync new parent directories in no-journal mode 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=1275408795-17487-12-git-send-email-tytso@mit.edu \
    --to=tytso@mit.edu \
    --cc=dmonakhov@openvz.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).