On Wed 31-07-24 14:22:45, Kemeng Shi wrote:
> Move escape handle to futher improve code readability and remove some
> repeat check.
>
> Signed-off-by: Kemeng Shi <shikemeng@xxxxxxxxxxxxxxx>
> Suggested-by: Jan Kara <jack@xxxxxxx>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> fs/jbd2/journal.c | 49 +++++++++++++++++++++++------------------------
> 1 file changed, 24 insertions(+), 25 deletions(-)
>
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index f17d05bc61df..85273fb1accb 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -281,6 +281,16 @@ static void journal_kill_thread(journal_t *journal)
> write_unlock(&journal->j_state_lock);
> }
>
> +static inline bool jbd2_data_needs_escaping(char *data)
> +{
> + return *((__be32 *)data) == cpu_to_be32(JBD2_MAGIC_NUMBER);
> +}
> +
> +static inline void jbd2_data_do_escape(char *data)
> +{
> + *((unsigned int *)data) = 0;
> +}
> +
> /*
> * jbd2_journal_write_metadata_buffer: write a metadata buffer to the journal.
> *
> @@ -319,7 +329,6 @@ int jbd2_journal_write_metadata_buffer(transaction_t *transaction,
> sector_t blocknr)
> {
> int do_escape = 0;
> - char *mapped_data;
> struct buffer_head *new_bh;
> struct folio *new_folio;
> unsigned int new_offset;
> @@ -350,8 +359,13 @@ int jbd2_journal_write_metadata_buffer(transaction_t *transaction,
> if (jh_in->b_frozen_data) {
> new_folio = virt_to_folio(jh_in->b_frozen_data);
> new_offset = offset_in_folio(new_folio, jh_in->b_frozen_data);
> - mapped_data = jh_in->b_frozen_data;
> + do_escape = jbd2_data_needs_escaping(jh_in->b_frozen_data);
> + if (do_escape)
> + jbd2_data_do_escape(jh_in->b_frozen_data);
> } else {
> + char *tmp;
> + char *mapped_data;
> +
> new_folio = bh_in->b_folio;
> new_offset = offset_in_folio(new_folio, bh_in->b_data);
> mapped_data = kmap_local_folio(new_folio, new_offset);
> @@ -363,21 +377,13 @@ int jbd2_journal_write_metadata_buffer(transaction_t *transaction,
> */
> jbd2_buffer_frozen_trigger(jh_in, mapped_data,
> jh_in->b_triggers);
> - }
> -
> - /*
> - * Check for escaping
> - */
> - if (*((__be32 *)mapped_data) == cpu_to_be32(JBD2_MAGIC_NUMBER))
> - do_escape = 1;
> - if (!jh_in->b_frozen_data)
> + do_escape = jbd2_data_needs_escaping(mapped_data);
> kunmap_local(mapped_data);
> -
> - /*
> - * Do we need to do a data copy?
> - */
> - if (do_escape && !jh_in->b_frozen_data) {
> - char *tmp;
> + /*
> + * Do we need to do a data copy?
> + */
> + if (!do_escape)
> + goto escape_done;
>
> spin_unlock(&jh_in->b_state_lock);
> tmp = jbd2_alloc(bh_in->b_size, GFP_NOFS);
> @@ -404,17 +410,10 @@ int jbd2_journal_write_metadata_buffer(transaction_t *transaction,
> copy_done:
> new_folio = virt_to_folio(jh_in->b_frozen_data);
> new_offset = offset_in_folio(new_folio, jh_in->b_frozen_data);
> + jbd2_data_do_escape(jh_in->b_frozen_data);
> }
>
> - /*
> - * Did we need to do an escaping? Now we've done all the
> - * copying, we can finally do so.
> - * b_frozen_data is from jbd2_alloc() which always provides an
> - * address from the direct kernels mapping.
> - */
> - if (do_escape)
> - *((unsigned int *)jh_in->b_frozen_data) = 0;
> -
> +escape_done:
> folio_set_bh(new_bh, new_folio, new_offset);
> new_bh->b_size = bh_in->b_size;
> new_bh->b_bdev = journal->j_dev;
> --
> 2.30.0
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
