On 2021/6/4 0:28, Jan Kara wrote:
> On Thu 27-05-21 21:56:37, Zhang Yi wrote:
>> Now that __jbd2_journal_remove_checkpoint() can detect buffer io error
>> and mark journal checkpoint error, then we abort the journal later
>> before updating log tail to ensure the filesystem works consistently.
>> So we could remove other redundant buffer io error checkes.
>>
>> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
>> ---
>> fs/jbd2/checkpoint.c | 7 +------
>> 1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
>> index 2cbac0e3cff3..c1f746a5cc1a 100644
>> --- a/fs/jbd2/checkpoint.c
>> +++ b/fs/jbd2/checkpoint.c
>> @@ -91,8 +91,7 @@ static int __try_to_free_cp_buf(struct journal_head *jh)
>> int ret = 0;
>> struct buffer_head *bh = jh2bh(jh);
>>
>> - if (jh->b_transaction == NULL && !buffer_locked(bh) &&
>> - !buffer_dirty(bh) && !buffer_write_io_error(bh)) {
>> + if (!jh->b_transaction && !buffer_locked(bh) && !buffer_dirty(bh)) {
>> JBUFFER_TRACE(jh, "remove from checkpoint list");
>> ret = __jbd2_journal_remove_checkpoint(jh) + 1;
>> }
>> @@ -295,8 +294,6 @@ int jbd2_log_do_checkpoint(journal_t *journal)
>> goto restart;
>> }
>> if (!buffer_dirty(bh)) {
>> - if (unlikely(buffer_write_io_error(bh)) && !result)
>> - result = -EIO;
>> BUFFER_TRACE(bh, "remove from checkpoint");
>> if (__jbd2_journal_remove_checkpoint(jh))
>> /* The transaction was released; we're done */
>> @@ -356,8 +353,6 @@ int jbd2_log_do_checkpoint(journal_t *journal)
>> spin_lock(&journal->j_list_lock);
>> goto restart2;
>> }
>> - if (unlikely(buffer_write_io_error(bh)) && !result)
>> - result = -EIO;
>>
>> /*
>> * Now in whatever state the buffer currently is, we
>
> You can also drop:
>
> if (result < 0)
> jbd2_journal_abort(journal, result);
>
> in jbd2_log_do_checkpoint() as there's now nothing which can set 'result'
> in the loops... Otherwise looks good. Feel free to add:
>
Yes, I will remove it in the next iteration, thanks for the review.
Thanks,
Yi.
