Re: [Ocfs2-devel] [PATCH] ocfs2: fix missing reset j_num_trans for sync - Heming Zhao via Ocfs2-devel

From: Heming Zhao via Ocfs2-devel <ocfs2-devel@oss.oracle.com>
To: Joseph Qi <joseph.qi@linux.alibaba.com>
Cc: ocfs2-devel@oss.oracle.com
Subject: Re: [Ocfs2-devel] [PATCH] ocfs2: fix missing reset j_num_trans for sync
Date: Sun, 30 Apr 2023 11:03:00 +0800	[thread overview]
Message-ID: <087e8f12-f03c-d175-4200-962d73641c23@suse.com> (raw)
In-Reply-To: <46f6fa85-9001-2cb9-3972-327ad8ba33a0@suse.com>

On 4/22/23 4:55 PM, Heming Zhao via Ocfs2-devel wrote:
> Sorry, please pause this patch review.

I will send a new patch to supersede this one.

- Heming

> When I was investigating fstest generic failed case 347 361, I found
> the wake_up() action should move out the 'if()' area. The correct way
> is calling wake_up() unconditionally.
> 
> Thanks,
> Heming
> 
> On 4/21/23 4:36 PM, Heming Zhao wrote:
>> fstest generic case 266 272 281 trigger hanging issue when umount.
>>
>> I use 266 to describe the root cause.
>>
>> ```
>>   49 _dmerror_unmount
>>   50 _dmerror_mount
>>   51
>>   52 echo "Compare files"
>>   53 md5sum $testdir/file1 | _filter_scratch
>>   54 md5sum $testdir/file2 | _filter_scratch
>>   55
>>   56 echo "CoW and unmount"
>>   57 sync
>>   58 _dmerror_load_error_table
>>   59 urk=$($XFS_IO_PROG -f -c "pwrite -S 0x63 -b $bufsize 0 $filesize" \
>>   60     -c "fdatasync" $testdir/file2 2>&1)
>>   61 echo $urk >> $seqres.full
>>   62 echo "$urk" | grep -q "error" || _fail "pwrite did not fail"
>>   63
>>   64 echo "Clean up the mess"
>>   65 _dmerror_unmount
>> ```
>>
>> After line 49 50 umount & mount ocfs2 dev, this case run md5sum to
>> verify target file. line 57 run 'sync' before line 58 changes the dm
>> target from dm-linear to dm-error. this case is hanging at line 65.
>>
>> In 266, md5sum calls jbd2 trans pair: ocfs2_[start|commit]_trans to
>> do journal job. but there is only ->j_num_trans+1 in ocfs2_start_trans,
>> the ocfs2_commit_trans doesn't do reduction operation.
>>
>> call flow:
>> ```
>> [md5sum]
>>   vfs_read
>>    ocfs2_file_read_iter
>>     ocfs2_inode_lock_atime
>>      ocfs2_update_inode_atime
>>       + ocfs2_start_trans //atomic_inc j_num_trans
>>       + ...
>>       + ocfs2_commit_trans//no modify j_num_trans
>>
>> sync //no modify j_num_trans
>>
>> _dmerror_load_error_table //all write will return error after this line
>>
>> _dmerror_unmount //found j_num_trans is not zero, run commit thread
>>                 //but the underlying dev is dm-error, journaling IO
>>                 //failed all the time and keep going to retry.
>> ```
>>
>> *** How to fix ***
>>
>> kick commit thread in sync path, which can reset j_num_trans to 0.
>>
>> Signed-off-by: Heming Zhao <heming.zhao@suse.com>
>> ---
>>   fs/ocfs2/super.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
>> index 0b0e6a132101..9929320bd967 100644
>> --- a/fs/ocfs2/super.c
>> +++ b/fs/ocfs2/super.c
>> @@ -408,6 +408,9 @@ static int ocfs2_sync_fs(struct super_block *sb, int wait)
>>       if (jbd2_journal_start_commit(osb->journal->j_journal,
>>                         &target)) {
>> +        /* kick commit thread to reset journal->j_num_trans */
>> +        wake_up(&osb->checkpoint_event);
>> +
>>           if (wait)
>>               jbd2_log_wait_commit(osb->journal->j_journal,
>>                            target);
>>
> 
> 
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/ocfs2-devel

_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel