Skip to contentSkip to conversation

bfs iget() abuses

PATCH In review Al Viro 2 participants 2 messages Oct 4, 2005 – Oct 5, 2005

lore
Share & tools
lore

Al Viro

author
#0 lore raw

PATCH 1/1 bfs iget() abuses

bfs_fill_super() walks the inode table to get the bitmap of
free inodes and collect stats. It has no business using iget() for that - it's a lot of extra work, extra icache pollution and more complex code. Switched to walking the damn thing directly.

	Note: that also allows to kill ->i_dsk_ino in there - separate

patch if Tigran can confirm that this field can be zero only for deleted inodes (i.e. something that could only be found during that scan and not by normal lookups).

----
[this is a followup to Alexey's bfs annotations patch]

fs/bfs/inode.c +30 -12
diff -urN RC14-rc3-git4-base/fs/bfs/inode.c current/fs/bfs/inode.c
@@ -362,23 +362,41 @@
 	info->si_lf_eblk = 0;
 	info->si_lf_sblk = 0;
 	info->si_lf_ioff = 0;
+	bh = NULL;
 	for (i=BFS_ROOT_INO; i<=info->si_lasti; i++) {
-		inode = iget(s,i);
-		if (BFS_I(inode)->i_dsk_ino == 0)
+		struct bfs_inode *di;
+		int block = (i - BFS_ROOT_INO)/BFS_INODES_PER_BLOCK + 1;
+		int off = (i - BFS_ROOT_INO) % BFS_INODES_PER_BLOCK;
+		unsigned long sblock, eblock;
+
+		if (!off) {
+			brelse(bh);
+			bh = sb_bread(s, block);
+		}
+
+		if (!bh)
+			continue;
+
+		di = (struct bfs_inode *)bh->b_data + off;
+
+		if (!di->i_ino) {
 			info->si_freei++;
-		else {
-			set_bit(i, info->si_imap);
-			info->si_freeb -= inode->i_blocks;
-			if (BFS_I(inode)->i_eblock > info->si_lf_eblk) {
-				info->si_lf_eblk = BFS_I(inode)->i_eblock;
-				info->si_lf_sblk = BFS_I(inode)->i_sblock;
-				info->si_lf_ioff = BFS_INO2OFF(i);
-			}
+			continue;
+		}
+		set_bit(i, info->si_imap);
+		info->si_freeb -= BFS_FILEBLOCKS(di);
+
+		sblock =  le32_to_cpu(di->i_sblock);
+		eblock =  le32_to_cpu(di->i_eblock);
+		if (eblock > info->si_lf_eblk) {
+			info->si_lf_eblk = eblock;
+			info->si_lf_sblk = sblock;
+			info->si_lf_ioff = BFS_INO2OFF(i);
 		}
-		iput(inode);
 	}
+	brelse(bh);
 	if (!(s->s_flags & MS_RDONLY)) {
-		mark_buffer_dirty(bh);
+		mark_buffer_dirty(info->si_sbh);
 		s->s_dirt = 1;
 	} 
 	dump_imap("read_super", s);
#1 lore raw

Hi Al,

Yes, looks good to me and also I confirm that ->i_dsk_ino is only zero for deletes inodes.

Kind regards
Tigran

Quoted 72 lines · Al Viro

bfs_fill_super() walks the inode table to get the bitmap of
free inodes and collect stats. It has no business using iget() for that - it's a lot of extra work, extra icache pollution and more complex code. Switched to walking the damn thing directly.

	Note: that also allows to kill ->i_dsk_ino in there - separate

patch if Tigran can confirm that this field can be zero only for deleted inodes (i.e. something that could only be found during that scan and not by normal lookups).

----
[this is a followup to Alexey's bfs annotations patch]

diff -urN RC14-rc3-git4-base/fs/bfs/inode.c current/fs/bfs/inode.c

fs/bfs/inode.c +30 -12
@@ -362,23 +362,41 @@
 	info->si_lf_eblk = 0;
 	info->si_lf_sblk = 0;
 	info->si_lf_ioff = 0;
+	bh = NULL;
 	for (i=BFS_ROOT_INO; i<=info->si_lasti; i++) {
-		inode = iget(s,i);
-		if (BFS_I(inode)->i_dsk_ino == 0)
+		struct bfs_inode *di;
+		int block = (i - BFS_ROOT_INO)/BFS_INODES_PER_BLOCK + 1;
+		int off = (i - BFS_ROOT_INO) % BFS_INODES_PER_BLOCK;
+		unsigned long sblock, eblock;
+
+		if (!off) {
+			brelse(bh);
+			bh = sb_bread(s, block);
+		}
+
+		if (!bh)
+			continue;
+
+		di = (struct bfs_inode *)bh->b_data + off;
+
+		if (!di->i_ino) {
 			info->si_freei++;
-		else {
-			set_bit(i, info->si_imap);
-			info->si_freeb -= inode->i_blocks;
-			if (BFS_I(inode)->i_eblock > info->si_lf_eblk) {
-				info->si_lf_eblk = BFS_I(inode)->i_eblock;
-				info->si_lf_sblk = BFS_I(inode)->i_sblock;
-				info->si_lf_ioff = BFS_INO2OFF(i);
-			}
+			continue;
+		}
+		set_bit(i, info->si_imap);
+		info->si_freeb -= BFS_FILEBLOCKS(di);
+
+		sblock =  le32_to_cpu(di->i_sblock);
+		eblock =  le32_to_cpu(di->i_eblock);
+		if (eblock > info->si_lf_eblk) {
+			info->si_lf_eblk = eblock;
+			info->si_lf_sblk = sblock;
+			info->si_lf_ioff = BFS_INO2OFF(i);
 		}
-		iput(inode);
 	}
+	brelse(bh);
 	if (!(s->s_flags & MS_RDONLY)) {
-		mark_buffer_dirty(bh);
+		mark_buffer_dirty(info->si_sbh);
 		s->s_dirt = 1;
 	}
 	dump_imap("read_super", s);