Re: [PATCH v3 6/8] ext4: move escape handle to futher improve jbd2_journal_write_metadata_buffer

[Kemeng Shi]

on 7/31/2024 8:17 PM, Zhang Yi wrote:
> The title: "ext4: move escape handle..." should be "jbd2: move escape handle..."
> 
Sorry for this, will fix it in next version. Thanks!
> Thanks,
> Yi.
> 
> On 2024/7/31 17:29, Kemeng Shi wrote:
>> Move escape handle to futher improve code readability and remove some
>> repeat check.
>>
>> Signed-off-by: Kemeng Shi <shikemeng@xxxxxxxxxxxxxxx>
>> ---
>>  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;
>>
> 
>