From patchwork Thu Aug 11 21:54:44 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Zijlstra X-Patchwork-Id: 9276079 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id E8D146022E for ; Thu, 11 Aug 2016 21:54:57 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id DC2AE287AC for ; Thu, 11 Aug 2016 21:54:57 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id CF950287B9; Thu, 11 Aug 2016 21:54:57 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from oss.sgi.com (oss.sgi.com [192.48.182.195]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 971D2287AC for ; Thu, 11 Aug 2016 21:54:56 +0000 (UTC) Received: from oss.sgi.com (localhost [IPv6:::1]) by oss.sgi.com (Postfix) with ESMTP id 7BF6F7CA2; Thu, 11 Aug 2016 16:54:54 -0500 (CDT) X-Original-To: xfs@oss.sgi.com Delivered-To: xfs@oss.sgi.com Received: from relay.sgi.com (relay1.corp.sgi.com [137.38.102.111]) by oss.sgi.com (Postfix) with ESMTP id 500F47CA1 for ; Thu, 11 Aug 2016 16:54:52 -0500 (CDT) Received: from cuda.sgi.com (cuda1.sgi.com [192.48.157.11]) by relay1.corp.sgi.com (Postfix) with ESMTP id 069778F8040 for ; Thu, 11 Aug 2016 14:54:51 -0700 (PDT) X-ASG-Debug-ID: 1470952486-0bf8151be1b6090001-NocioJ Received: from bombadil.infradead.org ([198.137.202.9]) by cuda.sgi.com with ESMTP id TtlArhS0AknXIQC5 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO) for ; Thu, 11 Aug 2016 14:54:46 -0700 (PDT) X-Barracuda-Envelope-From: peterz@infradead.org X-Barracuda-Effective-Source-IP: UNKNOWN[198.137.202.9] X-Barracuda-Apparent-Source-IP: 198.137.202.9 Received: from j217100.upc-j.chello.nl ([24.132.217.100] helo=twins.programming.kicks-ass.net) by bombadil.infradead.org with esmtpsa (Exim 4.85_2 #1 (Red Hat Linux)) id 1bXxw5-0005ny-IC; Thu, 11 Aug 2016 21:54:45 +0000 Received: by twins.programming.kicks-ass.net (Postfix, from userid 1000) id 8B50F1256AAA6; Thu, 11 Aug 2016 23:54:44 +0200 (CEST) Date: Thu, 11 Aug 2016 23:54:44 +0200 From: Peter Zijlstra To: Christoph Hellwig Subject: Re: [PATCH, RFC] xfs: remove i_iolock and use i_rwsem in the VFS inode instead Message-ID: <20160811215444.GY30192@twins.programming.kicks-ass.net> X-ASG-Orig-Subj: Re: [PATCH, RFC] xfs: remove i_iolock and use i_rwsem in the VFS inode instead References: <1470935423-12329-1-git-send-email-hch@lst.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1470935423-12329-1-git-send-email-hch@lst.de> User-Agent: Mutt/1.5.23.1 (2014-03-12) X-Barracuda-Connect: UNKNOWN[198.137.202.9] X-Barracuda-Start-Time: 1470952486 X-Barracuda-Encrypted: ECDHE-RSA-AES128-GCM-SHA256 X-Barracuda-URL: https://192.48.157.11:443/cgi-mod/mark.cgi X-Barracuda-Scan-Msg-Size: 7173 X-Virus-Scanned: by bsmtpd at sgi.com X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.70 X-Barracuda-Spam-Status: No, SCORE=0.70 using per-user scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=2.7 tests=BSF_SC0_MISMATCH_TO, COMMA_SUBJECT, RDNS_NONE X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.31947 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.60 COMMA_SUBJECT Subject is like 'Re: FDSDS, this is a subject' 0.00 BSF_SC0_MISMATCH_TO Envelope rcpt doesn't match header 0.10 RDNS_NONE Delivered to trusted network by a host with no rDNS Cc: xfs@oss.sgi.com X-BeenThere: xfs@oss.sgi.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: XFS Filesystem from SGI List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: xfs-bounces@oss.sgi.com Sender: xfs-bounces@oss.sgi.com X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Aug 11, 2016 at 10:10:23AM -0700, Christoph Hellwig wrote: > There is one major issue with this change though: lockdep currently > doesn't have a facility to assert a rw_sempahore is held exclusively, > which means we lose the nice ability to assert locking context in > XFS. > > Peter, I think you mentioned this would be fairly easy to add to > lockdep and the rw_semaphore code. Any chance to come up with a proof > of concept? Sure, find below. Not been near a compiler. --- include/linux/lockdep.h | 17 +++++++++++++++-- kernel/locking/lockdep.c | 41 +++++++++++++++++++++++++---------------- 2 files changed, 40 insertions(+), 18 deletions(-) diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h index eabe0138eb06..7f0098d3a7d7 100644 --- a/include/linux/lockdep.h +++ b/include/linux/lockdep.h @@ -338,9 +338,14 @@ extern void lock_acquire(struct lockdep_map *lock, unsigned int subclass, extern void lock_release(struct lockdep_map *lock, int nested, unsigned long ip); -#define lockdep_is_held(lock) lock_is_held(&(lock)->dep_map) +extern int _lock_is_held(struct lockdep_map *lock, int read); -extern int lock_is_held(struct lockdep_map *lock); +static inline int lock_is_held(struct lockdep_map *lock) +{ + return _lock_is_held(lock, -1); +} + +#define lockdep_is_held(lock) lock_is_held(&(lock)->dep_map) extern void lock_set_class(struct lockdep_map *lock, const char *name, struct lock_class_key *key, unsigned int subclass, @@ -372,6 +377,14 @@ extern void lock_unpin_lock(struct lockdep_map *lock, struct pin_cookie); WARN_ON(debug_locks && !lockdep_is_held(l)); \ } while (0) +#define lockdep_assert_held_exclusive(l) do { \ + WARN_ON(debug_locks && !_lockdep_is_held(l, 0));\ + } while (0) + +#define lockdep_assert_held_read(l) do { \ + WARN_ON(debug_locks && !_lockdep_is_held(l, 1));\ + } while (0) + #define lockdep_assert_held_once(l) do { \ WARN_ON_ONCE(debug_locks && !lockdep_is_held(l)); \ } while (0) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 589d763a49b3..abec578378e7 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -3188,7 +3188,7 @@ print_lock_nested_lock_not_held(struct task_struct *curr, return 0; } -static int __lock_is_held(struct lockdep_map *lock); +static int __lock_is_held(struct lockdep_map *lock, int read); /* * This gets called for every mutex_lock*()/spin_lock*() operation. @@ -3329,7 +3329,7 @@ static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass, } chain_key = iterate_chain_key(chain_key, class_idx); - if (nest_lock && !__lock_is_held(nest_lock)) + if (nest_lock && !__lock_is_held(nest_lock, -1)) return print_lock_nested_lock_not_held(curr, hlock, ip); if (!validate_chain(curr, lock, hlock, chain_head, chain_key)) @@ -3390,10 +3390,17 @@ print_unlock_imbalance_bug(struct task_struct *curr, struct lockdep_map *lock, return 0; } -static int match_held_lock(struct held_lock *hlock, struct lockdep_map *lock) +static int match_held_lock(struct held_lock *hlock, struct lockdep_map *lock, int read) { - if (hlock->instance == lock) - return 1; + if (hlock->instance == lock) { + if (read == -1) + return 1; + + if (hlock->read == read) + return 1; + + return 0; + } if (hlock->references) { struct lock_class *class = lock->class_cache[0]; @@ -3420,6 +3427,8 @@ static int match_held_lock(struct held_lock *hlock, struct lockdep_map *lock) if (hlock->class_idx == class - lock_classes + 1) return 1; + + /* XXX do we want @read stuff for nested locks !? */ } return 0; @@ -3452,7 +3461,7 @@ __lock_set_class(struct lockdep_map *lock, const char *name, */ if (prev_hlock && prev_hlock->irq_context != hlock->irq_context) break; - if (match_held_lock(hlock, lock)) + if (match_held_lock(hlock, lock, -1)) goto found_it; prev_hlock = hlock; } @@ -3523,7 +3532,7 @@ __lock_release(struct lockdep_map *lock, int nested, unsigned long ip) */ if (prev_hlock && prev_hlock->irq_context != hlock->irq_context) break; - if (match_held_lock(hlock, lock)) + if (match_held_lock(hlock, lock, -1)) goto found_it; prev_hlock = hlock; } @@ -3576,7 +3585,7 @@ __lock_release(struct lockdep_map *lock, int nested, unsigned long ip) return 1; } -static int __lock_is_held(struct lockdep_map *lock) +static int __lock_is_held(struct lockdep_map *lock, int read) { struct task_struct *curr = current; int i; @@ -3584,7 +3593,7 @@ static int __lock_is_held(struct lockdep_map *lock) for (i = 0; i < curr->lockdep_depth; i++) { struct held_lock *hlock = curr->held_locks + i; - if (match_held_lock(hlock, lock)) + if (match_held_lock(hlock, lock, read)) return 1; } @@ -3603,7 +3612,7 @@ static struct pin_cookie __lock_pin_lock(struct lockdep_map *lock) for (i = 0; i < curr->lockdep_depth; i++) { struct held_lock *hlock = curr->held_locks + i; - if (match_held_lock(hlock, lock)) { + if (match_held_lock(hlock, lock, -1)) { /* * Grab 16bits of randomness; this is sufficient to not * be guessable and still allows some pin nesting in @@ -3630,7 +3639,7 @@ static void __lock_repin_lock(struct lockdep_map *lock, struct pin_cookie cookie for (i = 0; i < curr->lockdep_depth; i++) { struct held_lock *hlock = curr->held_locks + i; - if (match_held_lock(hlock, lock)) { + if (match_held_lock(hlock, lock, -1)) { hlock->pin_count += cookie.val; return; } @@ -3650,7 +3659,7 @@ static void __lock_unpin_lock(struct lockdep_map *lock, struct pin_cookie cookie for (i = 0; i < curr->lockdep_depth; i++) { struct held_lock *hlock = curr->held_locks + i; - if (match_held_lock(hlock, lock)) { + if (match_held_lock(hlock, lock, -1)) { if (WARN(!hlock->pin_count, "unpinning an unpinned lock\n")) return; @@ -3769,7 +3778,7 @@ void lock_release(struct lockdep_map *lock, int nested, } EXPORT_SYMBOL_GPL(lock_release); -int lock_is_held(struct lockdep_map *lock) +int lock_is_held(struct lockdep_map *lock, int read) { unsigned long flags; int ret = 0; @@ -3781,7 +3790,7 @@ int lock_is_held(struct lockdep_map *lock) check_flags(flags); current->lockdep_recursion = 1; - ret = __lock_is_held(lock); + ret = __lock_is_held(lock, read); current->lockdep_recursion = 0; raw_local_irq_restore(flags); @@ -3908,7 +3917,7 @@ __lock_contended(struct lockdep_map *lock, unsigned long ip) */ if (prev_hlock && prev_hlock->irq_context != hlock->irq_context) break; - if (match_held_lock(hlock, lock)) + if (match_held_lock(hlock, lock, -1)) goto found_it; prev_hlock = hlock; } @@ -3961,7 +3970,7 @@ __lock_acquired(struct lockdep_map *lock, unsigned long ip) */ if (prev_hlock && prev_hlock->irq_context != hlock->irq_context) break; - if (match_held_lock(hlock, lock)) + if (match_held_lock(hlock, lock, -1)) goto found_it; prev_hlock = hlock; }