On 06/18/2014 08:03 PM, Marc Dionne wrote:

quoted

On Wed, Jun 18, 2014 at 7:53 PM, Chris Mason[off-list ref]  wrote:

quoted

On 06/18/2014 07:30 PM, Waiman Long wrote:

quoted

On 06/18/2014 07:27 PM, Chris Mason wrote:

quoted

On 06/18/2014 07:19 PM, Waiman Long wrote:

quoted

On 06/18/2014 07:10 PM, Josef Bacik wrote:

quoted

On 06/18/2014 03:47 PM, Waiman Long wrote:

quoted

On 06/18/2014 06:27 PM, Josef Bacik wrote:

quoted

On 06/18/2014 03:17 PM, Waiman Long wrote:

quoted

On 06/18/2014 04:57 PM, Marc Dionne wrote:

quoted

Hi,

I've been seeing very reproducible soft lockups with 3.16-rc1
similar
to what is reported here:

https://urldefense.proofpoint.com/v1/url?u=http://marc.info/?l%3Dlinux-btrfs%26m%3D140290088532203%26w%3D2&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=cKCbChRKsMpTX8ybrSkonQ%3D%3D%0A&m=aoagvtZMwVb16gh1HApZZL00I7eP50GurBpuEo3l%2B5g%3D%0A&s=c62558feb60a480bbb52802093de8c97b5e1f23d4100265b6120c8065bd99565



, along with the
occasional hard lockup, making it impossible to complete a
parallel
build on a btrfs filesystem for the package I work on.  This was
working fine just a few days before rc1.

Bisecting brought me to the following commit:

     commit bd01ec1a13f9a327950c8e3080096446c7804753
     Author: Waiman Long[off-list ref]
     Date:   Mon Feb 3 13:18:57 2014 +0100

         x86, locking/rwlocks: Enable qrwlocks on x86

And sure enough if I revert that commit on top of current
mainline,
I'm unable to reproduce the soft lockups and hangs.

Marc

The queue rwlock is fair. As a result, recursive read_lock is not
allowed unless the task is in an interrupt context. Doing
recursive
read_lock will hang the process when a write_lock happens
somewhere in
between. Are recursive read_lock being done in the btrfs code?

We walk down a tree and read lock each node as we walk down, is
that
what you mean?  Or do you mean read_lock multiple times on the same
lock in the same process, cause we definitely don't do that.
Thanks,

Josef

I meant recursively read_lock the same lock in a process.

I take it back, we do actually do this in some cases.  Thanks,

Josef

This is what I thought when I looked at the looking code in btrfs. The
unlock code doesn't clear the lock_owner pid, this may cause the
lock_nested to be set incorrectly.

Anyway, are you going to do something about it?

Thanks for reporting this, we shouldn't be actually taking the lock
recursively.  Could you please try with lockdep enabled?  If the
problem
goes away with lockdep on, I think I know what's causing it.
Otherwise,
lockdep should clue us in.

-chris

I am not sure if lockdep will report recursive read_lock as this is
possible in the past. If not, we certainly need to add that capability
to it.

One more thing, I saw comment in btrfs tree locking code about taking a
read lock after taking a write (partial?) lock. That is not possible
with even with the old rwlock code.

With lockdep on, the clear_path_blocking function you're hitting
softlockups in is different.  Futjitsu hit a similar problem during
quota rescans, and it goes away with lockdep on.  I'm trying to nail
down where we went wrong, but please try lockdep on.

-chris

With lockdep on I'm unable to reproduce the lockups, and there are no
lockdep warnings.

Marc

Enabling lockdep may change the lock timing that make it hard to reproduce
the problem. Anyway, could you try to apply the following patch to see if it
shows any warning?

-Longman

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index d24e433..b6c9f2e 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c

@@ -1766,12 +1766,22 @@ check_deadlock(struct task_struct *curr, struct

held_loc
                if (hlock_class(prev) != hlock_class(next))
                        continue;

+#ifdef CONFIG_QUEUE_RWLOCK
+               /*
+                * Queue rwlock only allows read-after-read recursion of the
+                * same lock class when the latter read is in an interrupt
+                * context.
+                */
+               if ((read == 2) && prev->read && in_interrupt())
+                       return 2;
+#else
                /*
                 * Allow read-after-read recursion of the same
                 * lock class (i.e. read_lock(lock)+read_lock(lock)):
                 */
                if ((read == 2) && prev->read)
                        return 2;
+#endif

                /*
                 * We're holding the nest_lock, which serializes this lock's

@@ -1852,8 +1862,10 @@ check_prev_add(struct task_struct *curr, struct

held_lock
         * write-lock never takes any other locks, then the reads are
         * equivalent to a NOP.
         */
+#ifndef CONFIG_QUEUE_RWLOCK
        if (next->read == 2 || prev->read == 2)
                return 1;
+#endif
        /*
         * Is the <prev> -> <next> dependency already present?
         *