On Sat, Jan 8, 2011 at 12:12 AM, Amir Goldstein [off-list ref] wrote:

On Fri, Jan 7, 2011 at 11:07 PM, Amir Goldstein [off-list ref] wrote:

quoted

On Fri, Jan 7, 2011 at 9:41 PM, Amir Goldstein [off-list ref] wrote:

quoted

On Fri, Jan 7, 2011 at 7:26 AM, Ted Ts'o [off-list ref] wrote:

quoted

Am I missing something?  The kernel stores up to 3k worth of data, on
a 4k block file system.  Whereas e2fsck patch blindly assume 2k worhth
of data regardless of the block size.  The kernel patch looks ok, but
the e2fsprogs patch seems badly broken....

So it's not badly broken, it copies blocksize-2K, which is clumsily
written like this:
+       int len = ctx->fs->blocksize - 2*SUPERBLOCK_OFFSET;

After that, only 4K block and 8K block will have a leftover,
which will be copied from journal sb+1K to ext4 sb+1K.
Yes, 2K blocks - no message buffer for you!

The reason I am only copying 2K and throwing the extra K is this:
print_message_buffer() is called from check_super_block(),
*after* journal recovery, which was executed either by e2fsck itself
or (and more likely in a errors=remount-ro situation) by ext4
on read-only mount.
In the latter case, the extra K must be discarded, so I saw no reason
to write special code for the first case.
Neither did I find a good reason to complicate the recording code
and limit it to record only blocksize-2K.

Ted,

I have a suggestion how to use the wasted extra K.

As I pointed out in the past, the first/last_error_xxx statistics are most
likely to be lost in errors=panic and errors=remount-ro (journal
recovery will override super block)
If you record this information in the last K of journal sb (even copy
the entire ext4 sb),
you can then override ext4 sb with the most up-to-date error stats
after journal recovery.

Amir.

Ted,

I just realize you did implement save&update of super block s_error_xxx fields.
However, I wonder if it is not a bug to call ext4_commit_super() from
save_error_info()
to commit the s_error_xxx fields in the first place.

The super block buffer is most likely participating in the current
transaction and should
not be committed to disk.

The alleged bug is likely to be hidden by the fact that the super
block has most likely
participated also in the last committed transaction, so a valid
version of it will most
likely override the invalid version.

I can imagine there could be a corner case, though, when committing
the super block
a head of transaction commit will result in inconsistencies.

Am I missing something?

Amir.
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html