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

spin lock contention in lock_res_and_lock

Details

    • Bug
    • Resolution: Fixed
    • Major
    • None
    • Lustre 2.4.1
    • lustre-2.4.0-26chaos
    • 3
    • 13209

    Description

      Our MDS experienced severe lock contention in lock_res_and_lock(). This had a large impact on client responsiveness because service threads were starved for CPU time. We have not yet identified the client workload that caused this problem. All active tasks had stack traces like this, but would eventually get scheduled out.

       ...
      __spin_lock
      lock_res_and_lock
      ldlm_handle_enqueue0
      mdt_handle_common
      mds_regular_handle
      ptlrpc_server_handle_request
      ...
      

      This raises the question of why the ldlm resource lock needs to be a spinlock. Couldn't we avoid this issue by converting it to a mutex? This question was raised in LU-3504.

      Attachments

        Issue Links

          Activity

            [LU-4801] spin lock contention in lock_res_and_lock

            Oleg Drokin (green@whamcloud.com) uploaded a new patch: https://review.whamcloud.com/38238
            Subject: Revert "LU-4801 ldlm: discard l_lock from struct ldlm_lock."
            Project: fs/lustre-release
            Branch: master
            Current Patch Set: 1
            Commit: 32095b5717954fa7260c9d6e369a208395bc39da

            gerrit Gerrit Updater added a comment - Oleg Drokin (green@whamcloud.com) uploaded a new patch: https://review.whamcloud.com/38238 Subject: Revert " LU-4801 ldlm: discard l_lock from struct ldlm_lock." Project: fs/lustre-release Branch: master Current Patch Set: 1 Commit: 32095b5717954fa7260c9d6e369a208395bc39da

            Oleg Drokin (green@whamcloud.com) merged in patch https://review.whamcloud.com/35483/
            Subject: LU-4801 ldlm: discard l_lock from struct ldlm_lock.
            Project: fs/lustre-release
            Branch: master
            Current Patch Set:
            Commit: 0584eb73dbb5b4c710a8c7eb1553ed5dad0c18d8

            gerrit Gerrit Updater added a comment - Oleg Drokin (green@whamcloud.com) merged in patch https://review.whamcloud.com/35483/ Subject: LU-4801 ldlm: discard l_lock from struct ldlm_lock. Project: fs/lustre-release Branch: master Current Patch Set: Commit: 0584eb73dbb5b4c710a8c7eb1553ed5dad0c18d8

            James Simmons (jsimmons@infradead.org) uploaded a new patch: https://review.whamcloud.com/35484
            Subject: LU-4801 ldlm: don't access l_resource when not locked.
            Project: fs/lustre-release
            Branch: master
            Current Patch Set: 1
            Commit: 5ea026bb3eb9020233569659189850519bc99a17

            gerrit Gerrit Updater added a comment - James Simmons (jsimmons@infradead.org) uploaded a new patch: https://review.whamcloud.com/35484 Subject: LU-4801 ldlm: don't access l_resource when not locked. Project: fs/lustre-release Branch: master Current Patch Set: 1 Commit: 5ea026bb3eb9020233569659189850519bc99a17

            James Simmons (jsimmons@infradead.org) uploaded a new patch: https://review.whamcloud.com/35483
            Subject: LU-4801 ldlm: discard l_lock from struct ldlm_lock.
            Project: fs/lustre-release
            Branch: master
            Current Patch Set: 1
            Commit: c7e16e62306abdb62f1039957573400c9114ea3f

            gerrit Gerrit Updater added a comment - James Simmons (jsimmons@infradead.org) uploaded a new patch: https://review.whamcloud.com/35483 Subject: LU-4801 ldlm: discard l_lock from struct ldlm_lock. Project: fs/lustre-release Branch: master Current Patch Set: 1 Commit: c7e16e62306abdb62f1039957573400c9114ea3f

            Neil pushed some patches to address. I will push

            simmonsja James A Simmons added a comment - Neil pushed some patches to address. I will push

            Oleg, it may not be right away, but we'll get this scheduled for testing.

            nedbass Ned Bass (Inactive) added a comment - Oleg, it may not be right away, but we'll get this scheduled for testing.
            green Oleg Drokin added a comment -

            Ned, I see.
            Well, I wonder if you can try my patch on your testbed to see if it hits similar overloaded issues right away by any chance?
            Cliff tried it with the patch and MDS was basically under no load during the test run, but he was then preempted by other important testing, so there was no test without the patch to actually ensure that the reproducer reproduces anything.
            So if you have time for that, it might be interesting exercise.

            green Oleg Drokin added a comment - Ned, I see. Well, I wonder if you can try my patch on your testbed to see if it hits similar overloaded issues right away by any chance? Cliff tried it with the patch and MDS was basically under no load during the test run, but he was then preempted by other important testing, so there was no test without the patch to actually ensure that the reproducer reproduces anything. So if you have time for that, it might be interesting exercise.

            Oleg, to clarify my comment regarding threads blocking in kmalloc(), I don't mean that they are doing so while holding a spinlock. My theory is that they spent so much time under the spin lock that they become eligible to reschedule. When they later call kmalloc() outside the spinlock it internally calls might_sleep() and reschedules the thread. Because there are other threads actively contending in lock_res_and_lock(), those blocked threads get starved for CPU time.

            nedbass Ned Bass (Inactive) added a comment - Oleg, to clarify my comment regarding threads blocking in kmalloc(), I don't mean that they are doing so while holding a spinlock. My theory is that they spent so much time under the spin lock that they become eligible to reschedule. When they later call kmalloc() outside the spinlock it internally calls might_sleep() and reschedules the thread. Because there are other threads actively contending in lock_res_and_lock(), those blocked threads get starved for CPU time.
            green Oleg Drokin added a comment -

            Attaching parallel_flock_v2.c - this is the same as before, only this version actually works as expected.

            green Oleg Drokin added a comment - Attaching parallel_flock_v2.c - this is the same as before, only this version actually works as expected.
            green Oleg Drokin added a comment -

            attached parallel_flock.c is my idea for a good reproducer of the flock issue.

            It takes three arguments: -f filename - filename on lustre fs to work on
            -n number of iterations
            -s how long for the first lock to be kept.

            To reproduce - run on a machine with a lot of clients using all available cores too.
            default sleep time is just 6 seconds so possbly you want more than that.
            Several iterations (default - 10).

            While running, I imagine MDS should grind to a total halt so that even userspace barely responds if at all.

            This is untested code, I just made sure it compiles.

            green Oleg Drokin added a comment - attached parallel_flock.c is my idea for a good reproducer of the flock issue. It takes three arguments: -f filename - filename on lustre fs to work on -n number of iterations -s how long for the first lock to be kept. To reproduce - run on a machine with a lot of clients using all available cores too. default sleep time is just 6 seconds so possbly you want more than that. Several iterations (default - 10). While running, I imagine MDS should grind to a total halt so that even userspace barely responds if at all. This is untested code, I just made sure it compiles.
            green Oleg Drokin added a comment -

            I guess an idea for a reproducer if it's just a long flock reprocessing issue is to have an multithreaded app run from a whole bunch of nodes where every thread would try to lock the same range (in blocking mode) in the same file to accumulate a multi-thousand list of blocked locks.

            Make whoever has the lock first to wait a minute or whatever amount of time is needed to ensure that all threads on all nodes has sent the flock requests and they all blocked.
            Then release the original lock, and after that every thread that has received a lock should release it immediately as well.

            green Oleg Drokin added a comment - I guess an idea for a reproducer if it's just a long flock reprocessing issue is to have an multithreaded app run from a whole bunch of nodes where every thread would try to lock the same range (in blocking mode) in the same file to accumulate a multi-thousand list of blocked locks. Make whoever has the lock first to wait a minute or whatever amount of time is needed to ensure that all threads on all nodes has sent the flock requests and they all blocked. Then release the original lock, and after that every thread that has received a lock should release it immediately as well.

            People

              green Oleg Drokin
              nedbass Ned Bass (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: