Re: [PATCH 2/4] jbd: ordered data integrity fix (rebased)

[Jan Kara <jack@suse.cz>]

On Wed 14-05-08 13:48:43, Hidehiro Kawai wrote:
> Subject: [PATCH 2/4] jbd: ordered data integrity fix
> 
> In ordered mode, if a buffer being dirtied exists in the committing
> transaction, we write the buffer to the disk, move it from the
> committing transaction to the running transaction, then dirty it.
> But we don't have to remove the buffer from the committing
> transaction when the buffer couldn't be written out, otherwise it
> breaks the ordered mode rule.
  Hmm, could you elaborate a bit more what exactly is broken and how does
this help to fix it? Because even if we find EIO happened on data buffer,
we currently don't do anything else than just remove the buffer from the
transaction and abort the journal. And even if we later managed to write
the data buffer from other process before the journal is aborted, ordered
mode guarantees are satisfied - we only guarantee that too old data cannot
be seen, newer can be seen easily... Thanks.

								Honza

> 
> Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
> ---
>  fs/jbd/transaction.c |   16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6.26-rc2/fs/jbd/transaction.c
> ===================================================================
> --- linux-2.6.26-rc2.orig/fs/jbd/transaction.c
> +++ linux-2.6.26-rc2/fs/jbd/transaction.c
> @@ -954,9 +954,10 @@ int journal_dirty_data(handle_t *handle,
>  	journal_t *journal = handle->h_transaction->t_journal;
>  	int need_brelse = 0;
>  	struct journal_head *jh;
> +	int ret = 0;
>  
>  	if (is_handle_aborted(handle))
> -		return 0;
> +		return ret;
>  
>  	jh = journal_add_journal_head(bh);
>  	JBUFFER_TRACE(jh, "entry");
> @@ -1067,7 +1068,16 @@ int journal_dirty_data(handle_t *handle,
>  				   time if it is redirtied */
>  			}
>  
> -			/* journal_clean_data_list() may have got there first */
> +			/*
> +			 * We shouldn't remove the buffer from the committing
> +			 * transaction if it has failed to be written.
> +			 * Otherwise, it breaks the ordered mode rule.
> +			 */
> +			if (unlikely(!buffer_uptodate(bh))) {
> +				ret = -EIO;
> +				goto no_journal;
> +			}
> +
>  			if (jh->b_transaction != NULL) {
>  				JBUFFER_TRACE(jh, "unfile from commit");
>  				__journal_temp_unlink_buffer(jh);
> @@ -1108,7 +1118,7 @@ no_journal:
>  	}
>  	JBUFFER_TRACE(jh, "exit");
>  	journal_put_journal_head(jh);
> -	return 0;
> +	return ret;
>  }
>  
>  /**
> 
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR