Re: [PATCH 33/49] ext4: Add the journal checksum feature

Girish Shilamkar
2008-02-01T20:42:49Z

Hi,

On Thu, 2008-01-24 at 13:24 -0800, Mingming Cao wrote:
> -static int journal_write_commit_record(journal_t *journal,
> - transaction_t *commit_transaction)
> +static int journal_submit_commit_record(journal_t *journal,
> + transaction_t *commit_transaction,
> + struct buffer_head **cbh,
> + __u32 crc32_sum)
> {
> struct journal_head *descriptor;
> + struct commit_header *tmp;
> struct buffer_head *bh;
> - int i, ret;
> + int ret;
> int barrier_done = 0;
>
> if (is_journal_aborted(journal))
> @@ -117,21 +122,33 @@ static int journal_write_commit_record(j
>
> bh = jh2bh(descriptor);
>
> - /* AKPM: buglet - add `i' to tmp! */
> - for (i = 0; i < bh->b_size; i += 512) {
> - journal_header_t *tmp = (journal_header_t*)bh->b_data;
> - tmp->h_magic = cpu_to_be32(JBD2_MAGIC_NUMBER);
> - tmp->h_blocktype = cpu_to_be32(JBD2_COMMIT_BLOCK);
> - tmp->h_sequence = cpu_to_be32(commit_transaction->t_tid);
> + tmp = (struct commit_header *)bh->b_data;
> + tmp->h_magic = cpu_to_be32(JBD2_MAGIC_NUMBER);
> + tmp->h_blocktype = cpu_to_be32(JBD2_COMMIT_BLOCK);
> + tmp->h_sequence = cpu_to_be32(commit_transaction->t_tid);
> +
> + if (JBD2_HAS_COMPAT_FEATURE(journal,
> + JBD2_FEATURE_COMPAT_CHECKSUM)) {
> + tmp->h_chksum_type = JBD2_CRC32_CHKSUM;
> + tmp->h_chksum_size = JBD2_CRC32_CHKSUM_SIZE;
> + tmp->h_chksum[0] = cpu_to_be32(crc32_sum);
> }
>
> - JBUFFER_TRACE(descriptor, "write commit block");
> + JBUFFER_TRACE(descriptor, "submit commit block");
> + lock_buffer(bh);
> +
get_bh() is missing here.
bh refcount is decremented in journal_wait_on_commit_record(), but it is
not incremented in journal_submit_commit_record().
Thanks to Johann Lombardi for pointing this out.

Comments.

> set_buffer_dirty(bh);
> - if (journal->j_flags & JBD2_BARRIER) {
> + set_buffer_uptodate(bh);
> + bh->b_end_io = journal_end_buffer_io_sync;
> +
> + if (journal->j_flags & JBD2_BARRIER &&
> + !JBD2_HAS_COMPAT_FEATURE(journal,
> + JBD2_FEATURE_INCOMPAT_ASYNC_COMMIT)) {
> set_buffer_ordered(bh);
> barrier_done = 1;
> }
> - ret = sync_dirty_buffer(bh);
> + ret = submit_bh(WRITE, bh);
> +
> /* is it possible for another commit to fail at roughly
> * the same time as this one? If so, we don't want to
> * trust the barrier flag in the super, but instead want
> @@ -152,14 +169,72 @@ static int journal_write_commit_record(j
> clear_buffer_ordered(bh);
> set_buffer_uptodate(bh);
> set_buffer_dirty(bh);
> - ret = sync_dirty_buffer(bh);
> + ret = submit_bh(WRITE, bh);
> }
> - put_bh(bh); /* One for getblk() */
> - jbd2_journal_put_journal_head(descriptor);
> + *cbh = bh;
> + return ret;
> +}
> +
> +/*
> + * This function along with journal_submit_commit_record
> + * allows to write the commit record asynchronously.
> + */
> +static int journal_wait_on_commit_record(struct buffer_head *bh)
> +{
> + int ret = 0;
> +
> + clear_buffer_dirty(bh);
> + wait_on_buffer(bh);
> +
> + if (unlikely(!buffer_uptodate(bh)))
> + ret = -EIO;
> + put_bh(bh); /* One for getblk() */
> + jbd2_journal_put_journal_head(bh2jh(bh));
>
> - return (ret == -EIO);
> + return ret;
> }
>
-Girish