All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Sandeen <sandeen@redhat.com>
To: Eric Sandeen <sandeen@redhat.com>,
	ext4 development <linux-ext4@vger.kernel.org>,
	Alex Tomas <alex@clusterfs.com>
Subject: [PATCH] UPDATED3: types fixup for mballoc
Date: Tue, 08 Jan 2008 13:54:10 -0600	[thread overview]
Message-ID: <4783D4E2.5080801@redhat.com> (raw)
In-Reply-To: <477D4152.4020901@redhat.com>

4th time's the charm?

Note, the calculations Andreas & I were discussing only work properly
for stripe <= blocks per group... I don't know if we'd need to enforce
that at mount time?

-------------

I ran into a potential overflow in ext4_mb_scan_aligned, 
and went looking for others in mballoc.  This patch hits a 
few spots, compile-tested only at this point, comments welcome.

This patch:

changes fe_len to an int, I don't think we need it to be a long,
looking at how it's used (should it be a grpblk_t?)  Also change
anything assigned to return value of mb_find_extent, since it returns
fe_len.

changes anything that does groupno * EXT4_BLOCKS_PER_GROUP
or pa->pa_pstart + <whatever> to an ext4_fsblk_t

avoids 64-bit divides & modulos, and...

fixes up any related formats

Signed-off-by: Eric Sandeen <sandeen@redhat.com>

---

