linux-kernel - Re: [PATCH 2/2] rwsem: do optimistic spinning for writer lock acquisition

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1372111092.22432.84.camel@schen9-DESK>
Date:	Mon, 24 Jun 2013 14:58:12 -0700
From:	Tim Chen <tim.c.chen@...ux.intel.com>
To:	Peter Hurley <peter@...leysoftware.com>
Cc:	Alex Shi <alex.shi@...el.com>,
	Michel Lespinasse <walken@...gle.com>,
	Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Andi Kleen <andi@...stfloor.org>,
	Davidlohr Bueso <davidlohr.bueso@...com>,
	Matthew R Wilcox <matthew.r.wilcox@...el.com>,
	Dave Hansen <dave.hansen@...el.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Rik van Riel <riel@...hat.com>, linux-kernel@...r.kernel.org,
	linux-mm <linux-mm@...ck.org>
Subject: Re: [PATCH 2/2] rwsem: do optimistic spinning for writer lock
 acquisition

On Sat, 2013-06-22 at 03:57 -0400, Peter Hurley wrote:
> Will this spin for full scheduler value on a reader-owned lock?
> 
> > +		/* wait_lock will be acquired if write_lock is obtained */
> > +		if (rwsem_try_write_lock(sem->count, true, sem)) {
> > +			ret = 1;
> > +			goto out;
> > +		}
> > +
> > +		/*
> > +		 * When there's no owner, we might have preempted between the
>                                                          ^^^^^^^^
> 
> Isn't pre-emption disabled?
> 

Peter, on further review, this code is needed.  This code guard against 
the case of this thread preempting another thread in the middle
of setting the  owner field.  Disabling preemption does not prevent this
thread from preempting others, even though others cannot preempt 
this thread.


> 
> > +		 * owner acquiring the lock and setting the owner field. If
> > +		 * we're an RT task that will live-lock because we won't let
> > +		 * the owner complete.
> > +		 */
> > +		if (!owner && (need_resched() || rt_task(current)))
> > +			break;
> > +
> > +		/*
> > +		 * The cpu_relax() call is a compiler barrier which forces
> > +		 * everything in this loop to be re-loaded. We don't need
> > +		 * memory barriers as we'll eventually observe the right
> > +		 * values at the cost of a few extra spins.
> > +		 */
> > +		arch_mutex_cpu_relax();
> > +
> > +	}
> > +
> > +out:
> > +	preempt_enable();
> > +	return ret;
> > +}
> > +#endif
> > +
> >   /*
> >    * wait until we successfully acquire the write lock
> >    */
> > @@ -200,6 +326,9 @@ struct rw_semaphore __sched *rwsem_down_write_failed(struct rw_semaphore *sem)
> >   	long count, adjustment = -RWSEM_ACTIVE_WRITE_BIAS;
> >   	struct rwsem_waiter waiter;
> >   	struct task_struct *tsk = current;
> > +#ifdef CONFIG_RWSEM_SPIN_ON_WRITE_OWNER
> > +	bool try_optimistic_spin = true;
> > +#endif
> >
> >   	/* set up my own style of waitqueue */
> >   	waiter.task = tsk;
> > @@ -223,20 +352,17 @@ struct rw_semaphore __sched *rwsem_down_write_failed(struct rw_semaphore *sem)
> >   	/* wait until we successfully acquire the lock */
> >   	set_task_state(tsk, TASK_UNINTERRUPTIBLE);
> >   	while (true) {
> > -		if (!(count & RWSEM_ACTIVE_MASK)) {
> > -			/* Try acquiring the write lock. */
> > -			count = RWSEM_ACTIVE_WRITE_BIAS;
> > -			if (!list_is_singular(&sem->wait_list))
> > -				count += RWSEM_WAITING_BIAS;
> > -
> > -			if (sem->count == RWSEM_WAITING_BIAS &&
> > -			    cmpxchg(&sem->count, RWSEM_WAITING_BIAS, count) ==
> > -							RWSEM_WAITING_BIAS)
> > -				break;
> > -		}
> > +		if (rwsem_try_write_lock(count, false, sem))
> > +			break;
> >
> >   		raw_spin_unlock_irq(&sem->wait_lock);
> >
> > +#ifdef CONFIG_RWSEM_SPIN_ON_WRITE_OWNER
> > +		/* do optimistic spinning */
> > +		if (try_optimistic_spin && rwsem_optimistic_spin(sem))
> > +			break;
> > +		try_optimistic_spin = false;
> > +#endif
> >   		/* Block until there are no active lockers. */
> >   		do {
> >   			schedule();
> 

Thanks.

Tim

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