Re: [PATCH -v7][RFC]: mutex: implement adaptive spinning

[Steven Rostedt]

On Thu, 8 Jan 2009, Chris Mason wrote:

> On Thu, 2009-01-08 at 08:58 -0800, Linus Torvalds wrote:
> > 
> > Ok, I've gone through -v7, and I'm sure you're all shocked to hear it, but 
> > I have no complaints. Except that you dropped all the good commit 
> > commentary you had earlier ;)
> > 
> 
> Seems to get stuck under load.  I've hit it with make -j 50 on ext3 and
> with my btrfs benchmarking.  This was against the latest git from about
> 5 minutes ago.
> 
> -chris
> 
> BUG: soft lockup - CPU#3 stuck for 61s! [python:3970] CPU 3: Modules 
> linked in: netconsole configfs btrfs zlib_deflate loop e1000e 3w_9xxx 

> Pid: 3970, comm: python Not tainted 2.6.28 #1 Call Trace:

>  [<ffffffff8024f4b1>] ? __cmpxchg+0x9/0x3f
>  [<ffffffff805e0068>] ? __mutex_lock_common+0x3d/0x178

Hmm, looking at the code...

mutex.c:

       for (;;) {
                struct thread_info *owner;

                old_val = atomic_cmpxchg(&lock->count, 1, 0);
                if (old_val == 1) {
                        lock_acquired(&lock->dep_map, ip);
                        mutex_set_owner(lock);
                        return 0;
                }

                if (old_val < 0 && !list_empty(&lock->wait_list))
                        break;

                /* See who owns it, and spin on him if anybody */
                owner = ACCESS_ONCE(lock->owner);
                if (owner && !spin_on_owner(lock, owner))
                        break;

                cpu_relax();
        }


and in sched.c:

int spin_on_owner(struct mutex *lock, struct thread_info *owner)
{
        unsigned int cpu;
        struct rq *rq;
        int ret = 1;
[...]
                if (lock->owner != owner)
                        break;


We keep spinning if the owner changes.  I wonder if you have many CPUS 
(Chris, how many cpus did this box have?), you could get one task 
constantly spinning while the mutex keeps changing owners on the other 
CPUS.

Perhaps, we should do something like:

mutex.c:

       for (;;) {
                struct thread_info *owner = NULL;

                old_val = atomic_cmpxchg(&lock->count, 1, 0);
                if (old_val == 1) {
                        lock_acquired(&lock->dep_map, ip);
                        mutex_set_owner(lock);
                        return 0;
                }

                if (old_val < 0 && !list_empty(&lock->wait_list)) 
                        break;

                /* See who owns it, and spin on him if anybody */
		if (!owner)
	                owner = ACCESS_ONCE(lock->owner);
                if (owner && !spin_on_owner(lock, owner)) 
                        break;

                cpu_relax();
        }

Or just pull assigning of the owner out of the loop. This way, we go to 
sleep if the owner changes and is not NULL.

Just a thought,

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/