From patchwork Tue Mar 29 17:09:49 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tejun Heo X-Patchwork-Id: 671752 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.3) with ESMTP id p2THH1qX011692 for ; Tue, 29 Mar 2011 17:17:01 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753899Ab1C2RQd (ORCPT ); Tue, 29 Mar 2011 13:16:33 -0400 Received: from mail-fx0-f46.google.com ([209.85.161.46]:63375 "EHLO mail-fx0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750991Ab1C2RQc (ORCPT ); Tue, 29 Mar 2011 13:16:32 -0400 Received: by fxm17 with SMTP id 17so409258fxm.19 for ; Tue, 29 Mar 2011 10:16:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:sender:date:from:to:cc:subject:message-id :references:mime-version:content-type:content-disposition :in-reply-to:user-agent; bh=LiovDwfS0FgwLhfGFUUt8AkL8K3Ti04uDsfnKVv6djk=; b=xc4yDiUuTnfuCia1cw04+yHjacmCSWFUlgoVN6dNRHcHrN5jmk3juFlioyu2OmCdCm lNxlIOx5fiO9mu95XSmwjjHt+mNM6EVKKwnzT6N6pTN8AAo42XtctkCUD9mmdLefgyBh s4iA0lTqPCAYbN9SVGdlgp7oNatcjiGw6n1cU= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; b=SKheVs1UJtA3JGrc4aCgjE6BDya+dnIUEWuD0EQ7syF5NGcQ8d8Lq52nnMNr9RSjBJ wvelWHDgryHXKpSuizEnHWEBgwdBiQ2vDFWnJWwg94OcWtLnZe7obd8pHx0EiQJBBB15 qXhu7X8l2zQsUi2nbEyKNtk8WWB/CncKxF9cY= Received: by 10.223.100.6 with SMTP id w6mr42413fan.39.1301418593434; Tue, 29 Mar 2011 10:09:53 -0700 (PDT) Received: from htj.dyndns.org ([130.75.117.88]) by mx.google.com with ESMTPS id n2sm2084523fam.4.2011.03.29.10.09.50 (version=SSLv3 cipher=OTHER); Tue, 29 Mar 2011 10:09:51 -0700 (PDT) Date: Tue, 29 Mar 2011 19:09:49 +0200 From: Tejun Heo To: Peter Zijlstra , Ingo Molnar , Linus Torvalds , Andrew Morton , Chris Mason Cc: linux-kernel@vger.kernel.org, linux-btrfs@vger.kernel.org Subject: Re: [PATCH 2/2] mutex: Apply adaptive spinning on mutex_trylock() Message-ID: <20110329170949.GF29865@htj.dyndns.org> References: <20110323153727.GB12003@htj.dyndns.org> <20110324094119.GD12038@htj.dyndns.org> <20110324094151.GE12038@htj.dyndns.org> <20110325101238.GC1409@htj.dyndns.org> <20110329163702.GE29865@htj.dyndns.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20110329163702.GE29865@htj.dyndns.org> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Tue, 29 Mar 2011 17:17:02 +0000 (UTC) Index: work1/include/linux/sched.h =================================================================== --- work1.orig/include/linux/sched.h +++ work1/include/linux/sched.h @@ -359,7 +359,7 @@ extern signed long schedule_timeout_inte extern signed long schedule_timeout_killable(signed long timeout); extern signed long schedule_timeout_uninterruptible(signed long timeout); asmlinkage void schedule(void); -extern int mutex_spin_on_owner(struct mutex *lock, struct thread_info *owner); +extern bool mutex_spin_on_owner(struct mutex *lock, struct thread_info *owner); struct nsproxy; struct user_namespace; Index: work1/kernel/sched.c =================================================================== --- work1.orig/kernel/sched.c +++ work1/kernel/sched.c @@ -536,6 +536,10 @@ struct rq { struct hrtimer hrtick_timer; #endif +#ifdef CONFIG_MUTEX_SPIN_ON_OWNER + bool spinning_on_mutex; +#endif + #ifdef CONFIG_SCHEDSTATS /* latency stats */ struct sched_info rq_sched_info; @@ -4021,16 +4025,44 @@ EXPORT_SYMBOL(schedule); #ifdef CONFIG_MUTEX_SPIN_ON_OWNER /* - * Look out! "owner" is an entirely speculative pointer - * access and not reliable. + * Maximum mutex owner spin duration in nsecs. Don't spin more then + * DEF_TIMESLICE. + */ +#define MAX_MUTEX_SPIN_NS (DEF_TIMESLICE * 1000000000LLU / HZ) + +/** + * mutex_spin_on_owner - optimistic adaptive spinning on locked mutex + * @lock: the mutex to spin on + * @owner: the current owner (speculative pointer) + * + * The caller is trying to acquire @lock held by @owner. If @owner is + * currently running, it might get unlocked soon and spinning on it can + * save the overhead of sleeping and waking up. + * + * Note that @owner is completely speculative and may be completely + * invalid. It should be accessed very carefully. + * + * Forward progress is guaranteed regardless of locking ordering by never + * spinning longer than MAX_MUTEX_SPIN_NS. This is necessary because + * mutex_trylock(), which doesn't have to follow the usual locking + * ordering, also uses this function. + * + * CONTEXT: + * Preemption disabled. + * + * RETURNS: + * %true if the lock was released and the caller should retry locking. + * %false if the caller better go sleeping. */ -int mutex_spin_on_owner(struct mutex *lock, struct thread_info *owner) +bool mutex_spin_on_owner(struct mutex *lock, struct thread_info *owner) { + unsigned long start; unsigned int cpu; struct rq *rq; + bool ret = true; if (!sched_feat(OWNER_SPIN)) - return 0; + return false; #ifdef CONFIG_DEBUG_PAGEALLOC /* @@ -4039,7 +4071,7 @@ int mutex_spin_on_owner(struct mutex *lo * the mutex owner just released it and exited. */ if (probe_kernel_address(&owner->cpu, cpu)) - return 0; + return false; #else cpu = owner->cpu; #endif @@ -4049,15 +4081,17 @@ int mutex_spin_on_owner(struct mutex *lo * the cpu field may no longer be valid. */ if (cpu >= nr_cpumask_bits) - return 0; + return false; /* * We need to validate that we can do a * get_cpu() and that we have the percpu area. */ if (!cpu_online(cpu)) - return 0; + return false; + this_rq()->spinning_on_mutex = true; + start = local_clock(); rq = cpu_rq(cpu); for (;;) { @@ -4070,21 +4104,30 @@ int mutex_spin_on_owner(struct mutex *lo * we likely have heavy contention. Return 0 to quit * optimistic spinning and not contend further: */ - if (lock->owner) - return 0; + ret = !lock->owner; break; } /* - * Is that owner really running on that cpu? + * Quit spinning if any of the followings is true. + * + * - The owner isn't running on that cpu. + * - The owner also is spinning on a mutex. + * - Someone else wants to use this cpu. + * - We've been spinning for too long. */ - if (task_thread_info(rq->curr) != owner || need_resched()) - return 0; + if (task_thread_info(rq->curr) != owner || + rq->spinning_on_mutex || need_resched() || + local_clock() > start + MAX_MUTEX_SPIN_NS) { + ret = false; + break; + } arch_mutex_cpu_relax(); } - return 1; + this_rq()->spinning_on_mutex = false; + return ret; } #endif