[Ocfs2-devel] discuss about jbd2 assertion in defragment path

[Joseph Qi]

Hi,

On 2/14/23 12:33 PM, Heming Zhao wrote:
> On Tue, Feb 14, 2023 at 10:52:30AM +0800, Joseph Qi wrote:
>> Hi, Sorry about the late reply.
>> This thread is indeed a long time ago:(
>> It seems that I said the two ocfs2_journal_access_di() are for different
>> buffer head. Anyway, I have to recall the discussion before and get back
>> to you.
>>
> If you belive from [a] to [b.1] doesn't need any journal protection, my patch
> makes sense from theory.
> 
> From code logic, if the defrag path (ocfs2_split_extent) doesn't call
> jbd2_journal_restart(), current code is absolute correct. Increasing credits
> can't help avoid jbd2 assert crash, but make more easily to trigger crash,
> because it makes jbd2_journal_extend() more easily to return "status > 0". In my
> view, the correct fix should delete the access/dirty pair and make sure the
> ocfs2_split_extent() is journal self-service.
> 
> Different buffer head issue could be handled by rechecking ocfs2_split_extent(),
> make sure all journal handle branches are correct. I already checked
> ocfs2_split_extent, but I can't assure my eyes didn't miss any corner.

ocfs2_split_extent are not just 'read' operations, it may grow the tree,
that's why we have to do a transaction here.

The whole code flow:
ocfs2_ioctl_move_extents
  ocfs2_move_extents
    __ocfs2_move_extents_range
      ocfs2_defrag_extent  [1]
        ocfs2_start_trans  [a]
        __ocfs2_move_extent
          ocfs2_journal_access_di
          ocfs2_split_extent
          ocfs2_journal_dirty
        ocfs2_commit_trans
      ocfs2_move_extent  [2]
        ocfs2_start_trans
        __ocfs2_move_extent
          ocfs2_journal_access_di
          ocfs2_split_extent
          ocfs2_journal_dirty
        ocfs2_commit_trans
    ocfs2_start_trans  [b]
    ocfs2_journal_access_di
    ocfs2_journal_dirty
    ocfs2_commit_trans

In above, [a] and [b] are different transaction start/commit pair, and
each allocates different journal credits, even they are operate the same
bh, they are still different operations.

My another question is, __ocfs2_move_extent will be called by both
ocfs2_defrag_extent and ocfs2_move_extent, why this doesn't happen on the
other path?

Thanks,
Joseph