Re: [RFC] ext4-locality-groups patch

[Valerie Clement <valerie.clement@bull.net>]

Alex Tomas wrote:

> +/*
> + * entry function for inode syncing
> + * it's responsbility is to sort all inode out in their locality groups
> + */
> +void ext4_lg_sync_inodes(struct super_block *sb, struct writeback_control *wbc)
> +{
> +	struct ext4_sb_info *sbi = EXT4_SB(sb);
> +	struct ext4_locality_group *lg;
> +
> +	/* refill pending groups from s_dirty */
> +	spin_lock(&inode_lock);
> +	while (!list_empty(&sb->s_dirty)) {
> +		struct inode *inode = list_entry(sb->s_dirty.prev,
> +						struct inode, i_list);
> +		struct ext4_inode_info *ei = EXT4_I(inode);
> +
> +		lg = ei->i_locality_group;
> +		if (lg == NULL) {
> +			if (S_ISDIR(inode->i_mode) || i_size_read(inode) == 0) {
> +				if (atomic_read(&inode->i_count)) {
> +					/*
> +					 * The inode is clean, inuse
> +					 */
> +					list_move(&inode->i_list, &inode_in_use);
> +				} else {
> +					/*
> +					 * The inode is clean, unused
> +					 */
> +					list_move(&inode->i_list, &inode_unused);
> +				}
> +				continue;
> +			}
> +			/* XXX: atime changed ? */
> +			list_move(&inode->i_list, &inode_in_use);
> +			continue;
> +		}
> +
> +		/* move inode in proper locality group's dirty list */
> +		spin_lock(&lg->lg_lock);
> +		list_move_tail(&inode->i_list, &lg->lg_dirty);
> +		spin_unlock(&lg->lg_lock);
> +
> +		if (!test_and_set_bit(EXT4_LG_DIRTY, &lg->lg_flags))
> +			list_move(&lg->lg_list, &sbi->s_locality_dirty);
> +	}
> +	spin_unlock(&inode_lock);
> +
> +	ext4_lg_sync_groups(sb, wbc);
> +}
Hi Alex,
I did the following test:
# mkfs /dev/sdc1
# mount -t ext4dev -o extents,mballoc,delalloc /dev/sdc1 /test
# dumpe2fs -h /dev/sdc1
# cp linux.tar /test
# tar xf /test/linux.tar
# dumpe2fs -h /dev/sdc1

The "Free blocks" and "Free inodes" counters in the dumpe2fs output did 
not change after running the cp and tar commands.

I think it misses a call to ext4_commit_super() at the end of the 
ext4_lg_sync_inodes function.
Am I right ?

    Valérie