[Ocfs2-devel] [PATCH] ocfs2: clear journal dirty flag after shutdown journal

[Junxiao Bi]

On 11/19/18 10:28 AM, jiangyiwen wrote:
> On 2018/11/16 16:58, Junxiao Bi wrote:
>> Dirty flag of the journal should be cleared at the last stage of umount,
>> if do it before jbd2_journal_destroy(), then some metadata in uncommitted
>> transaction could be lost due to io error, but as dirty flag of journal
>> was already cleared, we can't find that until run a full fsck. This may
>> cause system panic or other corruption.
>>
>> Signed-off-by: Junxiao Bi <junxiao.bi at oracle.com>
>> ---
>>   fs/ocfs2/journal.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
>> index b63c97f4318e..25d678c92fbb 100644
>> --- a/fs/ocfs2/journal.c
>> +++ b/fs/ocfs2/journal.c
>> @@ -1017,6 +1017,10 @@ void ocfs2_journal_shutdown(struct ocfs2_super *osb)
>>   			mlog_errno(status);
>>   	}
>>   
>> +	/* Shutdown the kernel journal system */
>> +	jbd2_journal_destroy(journal->j_journal);
>> +	journal->j_journal = NULL;
>> +
> Hi Junxiao,
>
> I feel this adjustment doesn't make any sense. When jbd2_journal_destroy() is
> done, it still call ocfs2_journal_toggle_dirty() to clean dirty flag. Am I
> wrong or understand error ?

Oh, missed checking the return value of jbd2_journal_destroy(), if 
failed, should not call ocfs2_journal_toggle_dirty(). Thanks for 
pointing this.

Thanks,

Junxiao.

>
> Thanks.
> Yiwen.
>
>>   	if (status == 0) {
>>   		/*
>>   		 * Do not toggle if flush was unsuccessful otherwise
>> @@ -1027,10 +1031,6 @@ void ocfs2_journal_shutdown(struct ocfs2_super *osb)
>>   			mlog_errno(status);
>>   	}
>>   
>> -	/* Shutdown the kernel journal system */
>> -	jbd2_journal_destroy(journal->j_journal);
>> -	journal->j_journal = NULL;
>> -
>>   	OCFS2_I(inode)->ip_open_count--;
>>   
>>   	/* unlock our journal */
>>
>