Index: linux-2.6.24-rc3/fs/ext4/mballoc.c
===================================================================
--- linux-2.6.24-rc3.orig/fs/ext4/mballoc.c
+++ linux-2.6.24-rc3/fs/ext4/mballoc.c
@@ -363,7 +363,7 @@ struct ext4_free_extent {
 	ext4_lblk_t fe_logical;
 	ext4_grpblk_t fe_start;
 	ext4_group_t fe_group;
-	unsigned long fe_len;
+	int fe_len;
 };
 
 /*
@@ -586,14 +586,14 @@ static void mb_free_blocks_double(struct
 	BUG_ON(!ext4_is_group_locked(sb, e4b->bd_group));
 	for (i = 0; i < count; i++) {
 		if (!mb_test_bit(first + i, e4b->bd_info->bb_bitmap)) {
-			unsigned long blocknr;
+			ext4_fsblk_t blocknr;
 			blocknr = e4b->bd_group * EXT4_BLOCKS_PER_GROUP(sb);
 			blocknr += first + i;
 			blocknr +=
 			    le32_to_cpu(EXT4_SB(sb)->s_es->s_first_data_block);
 
 			ext4_error(sb, __FUNCTION__, "double-free of inode"
-				   " %lu's block %lu(bit %u in group %lu)\n",
+				   " %lu's block %llu(bit %u in group %lu)\n",
 				   inode ? inode->i_ino : 0, blocknr,
 				   first + i, e4b->bd_group);
 		}
@@ -1226,14 +1226,14 @@ static int mb_free_blocks(struct inode *
 		order = 0;
 
 		if (!mb_test_bit(block, EXT4_MB_BITMAP(e4b))) {
-			unsigned long blocknr;
+			ext4_fsblk_t blocknr;
 			blocknr = e4b->bd_group * EXT4_BLOCKS_PER_GROUP(sb);
 			blocknr += block;
 			blocknr +=
 			    le32_to_cpu(EXT4_SB(sb)->s_es->s_first_data_block);
 
 			ext4_error(sb, __FUNCTION__, "double-free of inode"
-				   " %lu's block %lu(bit %u in group %lu)\n",
+				   " %lu's block %llu(bit %u in group %lu)\n",
 				   inode ? inode->i_ino : 0, blocknr, block,
 				   e4b->bd_group);
 		}
@@ -1416,7 +1416,7 @@ static void ext4_mb_use_best_found(struc
 					struct ext4_buddy *e4b)
 {
 	struct ext4_sb_info *sbi = EXT4_SB(ac->ac_sb);
-	unsigned long ret;
+	int ret;
 
 	BUG_ON(ac->ac_b_ex.fe_group != e4b->bd_group);
 	BUG_ON(ac->ac_status == AC_STATUS_FOUND);
@@ -1609,10 +1609,12 @@ static int ext4_mb_find_by_goal(struct e
 			     ac->ac_g_ex.fe_len, &ex);
 
 	if (max >= ac->ac_g_ex.fe_len && ac->ac_g_ex.fe_len == sbi->s_stripe) {
-		unsigned long start;
-		start = (e4b->bd_group * EXT4_BLOCKS_PER_GROUP(ac->ac_sb) +
-			ex.fe_start + le32_to_cpu(es->s_first_data_block));
-		if (start % sbi->s_stripe == 0) {
+		ext4_fsblk_t start;
+
+		start = (e4b->bd_group * EXT4_BLOCKS_PER_GROUP(ac->ac_sb)) +
+			ex.fe_start + le32_to_cpu(es->s_first_data_block);
+		/* use do_div to get remainder (would be 64-bit modulo) */
+		if (do_div(start, sbi->s_stripe) == 0) {
 			ac->ac_found++;
 			ac->ac_b_ex = ex;
 			ext4_mb_use_best_found(ac, e4b);
@@ -1724,6 +1726,7 @@ static void ext4_mb_complex_scan_group(s
 /*
  * This is a special case for storages like raid5
  * we try to find stripe-aligned chunks for stripe-size requests
+ * XXX should do so at least for multiples of stripe size as well
  */
 static void ext4_mb_scan_aligned(struct ext4_allocation_context *ac,
 				 struct ext4_buddy *e4b)
@@ -1732,17 +1735,19 @@ static void ext4_mb_scan_aligned(struct 
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	void *bitmap = EXT4_MB_BITMAP(e4b);
 	struct ext4_free_extent ex;
-	unsigned long i;
-	unsigned long max;
+	ext4_fsblk_t first_group_block;
+	ext4_fsblk_t a;
+	ext4_grpblk_t i;
+	int max;
 
 	BUG_ON(sbi->s_stripe == 0);
 
-	/* find first stripe-aligned block */
-	i = e4b->bd_group * EXT4_BLOCKS_PER_GROUP(sb)
+	/* find first stripe-aligned block in group */
+	first_group_block = e4b->bd_group * EXT4_BLOCKS_PER_GROUP(sb)
 		+ le32_to_cpu(sbi->s_es->s_first_data_block);
-	i = ((i + sbi->s_stripe - 1) / sbi->s_stripe) * sbi->s_stripe;
-	i = (i - le32_to_cpu(sbi->s_es->s_first_data_block))
-			% EXT4_BLOCKS_PER_GROUP(sb);
+	a = first_group_block + sbi->s_stripe - 1;
+	do_div(a, sbi->s_stripe);
+	i = (a * sbi->s_stripe) - first_group_block;
 
 	while (i < sb->s_blocksize * 8) {
 		if (!mb_test_bit(i, bitmap)) {
@@ -2026,35 +2031,35 @@ static int ext4_mb_seq_history_show(stru
 	if (hs->op == EXT4_MB_HISTORY_ALLOC) {
 		fmt = "%-5u %-8u %-23s %-23s %-23s %-5u %-5u %-2u "
 			"%-5u %-5s %-5u %-6u\n";
-		sprintf(buf2, "%lu/%d/%lu@%lu", hs->result.fe_group,
+		sprintf(buf2, "%lu/%d/%u@%u", hs->result.fe_group,
 			hs->result.fe_start, hs->result.fe_len,
-			(unsigned long)hs->result.fe_logical);
-		sprintf(buf, "%lu/%d/%lu@%lu", hs->orig.fe_group,
+			hs->result.fe_logical);
+		sprintf(buf, "%lu/%d/%u@%u", hs->orig.fe_group,
 			hs->orig.fe_start, hs->orig.fe_len,
-			(unsigned long)hs->orig.fe_logical);
-		sprintf(buf3, "%lu/%d/%lu@%lu", hs->goal.fe_group,
+			hs->orig.fe_logical);
+		sprintf(buf3, "%lu/%d/%u@%u", hs->goal.fe_group,
 			hs->goal.fe_start, hs->goal.fe_len,
-			(unsigned long)hs->goal.fe_logical);
+			hs->goal.fe_logical);
 		seq_printf(seq, fmt, hs->pid, hs->ino, buf, buf3, buf2,
 				hs->found, hs->groups, hs->cr, hs->flags,
 				hs->merged ? "M" : "", hs->tail,
 				hs->buddy ? 1 << hs->buddy : 0);
 	} else if (hs->op == EXT4_MB_HISTORY_PREALLOC) {
 		fmt = "%-5u %-8u %-23s %-23s %-23s\n";
-		sprintf(buf2, "%lu/%d/%lu@%lu", hs->result.fe_group,
+		sprintf(buf2, "%lu/%d/%u@%u", hs->result.fe_group,
 			hs->result.fe_start, hs->result.fe_len,
-			(unsigned long)hs->result.fe_logical);
-		sprintf(buf, "%lu/%d/%lu@%lu", hs->orig.fe_group,
+			hs->result.fe_logical);
+		sprintf(buf, "%lu/%d/%u@%u", hs->orig.fe_group,
 			hs->orig.fe_start, hs->orig.fe_len,
-			(unsigned long)hs->orig.fe_logical);
+			hs->orig.fe_logical);
 		seq_printf(seq, fmt, hs->pid, hs->ino, buf, "", buf2);
 	} else if (hs->op == EXT4_MB_HISTORY_DISCARD) {
-		sprintf(buf2, "%lu/%d/%lu", hs->result.fe_group,
+		sprintf(buf2, "%lu/%d/%u", hs->result.fe_group,
 			hs->result.fe_start, hs->result.fe_len);
 		seq_printf(seq, "%-5u %-8u %-23s discard\n",
 				hs->pid, hs->ino, buf2);
 	} else if (hs->op == EXT4_MB_HISTORY_FREE) {
-		sprintf(buf2, "%lu/%d/%lu", hs->result.fe_group,
+		sprintf(buf2, "%lu/%d/%u", hs->result.fe_group,
 			hs->result.fe_start, hs->result.fe_len);
 		seq_printf(seq, "%-5u %-8u %-23s free\n",
 				hs->pid, hs->ino, buf2);
@@ -2943,7 +2948,7 @@ static int ext4_mb_mark_diskspace_used(s
 	struct buffer_head *gdp_bh;
 	struct ext4_sb_info *sbi;
 	struct super_block *sb;
-	sector_t block;
+	ext4_fsblk_t block;
 	int err;
 
 	BUG_ON(ac->ac_status != AC_STATUS_FOUND);
@@ -2984,8 +2989,8 @@ static int ext4_mb_mark_diskspace_used(s
 				EXT4_SB(sb)->s_itb_per_group)) {
 
 		ext4_error(sb, __FUNCTION__,
-			   "Allocating block in system zone - block = %lu",
-			   (unsigned long) block);
+			   "Allocating block in system zone - block = %llu",
+			   block);
 	}
 #ifdef AGGRESSIVE_CHECK
 	{
@@ -3250,12 +3255,13 @@ static void ext4_mb_use_inode_pa(struct 
 				struct ext4_prealloc_space *pa)
 {
 	ext4_fsblk_t start;
-	unsigned long len;
+	ext4_fsblk_t end;
+	int len;
 
 	/* found preallocated blocks, use them */
 	start = pa->pa_pstart + (ac->ac_o_ex.fe_logical - pa->pa_lstart);
-	len = min(pa->pa_pstart + pa->pa_len, start + ac->ac_o_ex.fe_len);
-	len = len - start;
+	end = min(pa->pa_pstart + pa->pa_len, start + ac->ac_o_ex.fe_len);
+	len = end - start;
 	ext4_get_group_no_and_offset(ac->ac_sb, start, &ac->ac_b_ex.fe_group,
 					&ac->ac_b_ex.fe_start);
 	ac->ac_b_ex.fe_len = len;
@@ -3952,8 +3958,8 @@ static void ext4_mb_show_ac(struct ext4_
 
 	printk(KERN_ERR "EXT4-fs: can't allocate: status %d flags %d\n",
 			ac->ac_status, ac->ac_flags);
-	printk(KERN_ERR "EXT4-fs: orig %lu/%lu/%lu@%lu, goal %lu/%lu/%lu@%lu, "
-			"best %lu/%lu/%lu@%lu cr %d\n",
+	printk(KERN_ERR "EXT4-fs: orig %lu/%lu/%u@%u, goal %lu/%lu/%u@%u, "
+			"best %lu/%lu/%u@%u cr %d\n",
 			ac->ac_o_ex.fe_group, ac->ac_o_ex.fe_start,
 			ac->ac_o_ex.fe_len, ac->ac_o_ex.fe_logical,
 			ac->ac_g_ex.fe_group, ac->ac_g_ex.fe_start,

  parent reply	other threads:[~2008-01-08 19:54 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-15 22:56 [PATCH] types fixup for mballoc Eric Sandeen
2007-11-16  4:49 ` Eric Sandeen
2008-01-02 20:01   ` [PATCH] UPDATED: " Eric Sandeen
2008-01-03 19:17     ` Andreas Dilger
2008-01-03 19:24       ` Eric Sandeen
2008-01-03 20:10         ` [PATCH] UPDATED2: " Eric Sandeen
2008-01-03 20:35           ` Andreas Dilger
2008-01-03 21:07             ` Eric Sandeen
2008-01-08 19:54           ` Eric Sandeen [this message]
2008-01-09  3:47             ` [PATCH] UPDATED3: " Andreas Dilger

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=4783D4E2.5080801@redhat.com \
    --to=sandeen@redhat.com \
    --cc=alex@clusterfs.com \
    --cc=linux-ext4@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 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.