On 2009-11-06, at 15:33, Eric Sandeen wrote:
commit a71ce8c6c9bf269b192f352ea555217815cf027e updated
ext4_statfs() to update the on-disk superblock counters,
but modified this buffer directly without any journaling of
the change. This is one of the accesses that was causing the crc
errors in journal replay as seen in kernel.org bugzilla #14354.
Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
---
@@ -3650,20 +3652,33 @@ static int ext4_statfs(struct dentry
*dentry, struct kstatfs *buf)
+ handle = ext4_journal_start_sb(sb, 1);
+ if (IS_ERR(handle)) {
+ err = PTR_ERR(handle);
+ goto out;
+ }
+ err = ext4_journal_get_write_access(handle, EXT4_SB(sb)->s_sbh);
+ if (err)
+ goto journal_stop;
+ es->s_free_inodes_count = cpu_to_le32(buf->f_ffree);
+ ext4_free_blocks_count_set(es, buf->f_bfree);
+ err = ext4_handle_dirty_metadata(handle, NULL, EXT4_SB(sb)->s_sbh);
I admit to being the instigator of this change.
The intention is that we want to update the on-disk superblock block/
inode
counters from the per-cpu data periodically, since they are never
updated
anymore (only the group summaries are updated, to avoid contention).
However,
this isn't critical work, since it is only useful for read-only e2fsck
not
reporting spurious errors on the filesystem and dumpe2fs/debugfs
having some
chance at reporting a reasonable value for the filesystem space usage.
Starting a transaction as part of statfs is really counter-productive
to making
that code efficient, which was the whole point of the original patch
to remove
the per-call "overhead" calculation.
The intention was that the in-memory superblock would be updated
whenever statfs
is called (this doesn't cost anything, since we've already computed
the value
for statfs), and if the superblock is written to disk for some other
reason they
will go along for the ride.
If the choice is between adding a proper transaction here, or not
doing this at
all, I'd rather just not do it at all. Of course, I'd like to work
out some
kind of compromise, like only updating the superblock when there is
already a shadow BH that is being used to write to the journal, or
similar.
If there is a desire to keep a transaction here and update the
superblock
counters, it _definitely_ doesn't need to be done on every statfs, but
at most
once every 30 seconds or whatever.
Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
