Uploaded image for project: 'Lustre'
  1. Lustre
  2. LU-4135

mdt_save_lock() is broken

Details

    • Bug
    • Resolution: Fixed
    • Blocker
    • Lustre 2.6.0, Lustre 2.5.1
    • Lustre 2.5.0
    • None
    • 3
    • 11218

    Description

      The mdt_save_lock() is broken and doesn't save any lock ever but simply unlock it. That happens because mti_has_trans is always 0 and is not updated upon transaction execution since commit 607905a789357a34166f34e7c992b03f5040eafc.

      Another issue with mdt_save_lock is 'req' variable which can be NULL in codepath mdt_export_cleanup()>mdt_ctxt_add_dirty_flag>mdt_add_dirty_flag->mdt_object_unlock()->mdt_save_lock().

      Attachments

        Issue Links

          duplicates

          Bug - A problem which impairs or prevents the functions of the product. LU-4103 interop 2.5/2.6 replay-dual test_21a: Input/output error

          • Minor - Minor loss of function, or other problem where easy workaround is present.
          • Resolved

          Bug - A problem which impairs or prevents the functions of the product. LU-4143 MDS OOPS sanity-hsm/test_52 NULL Pointer deref: mdt_save_lock

          • Critical - Crashes, loss of data, severe memory leak.
          • Closed

          Bug - A problem which impairs or prevents the functions of the product. LU-4142 MDS OOPS sanity-hsm/test_52 NULL Pointer deref: mdt_save_lock

          • Minor - Minor loss of function, or other problem where easy workaround is present.
          • Closed
          is related to

          Bug - A problem which impairs or prevents the functions of the product. LU-4143 MDS OOPS sanity-hsm/test_52 NULL Pointer deref: mdt_save_lock

          • Critical - Crashes, loss of data, severe memory leak.
          • Closed

          Activity

            [LU-4135] mdt_save_lock() is broken
            adilger Andreas Dilger added a comment -

            Patch was also merged to b2_5 and will be in the 2.5.1 release.

            adilger Andreas Dilger added a comment - Patch was also merged to b2_5 and will be in the 2.5.1 release.
            tappro Mikhail Pershin added a comment -

            patch was merged

            tappro Mikhail Pershin added a comment - patch was merged
            tappro Mikhail Pershin added a comment - - edited

            I've found that during testing side patches for Unified Target but it is clear that req is taken from mdt_thread_info and it is NULL in case of mdt_export_cleanup() in master as well, so I created this bug. Meanwhile I wonder why we don't see that issue in master tests and find out that things are even worse, currently in master mdt_save_lock() never saves them but just do unlock because mti_has_trans is always 0 after commit 607905a789357a34166f34e7c992b03f5040eafc. In my patches for UT it works and bug happens.

            This issue is quite critical now because we broke important part of recovery so I'd change summary to something like "restore mdt_lock_save() functionality". Patch is refreshed already to fix both issues.

            With proper mdt_save_lock() functionality the oops happens in test 52 sanity-hsm.sh due to NULL req variable as I wrote in first comment.

            tappro Mikhail Pershin added a comment - - edited I've found that during testing side patches for Unified Target but it is clear that req is taken from mdt_thread_info and it is NULL in case of mdt_export_cleanup() in master as well, so I created this bug. Meanwhile I wonder why we don't see that issue in master tests and find out that things are even worse, currently in master mdt_save_lock() never saves them but just do unlock because mti_has_trans is always 0 after commit 607905a789357a34166f34e7c992b03f5040eafc. In my patches for UT it works and bug happens. This issue is quite critical now because we broke important part of recovery so I'd change summary to something like "restore mdt_lock_save() functionality". Patch is refreshed already to fix both issues. With proper mdt_save_lock() functionality the oops happens in test 52 sanity-hsm.sh due to NULL req variable as I wrote in first comment.
            green Oleg Drokin added a comment -

            So how does this manifests itself? a crash in a specific circumstances or what?
            Also how did you find it and when will this hit in real world?

            green Oleg Drokin added a comment - So how does this manifests itself? a crash in a specific circumstances or what? Also how did you find it and when will this hit in real world?
            tappro Mikhail Pershin added a comment -

            patch to fix this issue: http://review.whamcloud.com/8048

            tappro Mikhail Pershin added a comment - patch to fix this issue: http://review.whamcloud.com/8048

            People

              tappro Mikhail Pershin
              tappro Mikhail Pershin
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: