From patchwork Fri Nov 2 09:15:20 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 10665189 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 6975A13B5 for ; Fri, 2 Nov 2018 09:15:52 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5BF4A2BA77 for ; Fri, 2 Nov 2018 09:15:52 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4F1792BA84; Fri, 2 Nov 2018 09:15:52 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id DD8322BA63 for ; Fri, 2 Nov 2018 09:15:48 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 973FF6E553; Fri, 2 Nov 2018 09:15:46 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ed1-x541.google.com (mail-ed1-x541.google.com [IPv6:2a00:1450:4864:20::541]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9C3466E544 for ; Fri, 2 Nov 2018 09:15:42 +0000 (UTC) Received: by mail-ed1-x541.google.com with SMTP id n19-v6so1261349edq.11 for ; Fri, 02 Nov 2018 02:15:42 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:mime-version :content-transfer-encoding; bh=noeBz7cd2mEDIbT3q2m/WXRJ/bi0mGu5C9ZFfXX//ws=; b=g8NYz8CbQH8aZP1a3+KXPiMXAGIhn7q6fmAQfPVzjXKD8HnWQ+oOz+hdWCJbdzbKW/ PU7aHsNHnNV/0xCzzRdSfFsitbjT2mdi2ossVoNHUfdvqvvPOn31K/D3IPMPtIbiZfof wqF/b24O6uC/3R31w7IG5jrgsMf+/RURQrt+rdMAojZL+1+6guzU7mEDkleUgkHaxzxP KNtpGUWvn05H8g+z48QIhnEo3WJ6q94qoW0oVKZZ+aekVzVENqwp3yc3AVXeMW4IS90K KwMErVOMbq/v3FLzWZDxkqHVdxuUxTh58QuU5+UvbDxT+0ZURwq5deRCc6RZqP+PLnrK 41xA== X-Gm-Message-State: AGRZ1gJH10n1whtqN2Snkm+e4ALDvpytXlMfUwWOyqxjCADnLp0nM03y bfJrnVVlBhMzQferLTpWB5ur+69Oehw= X-Google-Smtp-Source: AJdET5esaBV94mpP27UwvDQ2ln4AfL+TcF6rb8irFpqtnGAm7OP4EIpfDZCTAhYaf624w3v6cTf/Ow== X-Received: by 2002:a17:906:4a4b:: with SMTP id a11-v6mr6204853ejv.68.1541150139846; Fri, 02 Nov 2018 02:15:39 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id p19-v6sm5658103ejw.69.2018.11.02.02.15.38 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 02 Nov 2018 02:15:38 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Fri, 2 Nov 2018 10:15:20 +0100 Message-Id: <20181102091531.7775-1-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.19.1 MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 01/12] locking/lockdep: restore cross-release checks X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP This reverts the following commits: 527187d28569 ("locking/lockdep: Remove cross-release leftovers") dba04eb76df9 ("locking/Documentation: Remove stale crossrelease_fullstack parameter") e966eaeeb623 ("locking/lockdep: Remove the cross-release locking checks") Since the first two are just fixups for the cross-release removal I figured I'll squash them all into one. Cc: Byungchul Park Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: linux-kernel@vger.kernel.org Cc: Ingo Molnar Cc: Will Deacon Cc: Jonathan Corbet Cc: Bjorn Helgaas Cc: Greg Kroah-Hartman Cc: Marc Zyngier Cc: Kai-Heng Feng Cc: Thymo van Beers Cc: Jiri Kosina Cc: Konrad Rzeszutek Wilk Cc: Logan Gunthorpe Cc: David Rientjes Cc: Kate Stewart Cc: Philippe Ombredanne Cc: Daniel Vetter Cc: "Joel Fernandes (Google)" Cc: "Steven Rostedt (VMware)" Cc: David Howells Cc: Namhyung Kim Cc: Andrew Morton Cc: Masahiro Yamada Cc: Randy Dunlap Cc: Arnd Bergmann Cc: Waiman Long Cc: Masami Hiramatsu Cc: Yury Norov Cc: Mikulas Patocka Cc: Robin Murphy Cc: Andy Shevchenko Signed-off-by: Daniel Vetter --- .../admin-guide/kernel-parameters.txt | 3 + Documentation/locking/crossrelease.txt | 874 ++++++++++++++++++ include/linux/completion.h | 46 + include/linux/irqflags.h | 4 + include/linux/lockdep.h | 127 +++ include/linux/sched.h | 11 + kernel/locking/lockdep.c | 652 ++++++++++++- kernel/sched/completion.c | 5 + lib/Kconfig.debug | 33 + 9 files changed, 1720 insertions(+), 35 deletions(-) create mode 100644 Documentation/locking/crossrelease.txt diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 92eb1f42240d..6ab3ad188c01 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -724,6 +724,9 @@ It will be ignored when crashkernel=X,high is not used or memory reserved is below 4G. + crossrelease_fullstack + [KNL] Allow to record full stack trace in cross-release + cryptomgr.notests [KNL] Disable crypto self-tests diff --git a/Documentation/locking/crossrelease.txt b/Documentation/locking/crossrelease.txt new file mode 100644 index 000000000000..bdf1423d5f99 --- /dev/null +++ b/Documentation/locking/crossrelease.txt @@ -0,0 +1,874 @@ +Crossrelease +============ + +Started by Byungchul Park + +Contents: + + (*) Background + + - What causes deadlock + - How lockdep works + + (*) Limitation + + - Limit lockdep + - Pros from the limitation + - Cons from the limitation + - Relax the limitation + + (*) Crossrelease + + - Introduce crossrelease + - Introduce commit + + (*) Implementation + + - Data structures + - How crossrelease works + + (*) Optimizations + + - Avoid duplication + - Lockless for hot paths + + (*) APPENDIX A: What lockdep does to work aggresively + + (*) APPENDIX B: How to avoid adding false dependencies + + +========== +Background +========== + +What causes deadlock +-------------------- + +A deadlock occurs when a context is waiting for an event to happen, +which is impossible because another (or the) context who can trigger the +event is also waiting for another (or the) event to happen, which is +also impossible due to the same reason. + +For example: + + A context going to trigger event C is waiting for event A to happen. + A context going to trigger event A is waiting for event B to happen. + A context going to trigger event B is waiting for event C to happen. + +A deadlock occurs when these three wait operations run at the same time, +because event C cannot be triggered if event A does not happen, which in +turn cannot be triggered if event B does not happen, which in turn +cannot be triggered if event C does not happen. After all, no event can +be triggered since any of them never meets its condition to wake up. + +A dependency might exist between two waiters and a deadlock might happen +due to an incorrect releationship between dependencies. Thus, we must +define what a dependency is first. A dependency exists between them if: + + 1. There are two waiters waiting for each event at a given time. + 2. The only way to wake up each waiter is to trigger its event. + 3. Whether one can be woken up depends on whether the other can. + +Each wait in the example creates its dependency like: + + Event C depends on event A. + Event A depends on event B. + Event B depends on event C. + + NOTE: Precisely speaking, a dependency is one between whether a + waiter for an event can be woken up and whether another waiter for + another event can be woken up. However from now on, we will describe + a dependency as if it's one between an event and another event for + simplicity. + +And they form circular dependencies like: + + -> C -> A -> B - + / \ + \ / + ---------------- + + where 'A -> B' means that event A depends on event B. + +Such circular dependencies lead to a deadlock since no waiter can meet +its condition to wake up as described. + +CONCLUSION + +Circular dependencies cause a deadlock. + + +How lockdep works +----------------- + +Lockdep tries to detect a deadlock by checking dependencies created by +lock operations, acquire and release. Waiting for a lock corresponds to +waiting for an event, and releasing a lock corresponds to triggering an +event in the previous section. + +In short, lockdep does: + + 1. Detect a new dependency. + 2. Add the dependency into a global graph. + 3. Check if that makes dependencies circular. + 4. Report a deadlock or its possibility if so. + +For example, consider a graph built by lockdep that looks like: + + A -> B - + \ + -> E + / + C -> D - + + where A, B,..., E are different lock classes. + +Lockdep will add a dependency into the graph on detection of a new +dependency. For example, it will add a dependency 'E -> C' when a new +dependency between lock E and lock C is detected. Then the graph will be: + + A -> B - + \ + -> E - + / \ + -> C -> D - \ + / / + \ / + ------------------ + + where A, B,..., E are different lock classes. + +This graph contains a subgraph which demonstrates circular dependencies: + + -> E - + / \ + -> C -> D - \ + / / + \ / + ------------------ + + where C, D and E are different lock classes. + +This is the condition under which a deadlock might occur. Lockdep +reports it on detection after adding a new dependency. This is the way +how lockdep works. + +CONCLUSION + +Lockdep detects a deadlock or its possibility by checking if circular +dependencies were created after adding each new dependency. + + +========== +Limitation +========== + +Limit lockdep +------------- + +Limiting lockdep to work on only typical locks e.g. spin locks and +mutexes, which are released within the acquire context, the +implementation becomes simple but its capacity for detection becomes +limited. Let's check pros and cons in next section. + + +Pros from the limitation +------------------------ + +Given the limitation, when acquiring a lock, locks in a held_locks +cannot be released if the context cannot acquire it so has to wait to +acquire it, which means all waiters for the locks in the held_locks are +stuck. It's an exact case to create dependencies between each lock in +the held_locks and the lock to acquire. + +For example: + + CONTEXT X + --------- + acquire A + acquire B /* Add a dependency 'A -> B' */ + release B + release A + + where A and B are different lock classes. + +When acquiring lock A, the held_locks of CONTEXT X is empty thus no +dependency is added. But when acquiring lock B, lockdep detects and adds +a new dependency 'A -> B' between lock A in the held_locks and lock B. +They can be simply added whenever acquiring each lock. + +And data required by lockdep exists in a local structure, held_locks +embedded in task_struct. Forcing to access the data within the context, +lockdep can avoid racy problems without explicit locks while handling +the local data. + +Lastly, lockdep only needs to keep locks currently being held, to build +a dependency graph. However, relaxing the limitation, it needs to keep +even locks already released, because a decision whether they created +dependencies might be long-deferred. + +To sum up, we can expect several advantages from the limitation: + + 1. Lockdep can easily identify a dependency when acquiring a lock. + 2. Races are avoidable while accessing local locks in a held_locks. + 3. Lockdep only needs to keep locks currently being held. + +CONCLUSION + +Given the limitation, the implementation becomes simple and efficient. + + +Cons from the limitation +------------------------ + +Given the limitation, lockdep is applicable only to typical locks. For +example, page locks for page access or completions for synchronization +cannot work with lockdep. + +Can we detect deadlocks below, under the limitation? + +Example 1: + + CONTEXT X CONTEXT Y CONTEXT Z + --------- --------- ---------- + mutex_lock A + lock_page B + lock_page B + mutex_lock A /* DEADLOCK */ + unlock_page B held by X + unlock_page B + mutex_unlock A + mutex_unlock A + + where A and B are different lock classes. + +No, we cannot. + +Example 2: + + CONTEXT X CONTEXT Y + --------- --------- + mutex_lock A + mutex_lock A + wait_for_complete B /* DEADLOCK */ + complete B + mutex_unlock A + mutex_unlock A + + where A is a lock class and B is a completion variable. + +No, we cannot. + +CONCLUSION + +Given the limitation, lockdep cannot detect a deadlock or its +possibility caused by page locks or completions. + + +Relax the limitation +-------------------- + +Under the limitation, things to create dependencies are limited to +typical locks. However, synchronization primitives like page locks and +completions, which are allowed to be released in any context, also +create dependencies and can cause a deadlock. So lockdep should track +these locks to do a better job. We have to relax the limitation for +these locks to work with lockdep. + +Detecting dependencies is very important for lockdep to work because +adding a dependency means adding an opportunity to check whether it +causes a deadlock. The more lockdep adds dependencies, the more it +thoroughly works. Thus Lockdep has to do its best to detect and add as +many true dependencies into a graph as possible. + +For example, considering only typical locks, lockdep builds a graph like: + + A -> B - + \ + -> E + / + C -> D - + + where A, B,..., E are different lock classes. + +On the other hand, under the relaxation, additional dependencies might +be created and added. Assuming additional 'FX -> C' and 'E -> GX' are +added thanks to the relaxation, the graph will be: + + A -> B - + \ + -> E -> GX + / + FX -> C -> D - + + where A, B,..., E, FX and GX are different lock classes, and a suffix + 'X' is added on non-typical locks. + +The latter graph gives us more chances to check circular dependencies +than the former. However, it might suffer performance degradation since +relaxing the limitation, with which design and implementation of lockdep +can be efficient, might introduce inefficiency inevitably. So lockdep +should provide two options, strong detection and efficient detection. + +Choosing efficient detection: + + Lockdep works with only locks restricted to be released within the + acquire context. However, lockdep works efficiently. + +Choosing strong detection: + + Lockdep works with all synchronization primitives. However, lockdep + suffers performance degradation. + +CONCLUSION + +Relaxing the limitation, lockdep can add additional dependencies giving +additional opportunities to check circular dependencies. + + +============ +Crossrelease +============ + +Introduce crossrelease +---------------------- + +In order to allow lockdep to handle additional dependencies by what +might be released in any context, namely 'crosslock', we have to be able +to identify those created by crosslocks. The proposed 'crossrelease' +feature provoides a way to do that. + +Crossrelease feature has to do: + + 1. Identify dependencies created by crosslocks. + 2. Add the dependencies into a dependency graph. + +That's all. Once a meaningful dependency is added into graph, then +lockdep would work with the graph as it did. The most important thing +crossrelease feature has to do is to correctly identify and add true +dependencies into the global graph. + +A dependency e.g. 'A -> B' can be identified only in the A's release +context because a decision required to identify the dependency can be +made only in the release context. That is to decide whether A can be +released so that a waiter for A can be woken up. It cannot be made in +other than the A's release context. + +It's no matter for typical locks because each acquire context is same as +its release context, thus lockdep can decide whether a lock can be +released in the acquire context. However for crosslocks, lockdep cannot +make the decision in the acquire context but has to wait until the +release context is identified. + +Therefore, deadlocks by crosslocks cannot be detected just when it +happens, because those cannot be identified until the crosslocks are +released. However, deadlock possibilities can be detected and it's very +worth. See 'APPENDIX A' section to check why. + +CONCLUSION + +Using crossrelease feature, lockdep can work with what might be released +in any context, namely crosslock. + + +Introduce commit +---------------- + +Since crossrelease defers the work adding true dependencies of +crosslocks until they are actually released, crossrelease has to queue +all acquisitions which might create dependencies with the crosslocks. +Then it identifies dependencies using the queued data in batches at a +proper time. We call it 'commit'. + +There are four types of dependencies: + +1. TT type: 'typical lock A -> typical lock B' + + Just when acquiring B, lockdep can see it's in the A's release + context. So the dependency between A and B can be identified + immediately. Commit is unnecessary. + +2. TC type: 'typical lock A -> crosslock BX' + + Just when acquiring BX, lockdep can see it's in the A's release + context. So the dependency between A and BX can be identified + immediately. Commit is unnecessary, too. + +3. CT type: 'crosslock AX -> typical lock B' + + When acquiring B, lockdep cannot identify the dependency because + there's no way to know if it's in the AX's release context. It has + to wait until the decision can be made. Commit is necessary. + +4. CC type: 'crosslock AX -> crosslock BX' + + When acquiring BX, lockdep cannot identify the dependency because + there's no way to know if it's in the AX's release context. It has + to wait until the decision can be made. Commit is necessary. + But, handling CC type is not implemented yet. It's a future work. + +Lockdep can work without commit for typical locks, but commit step is +necessary once crosslocks are involved. Introducing commit, lockdep +performs three steps. What lockdep does in each step is: + +1. Acquisition: For typical locks, lockdep does what it originally did + and queues the lock so that CT type dependencies can be checked using + it at the commit step. For crosslocks, it saves data which will be + used at the commit step and increases a reference count for it. + +2. Commit: No action is reauired for typical locks. For crosslocks, + lockdep adds CT type dependencies using the data saved at the + acquisition step. + +3. Release: No changes are required for typical locks. When a crosslock + is released, it decreases a reference count for it. + +CONCLUSION + +Crossrelease introduces commit step to handle dependencies of crosslocks +in batches at a proper time. + + +============== +Implementation +============== + +Data structures +--------------- + +Crossrelease introduces two main data structures. + +1. hist_lock + + This is an array embedded in task_struct, for keeping lock history so + that dependencies can be added using them at the commit step. Since + it's local data, it can be accessed locklessly in the owner context. + The array is filled at the acquisition step and consumed at the + commit step. And it's managed in circular manner. + +2. cross_lock + + One per lockdep_map exists. This is for keeping data of crosslocks + and used at the commit step. + + +How crossrelease works +---------------------- + +It's the key of how crossrelease works, to defer necessary works to an +appropriate point in time and perform in at once at the commit step. +Let's take a look with examples step by step, starting from how lockdep +works without crossrelease for typical locks. + + acquire A /* Push A onto held_locks */ + acquire B /* Push B onto held_locks and add 'A -> B' */ + acquire C /* Push C onto held_locks and add 'B -> C' */ + release C /* Pop C from held_locks */ + release B /* Pop B from held_locks */ + release A /* Pop A from held_locks */ + + where A, B and C are different lock classes. + + NOTE: This document assumes that readers already understand how + lockdep works without crossrelease thus omits details. But there's + one thing to note. Lockdep pretends to pop a lock from held_locks + when releasing it. But it's subtly different from the original pop + operation because lockdep allows other than the top to be poped. + +In this case, lockdep adds 'the top of held_locks -> the lock to acquire' +dependency every time acquiring a lock. + +After adding 'A -> B', a dependency graph will be: + + A -> B + + where A and B are different lock classes. + +And after adding 'B -> C', the graph will be: + + A -> B -> C + + where A, B and C are different lock classes. + +Let's performs commit step even for typical locks to add dependencies. +Of course, commit step is not necessary for them, however, it would work +well because this is a more general way. + + acquire A + /* + * Queue A into hist_locks + * + * In hist_locks: A + * In graph: Empty + */ + + acquire B + /* + * Queue B into hist_locks + * + * In hist_locks: A, B + * In graph: Empty + */ + + acquire C + /* + * Queue C into hist_locks + * + * In hist_locks: A, B, C + * In graph: Empty + */ + + commit C + /* + * Add 'C -> ?' + * Answer the following to decide '?' + * What has been queued since acquire C: Nothing + * + * In hist_locks: A, B, C + * In graph: Empty + */ + + release C + + commit B + /* + * Add 'B -> ?' + * Answer the following to decide '?' + * What has been queued since acquire B: C + * + * In hist_locks: A, B, C + * In graph: 'B -> C' + */ + + release B + + commit A + /* + * Add 'A -> ?' + * Answer the following to decide '?' + * What has been queued since acquire A: B, C + * + * In hist_locks: A, B, C + * In graph: 'B -> C', 'A -> B', 'A -> C' + */ + + release A + + where A, B and C are different lock classes. + +In this case, dependencies are added at the commit step as described. + +After commits for A, B and C, the graph will be: + + A -> B -> C + + where A, B and C are different lock classes. + + NOTE: A dependency 'A -> C' is optimized out. + +We can see the former graph built without commit step is same as the +latter graph built using commit steps. Of course the former way leads to +earlier finish for building the graph, which means we can detect a +deadlock or its possibility sooner. So the former way would be prefered +when possible. But we cannot avoid using the latter way for crosslocks. + +Let's look at how commit steps work for crosslocks. In this case, the +commit step is performed only on crosslock AX as real. And it assumes +that the AX release context is different from the AX acquire context. + + BX RELEASE CONTEXT BX ACQUIRE CONTEXT + ------------------ ------------------ + acquire A + /* + * Push A onto held_locks + * Queue A into hist_locks + * + * In held_locks: A + * In hist_locks: A + * In graph: Empty + */ + + acquire BX + /* + * Add 'the top of held_locks -> BX' + * + * In held_locks: A + * In hist_locks: A + * In graph: 'A -> BX' + */ + + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + It must be guaranteed that the following operations are seen after + acquiring BX globally. It can be done by things like barrier. + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + + acquire C + /* + * Push C onto held_locks + * Queue C into hist_locks + * + * In held_locks: C + * In hist_locks: C + * In graph: 'A -> BX' + */ + + release C + /* + * Pop C from held_locks + * + * In held_locks: Empty + * In hist_locks: C + * In graph: 'A -> BX' + */ + acquire D + /* + * Push D onto held_locks + * Queue D into hist_locks + * Add 'the top of held_locks -> D' + * + * In held_locks: A, D + * In hist_locks: A, D + * In graph: 'A -> BX', 'A -> D' + */ + acquire E + /* + * Push E onto held_locks + * Queue E into hist_locks + * + * In held_locks: E + * In hist_locks: C, E + * In graph: 'A -> BX', 'A -> D' + */ + + release E + /* + * Pop E from held_locks + * + * In held_locks: Empty + * In hist_locks: D, E + * In graph: 'A -> BX', 'A -> D' + */ + release D + /* + * Pop D from held_locks + * + * In held_locks: A + * In hist_locks: A, D + * In graph: 'A -> BX', 'A -> D' + */ + commit BX + /* + * Add 'BX -> ?' + * What has been queued since acquire BX: C, E + * + * In held_locks: Empty + * In hist_locks: D, E + * In graph: 'A -> BX', 'A -> D', + * 'BX -> C', 'BX -> E' + */ + + release BX + /* + * In held_locks: Empty + * In hist_locks: D, E + * In graph: 'A -> BX', 'A -> D', + * 'BX -> C', 'BX -> E' + */ + release A + /* + * Pop A from held_locks + * + * In held_locks: Empty + * In hist_locks: A, D + * In graph: 'A -> BX', 'A -> D', + * 'BX -> C', 'BX -> E' + */ + + where A, BX, C,..., E are different lock classes, and a suffix 'X' is + added on crosslocks. + +Crossrelease considers all acquisitions after acqiuring BX are +candidates which might create dependencies with BX. True dependencies +will be determined when identifying the release context of BX. Meanwhile, +all typical locks are queued so that they can be used at the commit step. +And then two dependencies 'BX -> C' and 'BX -> E' are added at the +commit step when identifying the release context. + +The final graph will be, with crossrelease: + + -> C + / + -> BX - + / \ + A - -> E + \ + -> D + + where A, BX, C,..., E are different lock classes, and a suffix 'X' is + added on crosslocks. + +However, the final graph will be, without crossrelease: + + A -> D + + where A and D are different lock classes. + +The former graph has three more dependencies, 'A -> BX', 'BX -> C' and +'BX -> E' giving additional opportunities to check if they cause +deadlocks. This way lockdep can detect a deadlock or its possibility +caused by crosslocks. + +CONCLUSION + +We checked how crossrelease works with several examples. + + +============= +Optimizations +============= + +Avoid duplication +----------------- + +Crossrelease feature uses a cache like what lockdep already uses for +dependency chains, but this time it's for caching CT type dependencies. +Once that dependency is cached, the same will never be added again. + + +Lockless for hot paths +---------------------- + +To keep all locks for later use at the commit step, crossrelease adopts +a local array embedded in task_struct, which makes access to the data +lockless by forcing it to happen only within the owner context. It's +like how lockdep handles held_locks. Lockless implmentation is important +since typical locks are very frequently acquired and released. + + +================================================= +APPENDIX A: What lockdep does to work aggresively +================================================= + +A deadlock actually occurs when all wait operations creating circular +dependencies run at the same time. Even though they don't, a potential +deadlock exists if the problematic dependencies exist. Thus it's +meaningful to detect not only an actual deadlock but also its potential +possibility. The latter is rather valuable. When a deadlock occurs +actually, we can identify what happens in the system by some means or +other even without lockdep. However, there's no way to detect possiblity +without lockdep unless the whole code is parsed in head. It's terrible. +Lockdep does the both, and crossrelease only focuses on the latter. + +Whether or not a deadlock actually occurs depends on several factors. +For example, what order contexts are switched in is a factor. Assuming +circular dependencies exist, a deadlock would occur when contexts are +switched so that all wait operations creating the dependencies run +simultaneously. Thus to detect a deadlock possibility even in the case +that it has not occured yet, lockdep should consider all possible +combinations of dependencies, trying to: + +1. Use a global dependency graph. + + Lockdep combines all dependencies into one global graph and uses them, + regardless of which context generates them or what order contexts are + switched in. Aggregated dependencies are only considered so they are + prone to be circular if a problem exists. + +2. Check dependencies between classes instead of instances. + + What actually causes a deadlock are instances of lock. However, + lockdep checks dependencies between classes instead of instances. + This way lockdep can detect a deadlock which has not happened but + might happen in future by others but the same class. + +3. Assume all acquisitions lead to waiting. + + Although locks might be acquired without waiting which is essential + to create dependencies, lockdep assumes all acquisitions lead to + waiting since it might be true some time or another. + +CONCLUSION + +Lockdep detects not only an actual deadlock but also its possibility, +and the latter is more valuable. + + +================================================== +APPENDIX B: How to avoid adding false dependencies +================================================== + +Remind what a dependency is. A dependency exists if: + + 1. There are two waiters waiting for each event at a given time. + 2. The only way to wake up each waiter is to trigger its event. + 3. Whether one can be woken up depends on whether the other can. + +For example: + + acquire A + acquire B /* A dependency 'A -> B' exists */ + release B + release A + + where A and B are different lock classes. + +A depedency 'A -> B' exists since: + + 1. A waiter for A and a waiter for B might exist when acquiring B. + 2. Only way to wake up each is to release what it waits for. + 3. Whether the waiter for A can be woken up depends on whether the + other can. IOW, TASK X cannot release A if it fails to acquire B. + +For another example: + + TASK X TASK Y + ------ ------ + acquire AX + acquire B /* A dependency 'AX -> B' exists */ + release B + release AX held by Y + + where AX and B are different lock classes, and a suffix 'X' is added + on crosslocks. + +Even in this case involving crosslocks, the same rule can be applied. A +depedency 'AX -> B' exists since: + + 1. A waiter for AX and a waiter for B might exist when acquiring B. + 2. Only way to wake up each is to release what it waits for. + 3. Whether the waiter for AX can be woken up depends on whether the + other can. IOW, TASK X cannot release AX if it fails to acquire B. + +Let's take a look at more complicated example: + + TASK X TASK Y + ------ ------ + acquire B + release B + fork Y + acquire AX + acquire C /* A dependency 'AX -> C' exists */ + release C + release AX held by Y + + where AX, B and C are different lock classes, and a suffix 'X' is + added on crosslocks. + +Does a dependency 'AX -> B' exist? Nope. + +Two waiters are essential to create a dependency. However, waiters for +AX and B to create 'AX -> B' cannot exist at the same time in this +example. Thus the dependency 'AX -> B' cannot be created. + +It would be ideal if the full set of true ones can be considered. But +we can ensure nothing but what actually happened. Relying on what +actually happens at runtime, we can anyway add only true ones, though +they might be a subset of true ones. It's similar to how lockdep works +for typical locks. There might be more true dependencies than what +lockdep has detected in runtime. Lockdep has no choice but to rely on +what actually happens. Crossrelease also relies on it. + +CONCLUSION + +Relying on what actually happens, lockdep can avoid adding false +dependencies. diff --git a/include/linux/completion.h b/include/linux/completion.h index 519e94915d18..0662a417febe 100644 --- a/include/linux/completion.h +++ b/include/linux/completion.h @@ -10,6 +10,9 @@ */ #include +#ifdef CONFIG_LOCKDEP_COMPLETIONS +#include +#endif /* * struct completion - structure used to maintain state for a "completion" @@ -26,15 +29,58 @@ struct completion { unsigned int done; wait_queue_head_t wait; +#ifdef CONFIG_LOCKDEP_COMPLETIONS + struct lockdep_map_cross map; +#endif }; +#ifdef CONFIG_LOCKDEP_COMPLETIONS +static inline void complete_acquire(struct completion *x) +{ + lock_acquire_exclusive((struct lockdep_map *)&x->map, 0, 0, NULL, _RET_IP_); +} + +static inline void complete_release(struct completion *x) +{ + lock_release((struct lockdep_map *)&x->map, 0, _RET_IP_); +} + +static inline void complete_release_commit(struct completion *x) +{ + lock_commit_crosslock((struct lockdep_map *)&x->map); +} + +#define init_completion_map(x, m) \ +do { \ + lockdep_init_map_crosslock((struct lockdep_map *)&(x)->map, \ + (m)->name, (m)->key, 0); \ + __init_completion(x); \ +} while (0) + +#define init_completion(x) \ +do { \ + static struct lock_class_key __key; \ + lockdep_init_map_crosslock((struct lockdep_map *)&(x)->map, \ + "(completion)" #x, \ + &__key, 0); \ + __init_completion(x); \ +} while (0) +#else #define init_completion_map(x, m) __init_completion(x) #define init_completion(x) __init_completion(x) static inline void complete_acquire(struct completion *x) {} static inline void complete_release(struct completion *x) {} +static inline void complete_release_commit(struct completion *x) {} +#endif +#ifdef CONFIG_LOCKDEP_COMPLETIONS +#define COMPLETION_INITIALIZER(work) \ + { 0, __WAIT_QUEUE_HEAD_INITIALIZER((work).wait), \ + STATIC_CROSS_LOCKDEP_MAP_INIT("(completion)" #work, &(work)) } +#else #define COMPLETION_INITIALIZER(work) \ { 0, __WAIT_QUEUE_HEAD_INITIALIZER((work).wait) } +#endif #define COMPLETION_INITIALIZER_ONSTACK_MAP(work, map) \ (*({ init_completion_map(&(work), &(map)); &(work); })) diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h index 21619c92c377..bd1bda7248bf 100644 --- a/include/linux/irqflags.h +++ b/include/linux/irqflags.h @@ -38,18 +38,22 @@ # define trace_hardirq_enter() \ do { \ current->hardirq_context++; \ + crossrelease_hist_start(XHLOCK_HARD); \ } while (0) # define trace_hardirq_exit() \ do { \ current->hardirq_context--; \ + crossrelease_hist_end(XHLOCK_HARD); \ } while (0) # define lockdep_softirq_enter() \ do { \ current->softirq_context++; \ + crossrelease_hist_start(XHLOCK_SOFT); \ } while (0) # define lockdep_softirq_exit() \ do { \ current->softirq_context--; \ + crossrelease_hist_end(XHLOCK_SOFT); \ } while (0) #else # define trace_hardirqs_on() do { } while (0) diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h index b0d0b51c4d85..59b973ee4ce5 100644 --- a/include/linux/lockdep.h +++ b/include/linux/lockdep.h @@ -158,6 +158,12 @@ struct lockdep_map { int cpu; unsigned long ip; #endif +#ifdef CONFIG_LOCKDEP_CROSSRELEASE + /* + * Whether it's a crosslock. + */ + int cross; +#endif }; static inline void lockdep_copy_map(struct lockdep_map *to, @@ -261,8 +267,95 @@ struct held_lock { unsigned int hardirqs_off:1; unsigned int references:12; /* 32 bits */ unsigned int pin_count; +#ifdef CONFIG_LOCKDEP_CROSSRELEASE + /* + * Generation id. + * + * A value of cross_gen_id will be stored when holding this, + * which is globally increased whenever each crosslock is held. + */ + unsigned int gen_id; +#endif +}; + +#ifdef CONFIG_LOCKDEP_CROSSRELEASE +#define MAX_XHLOCK_TRACE_ENTRIES 5 + +/* + * This is for keeping locks waiting for commit so that true dependencies + * can be added at commit step. + */ +struct hist_lock { + /* + * Id for each entry in the ring buffer. This is used to + * decide whether the ring buffer was overwritten or not. + * + * For example, + * + * |<----------- hist_lock ring buffer size ------->| + * pppppppppppppppppppppiiiiiiiiiiiiiiiiiiiiiiiiiiiii + * wrapped > iiiiiiiiiiiiiiiiiiiiiiiiiii....................... + * + * where 'p' represents an acquisition in process + * context, 'i' represents an acquisition in irq + * context. + * + * In this example, the ring buffer was overwritten by + * acquisitions in irq context, that should be detected on + * rollback or commit. + */ + unsigned int hist_id; + + /* + * Seperate stack_trace data. This will be used at commit step. + */ + struct stack_trace trace; + unsigned long trace_entries[MAX_XHLOCK_TRACE_ENTRIES]; + + /* + * Seperate hlock instance. This will be used at commit step. + * + * TODO: Use a smaller data structure containing only necessary + * data. However, we should make lockdep code able to handle the + * smaller one first. + */ + struct held_lock hlock; }; +/* + * To initialize a lock as crosslock, lockdep_init_map_crosslock() should + * be called instead of lockdep_init_map(). + */ +struct cross_lock { + /* + * When more than one acquisition of crosslocks are overlapped, + * we have to perform commit for them based on cross_gen_id of + * the first acquisition, which allows us to add more true + * dependencies. + * + * Moreover, when no acquisition of a crosslock is in progress, + * we should not perform commit because the lock might not exist + * any more, which might cause incorrect memory access. So we + * have to track the number of acquisitions of a crosslock. + */ + int nr_acquire; + + /* + * Seperate hlock instance. This will be used at commit step. + * + * TODO: Use a smaller data structure containing only necessary + * data. However, we should make lockdep code able to handle the + * smaller one first. + */ + struct held_lock hlock; +}; + +struct lockdep_map_cross { + struct lockdep_map map; + struct cross_lock xlock; +}; +#endif + /* * Initialization, self-test and debugging-output methods: */ @@ -464,6 +557,37 @@ enum xhlock_context_t { XHLOCK_CTX_NR, }; +#ifdef CONFIG_LOCKDEP_CROSSRELEASE +extern void lockdep_init_map_crosslock(struct lockdep_map *lock, + const char *name, + struct lock_class_key *key, + int subclass); +extern void lock_commit_crosslock(struct lockdep_map *lock); + +/* + * What we essencially have to initialize is 'nr_acquire'. Other members + * will be initialized in add_xlock(). + */ +#define STATIC_CROSS_LOCK_INIT() \ + { .nr_acquire = 0,} + +#define STATIC_CROSS_LOCKDEP_MAP_INIT(_name, _key) \ + { .map.name = (_name), .map.key = (void *)(_key), \ + .map.cross = 1, .xlock = STATIC_CROSS_LOCK_INIT(), } + +/* + * To initialize a lockdep_map statically use this macro. + * Note that _name must not be NULL. + */ +#define STATIC_LOCKDEP_MAP_INIT(_name, _key) \ + { .name = (_name), .key = (void *)(_key), .cross = 0, } + +extern void crossrelease_hist_start(enum xhlock_context_t c); +extern void crossrelease_hist_end(enum xhlock_context_t c); +extern void lockdep_invariant_state(bool force); +extern void lockdep_init_task(struct task_struct *task); +extern void lockdep_free_task(struct task_struct *task); +#else /* !CROSSRELEASE */ #define lockdep_init_map_crosslock(m, n, k, s) do {} while (0) /* * To initialize a lockdep_map statically use this macro. @@ -472,9 +596,12 @@ enum xhlock_context_t { #define STATIC_LOCKDEP_MAP_INIT(_name, _key) \ { .name = (_name), .key = (void *)(_key), } +static inline void crossrelease_hist_start(enum xhlock_context_t c) {} +static inline void crossrelease_hist_end(enum xhlock_context_t c) {} static inline void lockdep_invariant_state(bool force) {} static inline void lockdep_init_task(struct task_struct *task) {} static inline void lockdep_free_task(struct task_struct *task) {} +#endif /* CROSSRELEASE */ #ifdef CONFIG_LOCK_STAT diff --git a/include/linux/sched.h b/include/linux/sched.h index 977cb57d7bc9..155c7690eb15 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -936,6 +936,17 @@ struct task_struct { struct held_lock held_locks[MAX_LOCK_DEPTH]; #endif +#ifdef CONFIG_LOCKDEP_CROSSRELEASE +#define MAX_XHLOCKS_NR 64UL + struct hist_lock *xhlocks; /* Crossrelease history locks */ + unsigned int xhlock_idx; + /* For restoring at history boundaries */ + unsigned int xhlock_idx_hist[XHLOCK_CTX_NR]; + unsigned int hist_id; + /* For overwrite check at each context exit */ + unsigned int hist_id_save[XHLOCK_CTX_NR]; +#endif + #ifdef CONFIG_UBSAN unsigned int in_ubsan; #endif diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index dd13f865ad40..c4d0fe3538da 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -58,6 +58,10 @@ #define CREATE_TRACE_POINTS #include +#ifdef CONFIG_LOCKDEP_CROSSRELEASE +#include +#endif + #ifdef CONFIG_PROVE_LOCKING int prove_locking = 1; module_param(prove_locking, int, 0644); @@ -72,6 +76,19 @@ module_param(lock_stat, int, 0644); #define lock_stat 0 #endif +#ifdef CONFIG_BOOTPARAM_LOCKDEP_CROSSRELEASE_FULLSTACK +static int crossrelease_fullstack = 1; +#else +static int crossrelease_fullstack; +#endif +static int __init allow_crossrelease_fullstack(char *str) +{ + crossrelease_fullstack = 1; + return 0; +} + +early_param("crossrelease_fullstack", allow_crossrelease_fullstack); + /* * lockdep_lock: protects the lockdep graph, the hashes and the * class/list/hash allocators. @@ -731,6 +748,18 @@ static bool assign_lock_key(struct lockdep_map *lock) return true; } +#ifdef CONFIG_LOCKDEP_CROSSRELEASE +static void cross_init(struct lockdep_map *lock, int cross); +static int cross_lock(struct lockdep_map *lock); +static int lock_acquire_crosslock(struct held_lock *hlock); +static int lock_release_crosslock(struct lockdep_map *lock); +#else +static inline void cross_init(struct lockdep_map *lock, int cross) {} +static inline int cross_lock(struct lockdep_map *lock) { return 0; } +static inline int lock_acquire_crosslock(struct held_lock *hlock) { return 2; } +static inline int lock_release_crosslock(struct lockdep_map *lock) { return 2; } +#endif + /* * Register a lock's class in the hash-table, if the class is not present * yet. Otherwise we look it up. We cache the result in the lock object @@ -1125,22 +1154,41 @@ print_circular_lock_scenario(struct held_lock *src, printk(KERN_CONT "\n\n"); } - printk(" Possible unsafe locking scenario:\n\n"); - printk(" CPU0 CPU1\n"); - printk(" ---- ----\n"); - printk(" lock("); - __print_lock_name(target); - printk(KERN_CONT ");\n"); - printk(" lock("); - __print_lock_name(parent); - printk(KERN_CONT ");\n"); - printk(" lock("); - __print_lock_name(target); - printk(KERN_CONT ");\n"); - printk(" lock("); - __print_lock_name(source); - printk(KERN_CONT ");\n"); - printk("\n *** DEADLOCK ***\n\n"); + if (cross_lock(tgt->instance)) { + printk(" Possible unsafe locking scenario by crosslock:\n\n"); + printk(" CPU0 CPU1\n"); + printk(" ---- ----\n"); + printk(" lock("); + __print_lock_name(parent); + printk(KERN_CONT ");\n"); + printk(" lock("); + __print_lock_name(target); + printk(KERN_CONT ");\n"); + printk(" lock("); + __print_lock_name(source); + printk(KERN_CONT ");\n"); + printk(" unlock("); + __print_lock_name(target); + printk(KERN_CONT ");\n"); + printk("\n *** DEADLOCK ***\n\n"); + } else { + printk(" Possible unsafe locking scenario:\n\n"); + printk(" CPU0 CPU1\n"); + printk(" ---- ----\n"); + printk(" lock("); + __print_lock_name(target); + printk(KERN_CONT ");\n"); + printk(" lock("); + __print_lock_name(parent); + printk(KERN_CONT ");\n"); + printk(" lock("); + __print_lock_name(target); + printk(KERN_CONT ");\n"); + printk(" lock("); + __print_lock_name(source); + printk(KERN_CONT ");\n"); + printk("\n *** DEADLOCK ***\n\n"); + } } /* @@ -1166,7 +1214,10 @@ print_circular_bug_header(struct lock_list *entry, unsigned int depth, curr->comm, task_pid_nr(curr)); print_lock(check_src); - pr_warn("\nbut task is already holding lock:\n"); + if (cross_lock(check_tgt->instance)) + pr_warn("\nbut now in release context of a crosslock acquired at the following:\n"); + else + pr_warn("\nbut task is already holding lock:\n"); print_lock(check_tgt); pr_warn("\nwhich lock already depends on the new lock.\n\n"); @@ -1196,7 +1247,9 @@ static noinline int print_circular_bug(struct lock_list *this, if (!debug_locks_off_graph_unlock() || debug_locks_silent) return 0; - if (!save_trace(&this->trace)) + if (cross_lock(check_tgt->instance)) + this->trace = *trace; + else if (!save_trace(&this->trace)) return 0; depth = get_lock_depth(target); @@ -1800,6 +1853,9 @@ check_deadlock(struct task_struct *curr, struct held_lock *next, if (nest) return 2; + if (cross_lock(prev->instance)) + continue; + return print_deadlock_bug(curr, prev, next); } return 1; @@ -1965,26 +2021,31 @@ check_prevs_add(struct task_struct *curr, struct held_lock *next) for (;;) { int distance = curr->lockdep_depth - depth + 1; hlock = curr->held_locks + depth - 1; - /* - * Only non-recursive-read entries get new dependencies - * added: + * Only non-crosslock entries get new dependencies added. + * Crosslock entries will be added by commit later: */ - if (hlock->read != 2 && hlock->check) { - int ret = check_prev_add(curr, hlock, next, distance, &trace, save_trace); - if (!ret) - return 0; - + if (!cross_lock(hlock->instance)) { /* - * Stop after the first non-trylock entry, - * as non-trylock entries have added their - * own direct dependencies already, so this - * lock is connected to them indirectly: + * Only non-recursive-read entries get new dependencies + * added: */ - if (!hlock->trylock) - break; - } + if (hlock->read != 2 && hlock->check) { + int ret = check_prev_add(curr, hlock, next, + distance, &trace, save_trace); + if (!ret) + return 0; + /* + * Stop after the first non-trylock entry, + * as non-trylock entries have added their + * own direct dependencies already, so this + * lock is connected to them indirectly: + */ + if (!hlock->trylock) + break; + } + } depth--; /* * End of lock-stack? @@ -3216,10 +3277,21 @@ static void __lockdep_init_map(struct lockdep_map *lock, const char *name, void lockdep_init_map(struct lockdep_map *lock, const char *name, struct lock_class_key *key, int subclass) { + cross_init(lock, 0); __lockdep_init_map(lock, name, key, subclass); } EXPORT_SYMBOL_GPL(lockdep_init_map); +#ifdef CONFIG_LOCKDEP_CROSSRELEASE +void lockdep_init_map_crosslock(struct lockdep_map *lock, const char *name, + struct lock_class_key *key, int subclass) +{ + cross_init(lock, 1); + __lockdep_init_map(lock, name, key, subclass); +} +EXPORT_SYMBOL_GPL(lockdep_init_map_crosslock); +#endif + struct lock_class_key __lockdep_no_validate__; EXPORT_SYMBOL_GPL(__lockdep_no_validate__); @@ -3275,6 +3347,7 @@ static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass, int chain_head = 0; int class_idx; u64 chain_key; + int ret; if (unlikely(!debug_locks)) return 0; @@ -3323,7 +3396,8 @@ static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass, class_idx = class - lock_classes + 1; - if (depth) { + /* TODO: nest_lock is not implemented for crosslock yet. */ + if (depth && !cross_lock(lock)) { hlock = curr->held_locks + depth - 1; if (hlock->class_idx == class_idx && nest_lock) { if (hlock->references) { @@ -3411,6 +3485,14 @@ static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass, if (!validate_chain(curr, lock, hlock, chain_head, chain_key)) return 0; + ret = lock_acquire_crosslock(hlock); + /* + * 2 means normal acquire operations are needed. Otherwise, it's + * ok just to return with '0:fail, 1:success'. + */ + if (ret != 2) + return ret; + curr->curr_chain_key = chain_key; curr->lockdep_depth++; check_chain_key(curr); @@ -3649,11 +3731,19 @@ __lock_release(struct lockdep_map *lock, int nested, unsigned long ip) struct task_struct *curr = current; struct held_lock *hlock; unsigned int depth; - int i; + int ret, i; if (unlikely(!debug_locks)) return 0; + ret = lock_release_crosslock(lock); + /* + * 2 means normal release operations are needed. Otherwise, it's + * ok just to return with '0:fail, 1:success'. + */ + if (ret != 2) + return ret; + depth = curr->lockdep_depth; /* * So we're all set to release this lock.. wait what lock? We don't @@ -4536,3 +4626,495 @@ void lockdep_rcu_suspicious(const char *file, const int line, const char *s) dump_stack(); } EXPORT_SYMBOL_GPL(lockdep_rcu_suspicious); + +#ifdef CONFIG_LOCKDEP_CROSSRELEASE + +/* + * Crossrelease works by recording a lock history for each thread and + * connecting those historic locks that were taken after the + * wait_for_completion() in the complete() context. + * + * Task-A Task-B + * + * mutex_lock(&A); + * mutex_unlock(&A); + * + * wait_for_completion(&C); + * lock_acquire_crosslock(); + * atomic_inc_return(&cross_gen_id); + * | + * | mutex_lock(&B); + * | mutex_unlock(&B); + * | + * | complete(&C); + * `-- lock_commit_crosslock(); + * + * Which will then add a dependency between B and C. + */ + +#define xhlock(i) (current->xhlocks[(i) % MAX_XHLOCKS_NR]) + +/* + * Whenever a crosslock is held, cross_gen_id will be increased. + */ +static atomic_t cross_gen_id; /* Can be wrapped */ + +/* + * Make an entry of the ring buffer invalid. + */ +static inline void invalidate_xhlock(struct hist_lock *xhlock) +{ + /* + * Normally, xhlock->hlock.instance must be !NULL. + */ + xhlock->hlock.instance = NULL; +} + +/* + * Lock history stacks; we have 2 nested lock history stacks: + * + * HARD(IRQ) + * SOFT(IRQ) + * + * The thing is that once we complete a HARD/SOFT IRQ the future task locks + * should not depend on any of the locks observed while running the IRQ. So + * what we do is rewind the history buffer and erase all our knowledge of that + * temporal event. + */ + +void crossrelease_hist_start(enum xhlock_context_t c) +{ + struct task_struct *cur = current; + + if (!cur->xhlocks) + return; + + cur->xhlock_idx_hist[c] = cur->xhlock_idx; + cur->hist_id_save[c] = cur->hist_id; +} + +void crossrelease_hist_end(enum xhlock_context_t c) +{ + struct task_struct *cur = current; + + if (cur->xhlocks) { + unsigned int idx = cur->xhlock_idx_hist[c]; + struct hist_lock *h = &xhlock(idx); + + cur->xhlock_idx = idx; + + /* Check if the ring was overwritten. */ + if (h->hist_id != cur->hist_id_save[c]) + invalidate_xhlock(h); + } +} + +/* + * lockdep_invariant_state() is used to annotate independence inside a task, to + * make one task look like multiple independent 'tasks'. + * + * Take for instance workqueues; each work is independent of the last. The + * completion of a future work does not depend on the completion of a past work + * (in general). Therefore we must not carry that (lock) dependency across + * works. + * + * This is true for many things; pretty much all kthreads fall into this + * pattern, where they have an invariant state and future completions do not + * depend on past completions. Its just that since they all have the 'same' + * form -- the kthread does the same over and over -- it doesn't typically + * matter. + * + * The same is true for system-calls, once a system call is completed (we've + * returned to userspace) the next system call does not depend on the lock + * history of the previous system call. + * + * They key property for independence, this invariant state, is that it must be + * a point where we hold no locks and have no history. Because if we were to + * hold locks, the restore at _end() would not necessarily recover it's history + * entry. Similarly, independence per-definition means it does not depend on + * prior state. + */ +void lockdep_invariant_state(bool force) +{ + /* + * We call this at an invariant point, no current state, no history. + * Verify the former, enforce the latter. + */ + WARN_ON_ONCE(!force && current->lockdep_depth); + if (current->xhlocks) + invalidate_xhlock(&xhlock(current->xhlock_idx)); +} + +static int cross_lock(struct lockdep_map *lock) +{ + return lock ? lock->cross : 0; +} + +/* + * This is needed to decide the relationship between wrapable variables. + */ +static inline int before(unsigned int a, unsigned int b) +{ + return (int)(a - b) < 0; +} + +static inline struct lock_class *xhlock_class(struct hist_lock *xhlock) +{ + return hlock_class(&xhlock->hlock); +} + +static inline struct lock_class *xlock_class(struct cross_lock *xlock) +{ + return hlock_class(&xlock->hlock); +} + +/* + * Should we check a dependency with previous one? + */ +static inline int depend_before(struct held_lock *hlock) +{ + return hlock->read != 2 && hlock->check && !hlock->trylock; +} + +/* + * Should we check a dependency with next one? + */ +static inline int depend_after(struct held_lock *hlock) +{ + return hlock->read != 2 && hlock->check; +} + +/* + * Check if the xhlock is valid, which would be false if, + * + * 1. Has not used after initializaion yet. + * 2. Got invalidated. + * + * Remind hist_lock is implemented as a ring buffer. + */ +static inline int xhlock_valid(struct hist_lock *xhlock) +{ + /* + * xhlock->hlock.instance must be !NULL. + */ + return !!xhlock->hlock.instance; +} + +/* + * Record a hist_lock entry. + * + * Irq disable is only required. + */ +static void add_xhlock(struct held_lock *hlock) +{ + unsigned int idx = ++current->xhlock_idx; + struct hist_lock *xhlock = &xhlock(idx); + +#ifdef CONFIG_DEBUG_LOCKDEP + /* + * This can be done locklessly because they are all task-local + * state, we must however ensure IRQs are disabled. + */ + WARN_ON_ONCE(!irqs_disabled()); +#endif + + /* Initialize hist_lock's members */ + xhlock->hlock = *hlock; + xhlock->hist_id = ++current->hist_id; + + xhlock->trace.nr_entries = 0; + xhlock->trace.max_entries = MAX_XHLOCK_TRACE_ENTRIES; + xhlock->trace.entries = xhlock->trace_entries; + + if (crossrelease_fullstack) { + xhlock->trace.skip = 3; + save_stack_trace(&xhlock->trace); + } else { + xhlock->trace.nr_entries = 1; + xhlock->trace.entries[0] = hlock->acquire_ip; + } +} + +static inline int same_context_xhlock(struct hist_lock *xhlock) +{ + return xhlock->hlock.irq_context == task_irq_context(current); +} + +/* + * This should be lockless as far as possible because this would be + * called very frequently. + */ +static void check_add_xhlock(struct held_lock *hlock) +{ + /* + * Record a hist_lock, only in case that acquisitions ahead + * could depend on the held_lock. For example, if the held_lock + * is trylock then acquisitions ahead never depends on that. + * In that case, we don't need to record it. Just return. + */ + if (!current->xhlocks || !depend_before(hlock)) + return; + + add_xhlock(hlock); +} + +/* + * For crosslock. + */ +static int add_xlock(struct held_lock *hlock) +{ + struct cross_lock *xlock; + unsigned int gen_id; + + if (!graph_lock()) + return 0; + + xlock = &((struct lockdep_map_cross *)hlock->instance)->xlock; + + /* + * When acquisitions for a crosslock are overlapped, we use + * nr_acquire to perform commit for them, based on cross_gen_id + * of the first acquisition, which allows to add additional + * dependencies. + * + * Moreover, when no acquisition of a crosslock is in progress, + * we should not perform commit because the lock might not exist + * any more, which might cause incorrect memory access. So we + * have to track the number of acquisitions of a crosslock. + * + * depend_after() is necessary to initialize only the first + * valid xlock so that the xlock can be used on its commit. + */ + if (xlock->nr_acquire++ && depend_after(&xlock->hlock)) + goto unlock; + + gen_id = (unsigned int)atomic_inc_return(&cross_gen_id); + xlock->hlock = *hlock; + xlock->hlock.gen_id = gen_id; +unlock: + graph_unlock(); + return 1; +} + +/* + * Called for both normal and crosslock acquires. Normal locks will be + * pushed on the hist_lock queue. Cross locks will record state and + * stop regular lock_acquire() to avoid being placed on the held_lock + * stack. + * + * Return: 0 - failure; + * 1 - crosslock, done; + * 2 - normal lock, continue to held_lock[] ops. + */ +static int lock_acquire_crosslock(struct held_lock *hlock) +{ + /* + * CONTEXT 1 CONTEXT 2 + * --------- --------- + * lock A (cross) + * X = atomic_inc_return(&cross_gen_id) + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + * Y = atomic_read_acquire(&cross_gen_id) + * lock B + * + * atomic_read_acquire() is for ordering between A and B, + * IOW, A happens before B, when CONTEXT 2 see Y >= X. + * + * Pairs with atomic_inc_return() in add_xlock(). + */ + hlock->gen_id = (unsigned int)atomic_read_acquire(&cross_gen_id); + + if (cross_lock(hlock->instance)) + return add_xlock(hlock); + + check_add_xhlock(hlock); + return 2; +} + +static int copy_trace(struct stack_trace *trace) +{ + unsigned long *buf = stack_trace + nr_stack_trace_entries; + unsigned int max_nr = MAX_STACK_TRACE_ENTRIES - nr_stack_trace_entries; + unsigned int nr = min(max_nr, trace->nr_entries); + + trace->nr_entries = nr; + memcpy(buf, trace->entries, nr * sizeof(trace->entries[0])); + trace->entries = buf; + nr_stack_trace_entries += nr; + + if (nr_stack_trace_entries >= MAX_STACK_TRACE_ENTRIES-1) { + if (!debug_locks_off_graph_unlock()) + return 0; + + print_lockdep_off("BUG: MAX_STACK_TRACE_ENTRIES too low!"); + dump_stack(); + + return 0; + } + + return 1; +} + +static int commit_xhlock(struct cross_lock *xlock, struct hist_lock *xhlock) +{ + unsigned int xid, pid; + u64 chain_key; + + xid = xlock_class(xlock) - lock_classes; + chain_key = iterate_chain_key((u64)0, xid); + pid = xhlock_class(xhlock) - lock_classes; + chain_key = iterate_chain_key(chain_key, pid); + + if (lookup_chain_cache(chain_key)) + return 1; + + if (!add_chain_cache_classes(xid, pid, xhlock->hlock.irq_context, + chain_key)) + return 0; + + if (!check_prev_add(current, &xlock->hlock, &xhlock->hlock, 1, + &xhlock->trace, copy_trace)) + return 0; + + return 1; +} + +static void commit_xhlocks(struct cross_lock *xlock) +{ + unsigned int cur = current->xhlock_idx; + unsigned int prev_hist_id = xhlock(cur).hist_id; + unsigned int i; + + if (!graph_lock()) + return; + + if (xlock->nr_acquire) { + for (i = 0; i < MAX_XHLOCKS_NR; i++) { + struct hist_lock *xhlock = &xhlock(cur - i); + + if (!xhlock_valid(xhlock)) + break; + + if (before(xhlock->hlock.gen_id, xlock->hlock.gen_id)) + break; + + if (!same_context_xhlock(xhlock)) + break; + + /* + * Filter out the cases where the ring buffer was + * overwritten and the current entry has a bigger + * hist_id than the previous one, which is impossible + * otherwise: + */ + if (unlikely(before(prev_hist_id, xhlock->hist_id))) + break; + + prev_hist_id = xhlock->hist_id; + + /* + * commit_xhlock() returns 0 with graph_lock already + * released if fail. + */ + if (!commit_xhlock(xlock, xhlock)) + return; + } + } + + graph_unlock(); +} + +void lock_commit_crosslock(struct lockdep_map *lock) +{ + struct cross_lock *xlock; + unsigned long flags; + + if (unlikely(!debug_locks || current->lockdep_recursion)) + return; + + if (!current->xhlocks) + return; + + /* + * Do commit hist_locks with the cross_lock, only in case that + * the cross_lock could depend on acquisitions after that. + * + * For example, if the cross_lock does not have the 'check' flag + * then we don't need to check dependencies and commit for that. + * Just skip it. In that case, of course, the cross_lock does + * not depend on acquisitions ahead, either. + * + * WARNING: Don't do that in add_xlock() in advance. When an + * acquisition context is different from the commit context, + * invalid(skipped) cross_lock might be accessed. + */ + if (!depend_after(&((struct lockdep_map_cross *)lock)->xlock.hlock)) + return; + + raw_local_irq_save(flags); + check_flags(flags); + current->lockdep_recursion = 1; + xlock = &((struct lockdep_map_cross *)lock)->xlock; + commit_xhlocks(xlock); + current->lockdep_recursion = 0; + raw_local_irq_restore(flags); +} +EXPORT_SYMBOL_GPL(lock_commit_crosslock); + +/* + * Return: 0 - failure; + * 1 - crosslock, done; + * 2 - normal lock, continue to held_lock[] ops. + */ +static int lock_release_crosslock(struct lockdep_map *lock) +{ + if (cross_lock(lock)) { + if (!graph_lock()) + return 0; + ((struct lockdep_map_cross *)lock)->xlock.nr_acquire--; + graph_unlock(); + return 1; + } + return 2; +} + +static void cross_init(struct lockdep_map *lock, int cross) +{ + if (cross) + ((struct lockdep_map_cross *)lock)->xlock.nr_acquire = 0; + + lock->cross = cross; + + /* + * Crossrelease assumes that the ring buffer size of xhlocks + * is aligned with power of 2. So force it on build. + */ + BUILD_BUG_ON(MAX_XHLOCKS_NR & (MAX_XHLOCKS_NR - 1)); +} + +void lockdep_init_task(struct task_struct *task) +{ + int i; + + task->xhlock_idx = UINT_MAX; + task->hist_id = 0; + + for (i = 0; i < XHLOCK_CTX_NR; i++) { + task->xhlock_idx_hist[i] = UINT_MAX; + task->hist_id_save[i] = 0; + } + + task->xhlocks = kzalloc(sizeof(struct hist_lock) * MAX_XHLOCKS_NR, + GFP_KERNEL); +} + +void lockdep_free_task(struct task_struct *task) +{ + if (task->xhlocks) { + void *tmp = task->xhlocks; + /* Diable crossrelease for current */ + task->xhlocks = NULL; + kfree(tmp); + } +} +#endif diff --git a/kernel/sched/completion.c b/kernel/sched/completion.c index a1ad5b7d5521..0f7c442f9dd7 100644 --- a/kernel/sched/completion.c +++ b/kernel/sched/completion.c @@ -31,6 +31,11 @@ void complete(struct completion *x) spin_lock_irqsave(&x->wait.lock, flags); + /* + * Perform commit of crossrelease here. + */ + complete_release_commit(x); + if (x->done != UINT_MAX) x->done++; __wake_up_locked(&x->wait, TASK_NORMAL, 1); diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 1ead06829fdb..fbfdf7c263d2 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1055,6 +1055,8 @@ config PROVE_LOCKING select DEBUG_RWSEMS if RWSEM_SPIN_ON_OWNER select DEBUG_WW_MUTEX_SLOWPATH select DEBUG_LOCK_ALLOC + select LOCKDEP_CROSSRELEASE + select LOCKDEP_COMPLETIONS select TRACE_IRQFLAGS default n help @@ -1187,6 +1189,37 @@ config LOCKDEP config LOCKDEP_SMALL bool +config LOCKDEP_CROSSRELEASE + bool + help + This makes lockdep work for crosslock which is a lock allowed to + be released in a different context from the acquisition context. + Normally a lock must be released in the context acquiring the lock. + However, relexing this constraint helps synchronization primitives + such as page locks or completions can use the lock correctness + detector, lockdep. + +config LOCKDEP_COMPLETIONS + bool + help + A deadlock caused by wait_for_completion() and complete() can be + detected by lockdep using crossrelease feature. + +config BOOTPARAM_LOCKDEP_CROSSRELEASE_FULLSTACK + bool "Enable the boot parameter, crossrelease_fullstack" + depends on LOCKDEP_CROSSRELEASE + default n + help + The lockdep "cross-release" feature needs to record stack traces + (of calling functions) for all acquisitions, for eventual later + use during analysis. By default only a single caller is recorded, + because the unwind operation can be very expensive with deeper + stack chains. + + However a boot parameter, crossrelease_fullstack, was + introduced since sometimes deeper traces are required for full + analysis. This option turns on the boot parameter. + config DEBUG_LOCKDEP bool "Lock dependency engine debugging" depends on DEBUG_KERNEL && LOCKDEP From patchwork Fri Nov 2 09:15:21 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 10665179 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 6952615E9 for ; Fri, 2 Nov 2018 09:15:46 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 563542BA63 for ; Fri, 2 Nov 2018 09:15:46 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4A5EE2BA84; Fri, 2 Nov 2018 09:15:46 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 341792BA63 for ; Fri, 2 Nov 2018 09:15:45 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id CFDE86E54D; Fri, 2 Nov 2018 09:15:43 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ed1-x543.google.com (mail-ed1-x543.google.com [IPv6:2a00:1450:4864:20::543]) by gabe.freedesktop.org (Postfix) with ESMTPS id 86B776E542 for ; Fri, 2 Nov 2018 09:15:42 +0000 (UTC) Received: by mail-ed1-x543.google.com with SMTP id y20-v6so1268499eds.10 for ; Fri, 02 Nov 2018 02:15:42 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=1W7bOKN7lGjKY9PT+sjxcZ4tw0LsJaIf5qAGHwI2zcE=; b=UTk4JjxWA4bi4T9dFhQnXT3Jzm4QEUnKymB93BJE9/tW0O/PE0mFITtAFJqdZz0I5k GPNfzH9Ch/5vIobVRw+ISF97yLerGOPW7YdnTNA94Y18hJiQw6YrXGmBobF5lDfk+nEj EBRN+vMLQSjWKg3bY2bwuePXfXvWNofhwRbyFyRFpafWy5eVUu6ltQrNJ+Tn8agSBSJ3 QkAfQwJAexPJQBP2XaWrOZy57RQeEhVvNDq52s4aBmIAy13Kh7QesP5z+OHAkoYXNnjc h70u81ap+erQ/Nzxkqcnrm1WZPczsMxxu5/JJCuXl+nVqzSEI1sFWU3kNB+VAA73C1Hd r6oQ== X-Gm-Message-State: AGRZ1gJBYWl+QnC0US6Gf1YpJ7AhoJ8rUIy9xvLD2fdlmWGktmgGLDLy LaypwPhhEInAlX8v/8/d8vyw38fd+gw= X-Google-Smtp-Source: AJdET5elaqk65rBQk9f1B6kLPwgnFv4ZW6inG4GEUH9FZXE9Qp52bqiEN273v9KX8ofzONzl/bvKrg== X-Received: by 2002:a50:a541:: with SMTP id z1-v6mr8128348edb.119.1541150140722; Fri, 02 Nov 2018 02:15:40 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id p19-v6sm5658103ejw.69.2018.11.02.02.15.39 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 02 Nov 2018 02:15:40 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Fri, 2 Nov 2018 10:15:21 +0100 Message-Id: <20181102091531.7775-2-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.19.1 In-Reply-To: <20181102091531.7775-1-daniel.vetter@ffwll.ch> References: <20181102091531.7775-1-daniel.vetter@ffwll.ch> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 02/12] kthread: finer-grained lockdep/cross-release completion X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP When cross release was originally merged we've hit a bunch of lockdep splats around the 2 kthread completions. In all cases they are because totally independent uses of kthread are mixed up by lockdep into the same locking class, creating artificial deadlocks. Fix this by converting kthread code in the same way as e.g. alloc_workqueue already works: Use macros for the public api so we can have a callsite specific lockdep key, then pass that through the entire callchain. Due to the many entry points this is slightly tedious. Cc: Tvrtko Ursulin Cc: Marta Lofstedt Cc: Byungchul Park Cc: Ingo Molnar Cc: Peter Zijlstra Cc: Tejun Heo Cc: Kees Cook Cc: Thomas Gleixner Cc: Shaohua Li Cc: Andrew Morton Cc: Jens Axboe Cc: Daniel Vetter Cc: Greg Kroah-Hartman Cc: Jonathan Corbet Cc: Oleg Nesterov Cc: "Steven Rostedt (VMware)" Cc: Snild Dolkow References: https://bugs.freedesktop.org/show_bug.cgi?id=103950 Signed-off-by: Daniel Vetter --- Peter acked the original patch but then cross-release got removed before this one here landed. -Daniel --- include/linux/kthread.h | 48 ++++++++++++++++++++++++----- kernel/kthread.c | 68 ++++++++++++++++++++++++++++------------- 2 files changed, 88 insertions(+), 28 deletions(-) diff --git a/include/linux/kthread.h b/include/linux/kthread.h index c1961761311d..7a9463f0be5c 100644 --- a/include/linux/kthread.h +++ b/include/linux/kthread.h @@ -6,10 +6,12 @@ #include #include -__printf(4, 5) -struct task_struct *kthread_create_on_node(int (*threadfn)(void *data), +__printf(6, 7) +struct task_struct *_kthread_create_on_node(int (*threadfn)(void *data), void *data, int node, + struct lock_class_key *exited_key, + struct lock_class_key *parked_key, const char namefmt[], ...); /** @@ -25,12 +27,27 @@ struct task_struct *kthread_create_on_node(int (*threadfn)(void *data), */ #define kthread_create(threadfn, data, namefmt, arg...) \ kthread_create_on_node(threadfn, data, NUMA_NO_NODE, namefmt, ##arg) +#define kthread_create_on_node(threadfn, data, node, namefmt, arg...) \ +({ \ + static struct lock_class_key __exited_key, __parked_key; \ + _kthread_create_on_node(threadfn, data, node, &__exited_key, \ + &__parked_key, namefmt, ##arg); \ +}) -struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data), +struct task_struct *_kthread_create_on_cpu(int (*threadfn)(void *data), void *data, unsigned int cpu, + struct lock_class_key *exited_key, + struct lock_class_key *parked_key, const char *namefmt); +#define kthread_create_on_cpu(threadfn, data, cpu, namefmt) \ +({ \ + static struct lock_class_key __exited_key, __parked_key; \ + _kthread_create_on_cpu(threadfn, data, cpu, &__exited_key,\ + &__parked_key, namefmt); \ +}) + /** * kthread_run - create and wake a thread. @@ -171,13 +188,30 @@ extern void __kthread_init_worker(struct kthread_worker *worker, int kthread_worker_fn(void *worker_ptr); -__printf(2, 3) +__printf(4, 5) struct kthread_worker * -kthread_create_worker(unsigned int flags, const char namefmt[], ...); +_kthread_create_worker(unsigned int flags, + struct lock_class_key *exited_key, + struct lock_class_key *parked_key, + const char namefmt[], ...); +#define kthread_create_worker(flags, namefmt...) \ +({ \ + static struct lock_class_key __exited_key, __parked_key; \ + _kthread_create_worker(flags, &__exited_key, &__parked_key, \ + ##namefmt); \ +}) -__printf(3, 4) struct kthread_worker * -kthread_create_worker_on_cpu(int cpu, unsigned int flags, +__printf(5, 6) struct kthread_worker * +_kthread_create_worker_on_cpu(int cpu, unsigned int flags, + struct lock_class_key *exited_key, + struct lock_class_key *parked_key, const char namefmt[], ...); +#define kthread_create_worker_on_cpu(cpu, flags, namefmt...) \ +({ \ + static struct lock_class_key __exited_key, __parked_key; \ + _kthread_create_worker_on_cpu(cpu, flags, &__exited_key, &__parked_key,\ + ##namefmt); \ +}) bool kthread_queue_work(struct kthread_worker *worker, struct kthread_work *work); diff --git a/kernel/kthread.c b/kernel/kthread.c index 087d18d771b5..64c0464a17dd 100644 --- a/kernel/kthread.c +++ b/kernel/kthread.c @@ -32,6 +32,7 @@ struct kthread_create_info int (*threadfn)(void *data); void *data; int node; + struct lock_class_key *exited_key, *parked_key; /* Result passed back to kthread_create() from kthreadd. */ struct task_struct *result; @@ -229,8 +230,15 @@ static int kthread(void *_create) } self->data = data; - init_completion(&self->exited); - init_completion(&self->parked); + lockdep_init_map_crosslock(&self->exited.map.map, + "(kthread completion)->exited", + create->exited_key, 0); + init_completion_map(&self->exited, &self->exited.map.map); + lockdep_init_map_crosslock(&self->parked.map.map, + "(kthread completion)->parked", + create->parked_key, 0); + init_completion_map(&self->parked, &self->exited.map.map); + current->vfork_done = &self->exited; /* OK, tell user we're spawned, wait for stop or wakeup */ @@ -280,9 +288,11 @@ static void create_kthread(struct kthread_create_info *create) } } -static __printf(4, 0) +static __printf(6, 0) struct task_struct *__kthread_create_on_node(int (*threadfn)(void *data), void *data, int node, + struct lock_class_key *exited_key, + struct lock_class_key *parked_key, const char namefmt[], va_list args) { @@ -297,6 +307,8 @@ struct task_struct *__kthread_create_on_node(int (*threadfn)(void *data), create->data = data; create->node = node; create->done = &done; + create->exited_key = exited_key; + create->parked_key = parked_key; spin_lock(&kthread_create_lock); list_add_tail(&create->list, &kthread_create_list); @@ -367,21 +379,24 @@ struct task_struct *__kthread_create_on_node(int (*threadfn)(void *data), * * Returns a task_struct or ERR_PTR(-ENOMEM) or ERR_PTR(-EINTR). */ -struct task_struct *kthread_create_on_node(int (*threadfn)(void *data), - void *data, int node, - const char namefmt[], - ...) +struct task_struct *_kthread_create_on_node(int (*threadfn)(void *data), + void *data, int node, + struct lock_class_key *exited_key, + struct lock_class_key *parked_key, + const char namefmt[], + ...) { struct task_struct *task; va_list args; va_start(args, namefmt); - task = __kthread_create_on_node(threadfn, data, node, namefmt, args); + task = __kthread_create_on_node(threadfn, data, node, + exited_key, parked_key, namefmt, args); va_end(args); return task; } -EXPORT_SYMBOL(kthread_create_on_node); +EXPORT_SYMBOL(_kthread_create_on_node); static void __kthread_bind_mask(struct task_struct *p, const struct cpumask *mask, long state) { @@ -435,14 +450,16 @@ EXPORT_SYMBOL(kthread_bind); * Description: This helper function creates and names a kernel thread * The thread will be woken and put into park mode. */ -struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data), +struct task_struct *_kthread_create_on_cpu(int (*threadfn)(void *data), void *data, unsigned int cpu, + struct lock_class_key *exited_key, + struct lock_class_key *parked_key, const char *namefmt) { struct task_struct *p; - p = kthread_create_on_node(threadfn, data, cpu_to_node(cpu), namefmt, - cpu); + p = _kthread_create_on_node(threadfn, data, cpu_to_node(cpu), + exited_key, parked_key, namefmt, cpu); if (IS_ERR(p)) return p; kthread_bind(p, cpu); @@ -669,8 +686,10 @@ int kthread_worker_fn(void *worker_ptr) } EXPORT_SYMBOL_GPL(kthread_worker_fn); -static __printf(3, 0) struct kthread_worker * +static __printf(5, 0) struct kthread_worker * __kthread_create_worker(int cpu, unsigned int flags, + struct lock_class_key *exited_key, + struct lock_class_key *parked_key, const char namefmt[], va_list args) { struct kthread_worker *worker; @@ -686,8 +705,8 @@ __kthread_create_worker(int cpu, unsigned int flags, if (cpu >= 0) node = cpu_to_node(cpu); - task = __kthread_create_on_node(kthread_worker_fn, worker, - node, namefmt, args); + task = __kthread_create_on_node(kthread_worker_fn, worker, node, + exited_key, parked_key, namefmt, args); if (IS_ERR(task)) goto fail_task; @@ -714,18 +733,22 @@ __kthread_create_worker(int cpu, unsigned int flags, * when the worker was SIGKILLed. */ struct kthread_worker * -kthread_create_worker(unsigned int flags, const char namefmt[], ...) +_kthread_create_worker(unsigned int flags, + struct lock_class_key *exited_key, + struct lock_class_key *parked_key, + const char namefmt[], ...) { struct kthread_worker *worker; va_list args; va_start(args, namefmt); - worker = __kthread_create_worker(-1, flags, namefmt, args); + worker = __kthread_create_worker(-1, flags, exited_key, parked_key, + namefmt, args); va_end(args); return worker; } -EXPORT_SYMBOL(kthread_create_worker); +EXPORT_SYMBOL(_kthread_create_worker); /** * kthread_create_worker_on_cpu - create a kthread worker and bind it @@ -745,19 +768,22 @@ EXPORT_SYMBOL(kthread_create_worker); * when the worker was SIGKILLed. */ struct kthread_worker * -kthread_create_worker_on_cpu(int cpu, unsigned int flags, +_kthread_create_worker_on_cpu(int cpu, unsigned int flags, + struct lock_class_key *exited_key, + struct lock_class_key *parked_key, const char namefmt[], ...) { struct kthread_worker *worker; va_list args; va_start(args, namefmt); - worker = __kthread_create_worker(cpu, flags, namefmt, args); + worker = __kthread_create_worker(cpu, flags, exited_key, parked_key, + namefmt, args); va_end(args); return worker; } -EXPORT_SYMBOL(kthread_create_worker_on_cpu); +EXPORT_SYMBOL(_kthread_create_worker_on_cpu); /* * Returns true when the work could not be queued at the moment. From patchwork Fri Nov 2 09:15:22 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 10665181 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 97E3613B5 for ; Fri, 2 Nov 2018 09:15:47 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 859212BA63 for ; Fri, 2 Nov 2018 09:15:47 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 79B5F2BA84; Fri, 2 Nov 2018 09:15:47 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id EF7592BA63 for ; Fri, 2 Nov 2018 09:15:46 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1F8456E546; Fri, 2 Nov 2018 09:15:46 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ed1-x541.google.com (mail-ed1-x541.google.com [IPv6:2a00:1450:4864:20::541]) by gabe.freedesktop.org (Postfix) with ESMTPS id C0DAA6E546 for ; Fri, 2 Nov 2018 09:15:43 +0000 (UTC) Received: by mail-ed1-x541.google.com with SMTP id c25-v6so1276496edt.8 for ; Fri, 02 Nov 2018 02:15:43 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=TG4+0G/BArGLSJFBALClDGfolPpoWn6PmRENQ5esAWY=; b=bNna8HT7F6hYCwrIQrlJa6ja+2AKSgYqjaF/EtgR59bC9Ji5iWlOUE0Xs1KAlU3pzi 0DYulR5jRshHbcfcMjKQEWVzXuD9/qktjiOL72VWHPG162KuasY/RdF5q7yUS5hA9qpk SX42tcEMMlP0zpTCvD/+DDkPF2k4JJN64wXwSgG7O/k/cSpeNctiBRvhnMaGgsRQXI3o S0o1+WY2TFbcYQw5oKsnZLs3NwxpBQ+avbtUFW6rObn6b7cL8rYEE9nrceYMFF4MXwae /y9vh49j+HXxQeB2CBnRaWxeld3JARJ8v4RhsIi1+hDDxyxpJPjdc6rrNEHMJ6viALmu 94BQ== X-Gm-Message-State: AGRZ1gI9yS5VaCmEAiqpb2Ld5TpVoeDugOpx3nnvMD5qWOn3LAd/rqZ9 L5ZaRULKD80/mGT5mH0IJVcXgs3iIBU= X-Google-Smtp-Source: AJdET5d+eqegwY9fuCx2P1JKd3n/3NNTXE8Z28TyQR35hVvZ7GHkT1YzwYMJauc7K5ShtpGKsvD4vA== X-Received: by 2002:a17:906:90d9:: with SMTP id v25-v6mr6118568ejw.214.1541150142067; Fri, 02 Nov 2018 02:15:42 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id p19-v6sm5658103ejw.69.2018.11.02.02.15.40 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 02 Nov 2018 02:15:40 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Fri, 2 Nov 2018 10:15:22 +0100 Message-Id: <20181102091531.7775-3-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.19.1 In-Reply-To: <20181102091531.7775-1-daniel.vetter@ffwll.ch> References: <20181102091531.7775-1-daniel.vetter@ffwll.ch> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 03/12] lockdep: Remove GFP_NOLOCKDEP annotation X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP This was originally added in 7e7844226f10 ("lockdep: allow to disable reclaim lockup detection") but a git log -G "GFP_NOLOCKDEP" indicates it was never used. Cc: Andrew Morton Cc: Vlastimil Babka Cc: Michal Hocko Cc: Mel Gorman Cc: Daniel Vetter Cc: "Levin, Alexander (Sasha Levin)" Cc: Mike Rapoport Cc: Huaisheng Ye Cc: Pavel Tatashin Cc: Aaron Lu Cc: Oscar Salvador Cc: Joonsoo Kim Cc: linux-mm@kvack.org Cc: Dave Chinner Cc: Peter Zijlstra Cc: Shakeel Butt Signed-off-by: Daniel Vetter --- include/linux/gfp.h | 10 +--------- mm/page_alloc.c | 3 --- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/include/linux/gfp.h b/include/linux/gfp.h index 24bcc5eec6b4..7709106d87fd 100644 --- a/include/linux/gfp.h +++ b/include/linux/gfp.h @@ -39,11 +39,6 @@ struct vm_area_struct; #define ___GFP_ACCOUNT 0x100000u #define ___GFP_DIRECT_RECLAIM 0x200000u #define ___GFP_KSWAPD_RECLAIM 0x400000u -#ifdef CONFIG_LOCKDEP -#define ___GFP_NOLOCKDEP 0x800000u -#else -#define ___GFP_NOLOCKDEP 0 -#endif /* If the above are modified, __GFP_BITS_SHIFT may need updating */ /* @@ -213,11 +208,8 @@ struct vm_area_struct; #define __GFP_COMP ((__force gfp_t)___GFP_COMP) #define __GFP_ZERO ((__force gfp_t)___GFP_ZERO) -/* Disable lockdep for GFP context tracking */ -#define __GFP_NOLOCKDEP ((__force gfp_t)___GFP_NOLOCKDEP) - /* Room for N __GFP_FOO bits */ -#define __GFP_BITS_SHIFT (23 + IS_ENABLED(CONFIG_LOCKDEP)) +#define __GFP_BITS_SHIFT 23 #define __GFP_BITS_MASK ((__force gfp_t)((1 << __GFP_BITS_SHIFT) - 1)) /** diff --git a/mm/page_alloc.c b/mm/page_alloc.c index e2ef1c17942f..b238ab7c1c0d 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -3717,9 +3717,6 @@ static bool __need_fs_reclaim(gfp_t gfp_mask) if (!(gfp_mask & __GFP_FS)) return false; - if (gfp_mask & __GFP_NOLOCKDEP) - return false; - return true; } From patchwork Fri Nov 2 09:15:23 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 10665183 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A25B213B5 for ; Fri, 2 Nov 2018 09:15:48 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 909B52BA63 for ; Fri, 2 Nov 2018 09:15:48 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 84E632BA84; Fri, 2 Nov 2018 09:15:48 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id F1E492BA63 for ; Fri, 2 Nov 2018 09:15:47 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 28AFC6E551; Fri, 2 Nov 2018 09:15:46 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ed1-x542.google.com (mail-ed1-x542.google.com [IPv6:2a00:1450:4864:20::542]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2575A6E542 for ; Fri, 2 Nov 2018 09:15:45 +0000 (UTC) Received: by mail-ed1-x542.google.com with SMTP id h21-v6so841358edq.9 for ; Fri, 02 Nov 2018 02:15:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=7r8JcOoypJMBDi2yYbzHpwWac/QFtzC/cNWuGiktntw=; b=DVqHmByt9LoSw/whMPQKUtK6FMRO75pq51b6x5yNeulii4srQH6WvBa5H/aedFnT5H I5nh1KkIHgdDxM97lRevE2u/ejzhmDLTv9Of0mP0iWm4fj7yFduZL0/iAvxScrhjbDLf 62dx1of8WTU3AqFFn9NJaKspuXtr6qfHqLmPpTLOhgmDG4jQW/8Al8QGxKVCdOaSe8Vd K22VLI5mlqj6dFYMlCtoAkmyf9umHGIqRSI2p5aeVnZ94/MwaCDGN4xZd7MGRGQ99QbU BTd9UrnmkGpDbzn0kF9HZwbfX7AAttpFkGw0SopuHsT10Q7iLhLQ+D55HaIkuF+4dXPS ABgQ== X-Gm-Message-State: AGRZ1gL4aYgoWLKePhzBRk9i4nGe6Lk0Ni61eUWRwE79S+tRuQ4QC/tP R5lZrjfxwbxZptkF4hRuVRCyuzJpLQo= X-Google-Smtp-Source: AJdET5cafOxEGEaAmTJbBxznMjSbtb9htz0gUzMuxL6Tj6wM660vBSADnUQRB1CSo2raKyViOYAzDg== X-Received: by 2002:aa7:c2c2:: with SMTP id m2-v6mr7378373edp.79.1541150143289; Fri, 02 Nov 2018 02:15:43 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id p19-v6sm5658103ejw.69.2018.11.02.02.15.42 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 02 Nov 2018 02:15:42 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Fri, 2 Nov 2018 10:15:23 +0100 Message-Id: <20181102091531.7775-4-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.19.1 In-Reply-To: <20181102091531.7775-1-daniel.vetter@ffwll.ch> References: <20181102091531.7775-1-daniel.vetter@ffwll.ch> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 04/12] kernel/lockdep: Make cross-release a config option X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP cross-release annotations will need some serious amounts of vetting before they can be enabled by default, or we'll just annoy everyone. Instead split it into a separate option, which for now stays disabled by default even if you enable overall lockdep. Cc: Andrew Morton Cc: Ingo Molnar Cc: Masahiro Yamada Cc: Arnd Bergmann Cc: Randy Dunlap Cc: Daniel Vetter Cc: Waiman Long Cc: Masami Hiramatsu Cc: Yury Norov Cc: Mikulas Patocka Cc: Robin Murphy Cc: Andy Shevchenko Signed-off-by: Daniel Vetter --- lib/Kconfig.debug | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index fbfdf7c263d2..7e504b7fbbe1 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1055,8 +1055,6 @@ config PROVE_LOCKING select DEBUG_RWSEMS if RWSEM_SPIN_ON_OWNER select DEBUG_WW_MUTEX_SLOWPATH select DEBUG_LOCK_ALLOC - select LOCKDEP_CROSSRELEASE - select LOCKDEP_COMPLETIONS select TRACE_IRQFLAGS default n help @@ -1093,6 +1091,18 @@ config PROVE_LOCKING For more details, see Documentation/locking/lockdep-design.txt. +config LOCKDEP_CROSSRELEASE + bool "Enable cross-release checking" + depends on PROVE_LOCKING + select LOCKDEP_COMPLETIONS + help + This makes lockdep work for crosslock which is a lock allowed to + be released in a different context from the acquisition context. + Normally a lock must be released in the context acquiring the lock. + However, relexing this constraint helps synchronization primitives + such as page locks or completions can use the lock correctness + detector, lockdep. + config LOCK_STAT bool "Lock usage statistics" depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT @@ -1189,16 +1199,6 @@ config LOCKDEP config LOCKDEP_SMALL bool -config LOCKDEP_CROSSRELEASE - bool - help - This makes lockdep work for crosslock which is a lock allowed to - be released in a different context from the acquisition context. - Normally a lock must be released in the context acquiring the lock. - However, relexing this constraint helps synchronization primitives - such as page locks or completions can use the lock correctness - detector, lockdep. - config LOCKDEP_COMPLETIONS bool help From patchwork Fri Nov 2 09:15:24 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 10665187 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id DC3B315E9 for ; Fri, 2 Nov 2018 09:15:51 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CAD4A2BA77 for ; Fri, 2 Nov 2018 09:15:51 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id BF6432BA9B; Fri, 2 Nov 2018 09:15:51 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 6A35E2BA77 for ; Fri, 2 Nov 2018 09:15:51 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C3F9A6E557; Fri, 2 Nov 2018 09:15:49 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ed1-x542.google.com (mail-ed1-x542.google.com [IPv6:2a00:1450:4864:20::542]) by gabe.freedesktop.org (Postfix) with ESMTPS id 4F8816E552 for ; Fri, 2 Nov 2018 09:15:46 +0000 (UTC) Received: by mail-ed1-x542.google.com with SMTP id z12-v6so1284042edp.7 for ; Fri, 02 Nov 2018 02:15:46 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=0hcLgSbnuVeYsG8/H4zoM7531Zi0SnXvmscx9g6zWio=; b=Dh5GsjRFOF0S/PGC2DP02lD+0KyudQ7ItYwEaNJq0Uy3TxysTIuopAA4kXschvqWBq VjSFpggZb+6JZsePxZ9Foc+PlOKD9d18vktl+EJWFDbA/dBKCImCMx5MdEbmymmHeZXK 9tDVgNcCzMgPOi9pdbcTtdVZCItJbDJFnn6jZC4MtTjKoTT7aXS1DUEgsdsIFatioAd+ OLg64jttl9YAvso/x4cZVC/nk7dBm55oCVPRZQD724svx0t3DqVCWQ3/Y6wmHjotWqv6 +4O2tdydL1efIycAUwAky222IsgkUBk2GLxii9s1LkK598Zac3p2kV2C9xtcil5FjeUn N5bw== X-Gm-Message-State: AGRZ1gKSKACTvDxwzuYVu4L1O1zI6yaG0EkM8xpKxyo9AgbEA7jAzbVG csPpT/3VIVm2pMMErSscN/oiiDIxxV4= X-Google-Smtp-Source: AJdET5f7/37ME/hrK3tn1IRvgxHFNAnbMgvCTKojxMFEDIxlUdiV3I13QjHOumNCbM+ejPU0z1/jvA== X-Received: by 2002:aa7:cf1a:: with SMTP id a26-v6mr7688764edy.91.1541150144456; Fri, 02 Nov 2018 02:15:44 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id p19-v6sm5658103ejw.69.2018.11.02.02.15.43 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 02 Nov 2018 02:15:43 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Fri, 2 Nov 2018 10:15:24 +0100 Message-Id: <20181102091531.7775-5-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.19.1 In-Reply-To: <20181102091531.7775-1-daniel.vetter@ffwll.ch> References: <20181102091531.7775-1-daniel.vetter@ffwll.ch> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 05/12] mm: Check if mmu notifier callbacks are allowed to fail X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP Just a bit of paranoia, since if we start pushing this deep into callchains it's hard to spot all places where an mmu notifier implementation might fail when it's not allowed to. Cc: Andrew Morton Cc: Michal Hocko Cc: "Christian König" Cc: David Rientjes Cc: Daniel Vetter Cc: "Jérôme Glisse" Cc: linux-mm@kvack.org Cc: Paolo Bonzini Signed-off-by: Daniel Vetter --- mm/mmu_notifier.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mm/mmu_notifier.c b/mm/mmu_notifier.c index 82bb1a939c0e..744840e5636e 100644 --- a/mm/mmu_notifier.c +++ b/mm/mmu_notifier.c @@ -190,6 +190,8 @@ int __mmu_notifier_invalidate_range_start(struct mm_struct *mm, pr_info("%pS callback failed with %d in %sblockable context.\n", mn->ops->invalidate_range_start, _ret, !blockable ? "non-" : ""); + WARN(blockable,"%pS callback failure not allowed\n", + mn->ops->invalidate_range_start); ret = _ret; } } From patchwork Fri Nov 2 09:15:25 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 10665185 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id DFC2B15E9 for ; Fri, 2 Nov 2018 09:15:50 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CB4E42BA77 for ; Fri, 2 Nov 2018 09:15:50 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id BFF0E2BA9B; Fri, 2 Nov 2018 09:15:50 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 74E802BA77 for ; Fri, 2 Nov 2018 09:15:50 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 7DA856E555; Fri, 2 Nov 2018 09:15:49 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ed1-x543.google.com (mail-ed1-x543.google.com [IPv6:2a00:1450:4864:20::543]) by gabe.freedesktop.org (Postfix) with ESMTPS id 90CA76E552 for ; Fri, 2 Nov 2018 09:15:47 +0000 (UTC) Received: by mail-ed1-x543.google.com with SMTP id f8-v6so1246250edt.13 for ; Fri, 02 Nov 2018 02:15:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=l9RQ5iD4W4W0uIvFZ9Pu7IIeWepUMXVSw/+/Qz476uI=; b=CypHSTjnqq8veiPEN6WfYq0eghopbMugmwCKexM//A+iXSVtbExCZD9D/JzFEK5vrP 5UmOAIrBRhA0qLGqtdx7KXWDkb52dYNzxVnqo9ir4dmR6qaslbLQP6LE5BX+ydvGTMEN TjC4JaEViWtYy9k+8Oe6GEOq9f3v5LUHJ6w+yPoRExP2d+OlbiHG65+9mzF95GOYcrPj Zrtq79R3LLsrbe6b+DyGsv42h5P9u53cyqEgmCdqeav9jRueGqNfXiEwfdlZAe36SyA1 sr3oUPCK2Sro6gmcUDvmaocTNceBAlF9tPNgkevtDjp6m1KCSGfChxoJrWTcFK79fsNI FWAQ== X-Gm-Message-State: AGRZ1gKOxOXI+bBHHm2Tg9Kc+TL5aBYisLUal1V5bnUCJGltrBVFUuaY xrDvgO8cosZTZns0VVsdhSa/CaOt3Ro= X-Google-Smtp-Source: AJdET5cSJoh7KMqEXC+pPZ3XfwEutyAnYoZjLX8w+2gPglDiS+vIa4GVBilOQX8TzxygW3HtiXTmLQ== X-Received: by 2002:a17:906:32d9:: with SMTP id k25-v6mr6195155ejk.211.1541150145766; Fri, 02 Nov 2018 02:15:45 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id p19-v6sm5658103ejw.69.2018.11.02.02.15.44 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 02 Nov 2018 02:15:44 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Fri, 2 Nov 2018 10:15:25 +0100 Message-Id: <20181102091531.7775-6-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.19.1 In-Reply-To: <20181102091531.7775-1-daniel.vetter@ffwll.ch> References: <20181102091531.7775-1-daniel.vetter@ffwll.ch> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 06/12] mm, notifier: Catch sleeping/blocking for !blockable X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP We need to make sure implementations don't cheat and don't have a possible schedule/blocking point deeply burried where review can't catch it. I'm not sure whether this is the best way to make sure all the might_sleep() callsites trigger, and it's a bit ugly in the code flow. But it gets the job done. Cc: Andrew Morton Cc: Michal Hocko Cc: David Rientjes Cc: "Christian König" Cc: Daniel Vetter Cc: "Jérôme Glisse" Cc: linux-mm@kvack.org Signed-off-by: Daniel Vetter --- mm/mmu_notifier.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/mm/mmu_notifier.c b/mm/mmu_notifier.c index 744840e5636e..4d56e2645b6c 100644 --- a/mm/mmu_notifier.c +++ b/mm/mmu_notifier.c @@ -185,7 +185,13 @@ int __mmu_notifier_invalidate_range_start(struct mm_struct *mm, id = srcu_read_lock(&srcu); hlist_for_each_entry_rcu(mn, &mm->mmu_notifier_mm->list, hlist) { if (mn->ops->invalidate_range_start) { - int _ret = mn->ops->invalidate_range_start(mn, mm, start, end, blockable); + int _ret; + + if (IS_ENABLED(CONFIG_DEBUG_ATOMIC_SLEEP) && !blockable) + preempt_disable(); + _ret = mn->ops->invalidate_range_start(mn, mm, start, end, blockable); + if (IS_ENABLED(CONFIG_DEBUG_ATOMIC_SLEEP) && !blockable) + preempt_enable(); if (_ret) { pr_info("%pS callback failed with %d in %sblockable context.\n", mn->ops->invalidate_range_start, _ret, From patchwork Fri Nov 2 09:15:26 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 10665191 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id D7CFF17D5 for ; Fri, 2 Nov 2018 09:15:52 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C60452BA77 for ; Fri, 2 Nov 2018 09:15:52 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B9C732BA63; Fri, 2 Nov 2018 09:15:52 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 53DE52BA63 for ; Fri, 2 Nov 2018 09:15:52 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 5AF1C6E552; Fri, 2 Nov 2018 09:15:51 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ed1-x543.google.com (mail-ed1-x543.google.com [IPv6:2a00:1450:4864:20::543]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6581E6E552 for ; Fri, 2 Nov 2018 09:15:49 +0000 (UTC) Received: by mail-ed1-x543.google.com with SMTP id z12-v6so1284136edp.7 for ; Fri, 02 Nov 2018 02:15:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=ObFErr483tLFfIVUQhf5YQOwkFFpHBdq4sa773fiJaY=; b=aOilhPv81O9dwRf9omTqYKP0vCUWlpvJrNh0izvZQOrL1WjJflisclOSSRsfDMfF+P rOEzdpN7CHD+8Go+RPvkdnaw00HUmkc/qf29Rlm7dAD5kBe1rpV8GHTKp80PPY8kKfgv 7SWDTTK5SSMcwgY2u+5BlZDe44rmDA0TXL89qduHnL7+ojQgYiVG9QsTq38jxgP2Yk7H TxCimba6hAtzTJpasP2F8QMxiuqjTLMtAAJhSAikzzEfRwqyHCB6O/uqVDtkAohz7WET ZoP0GY/FGdma71kG5ATrQV/8ZMYLwbJxytaX/q49BszQ4UFShm8VK7WiWQgTkGhnIHWp UHZA== X-Gm-Message-State: AGRZ1gKglyzoNkxTW/IvGAWpOspr8q5rdI5v8oDhEiBBZUSTeHg7xzcC CVY6EclvG3lBGc98j2ayFK8PXFkEsmo= X-Google-Smtp-Source: AJdET5etuASNTLx1XJyh2IyuXEEdLmQFdg74gVABcCN02uTy26ccOQ9VrtgnTMr5Txfb/A+D1RhjyA== X-Received: by 2002:a17:906:f1c9:: with SMTP id gx9-v6mr6213353ejb.144.1541150147507; Fri, 02 Nov 2018 02:15:47 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id p19-v6sm5658103ejw.69.2018.11.02.02.15.45 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 02 Nov 2018 02:15:46 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Fri, 2 Nov 2018 10:15:26 +0100 Message-Id: <20181102091531.7775-7-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.19.1 In-Reply-To: <20181102091531.7775-1-daniel.vetter@ffwll.ch> References: <20181102091531.7775-1-daniel.vetter@ffwll.ch> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 07/12] mm, notifier: Add a lockdep map for invalidate_range_start X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP This is a similar idea to the fs_reclaim fake lockdep lock. It's fairly easy to provoke a specific notifier to be run on a specific range: Just prep it, and then munmap() it. A bit harder, but still doable, is to provoke the mmu notifiers for all the various callchains that might lead to them. But both at the same time is really hard to reliable hit, especially when you want to exercise paths like direct reclaim or compaction, where it's not easy to control what exactly will be unmapped. By introducing a lockdep map to tie them all together we allow lockdep to see a lot more dependencies, without having to actually hit them in a single challchain while testing. Aside: Since I typed this to test i915 mmu notifiers I've only rolled this out for the invaliate_range_start callback. If there's interest, we should probably roll this out to all of them. But my undestanding of core mm is seriously lacking, and I'm not clear on whether we need a lockdep map for each callback, or whether some can be shared. Cc: Andrew Morton Cc: David Rientjes Cc: "Jérôme Glisse" Cc: Michal Hocko Cc: "Christian König" Cc: Greg Kroah-Hartman Cc: Daniel Vetter Cc: Mike Rapoport Cc: linux-mm@kvack.org Signed-off-by: Daniel Vetter --- include/linux/mmu_notifier.h | 7 +++++++ mm/mmu_notifier.c | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h index 133ba78820ee..080a9173c7dc 100644 --- a/include/linux/mmu_notifier.h +++ b/include/linux/mmu_notifier.h @@ -16,6 +16,10 @@ struct mmu_notifier_ops; #ifdef CONFIG_MMU_NOTIFIER +#ifdef CONFIG_LOCKDEP +extern struct lockdep_map __mmu_notifier_invalidate_range_start_map; +#endif + /* * The mmu notifier_mm structure is allocated and installed in * mm->mmu_notifier_mm inside the mm_take_all_locks() protected @@ -283,8 +287,11 @@ static inline void mmu_notifier_change_pte(struct mm_struct *mm, static inline void mmu_notifier_invalidate_range_start(struct mm_struct *mm, unsigned long start, unsigned long end) { + mutex_acquire(&__mmu_notifier_invalidate_range_start_map, 0, 0, + _RET_IP_); if (mm_has_notifiers(mm)) __mmu_notifier_invalidate_range_start(mm, start, end, true); + mutex_release(&__mmu_notifier_invalidate_range_start_map, 1, _RET_IP_); } static inline int mmu_notifier_invalidate_range_start_nonblock(struct mm_struct *mm, diff --git a/mm/mmu_notifier.c b/mm/mmu_notifier.c index 4d56e2645b6c..12b917733856 100644 --- a/mm/mmu_notifier.c +++ b/mm/mmu_notifier.c @@ -23,6 +23,13 @@ /* global SRCU for all MMs */ DEFINE_STATIC_SRCU(srcu); +#ifdef CONFIG_LOCKDEP +struct lockdep_map __mmu_notifier_invalidate_range_start_map = { + .name = "mmu_notifier_invalidate_range_start" +}; +EXPORT_SYMBOL_GPL(__mmu_notifier_invalidate_range_start_map); +#endif + /* * This function allows mmu_notifier::release callback to delay a call to * a function that will free appropriate resources. The function must be From patchwork Fri Nov 2 09:15:27 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 10665193 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1AD9813B5 for ; Fri, 2 Nov 2018 09:15:54 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0868D2BA63 for ; Fri, 2 Nov 2018 09:15:54 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id F01D42BA84; Fri, 2 Nov 2018 09:15:53 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 9358B2BA63 for ; Fri, 2 Nov 2018 09:15:53 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 076716E550; Fri, 2 Nov 2018 09:15:53 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ed1-x543.google.com (mail-ed1-x543.google.com [IPv6:2a00:1450:4864:20::543]) by gabe.freedesktop.org (Postfix) with ESMTPS id D27216E550 for ; Fri, 2 Nov 2018 09:15:50 +0000 (UTC) Received: by mail-ed1-x543.google.com with SMTP id h21-v6so841552edq.9 for ; Fri, 02 Nov 2018 02:15:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=v/nJvkO8BlzHVYuslWZiocYbN0/cuXO3PIU87c+u8Bg=; b=rWZwh/k1GUPRdMRdkHJ+9DduYBK6l2wY6jDVw0LCRm3DdaOJS9OxxRwUttOaisM+28 /xgYOLh6s9eCGprwScaPAYk947TJwa46k+Geyv18s0+zPVEm2puecAsx7je2cPaqZQPG 81+aYNC5zGS5OGVVEQh9S5v++Sm8i5lDKaKY2cFiQA1QogryiNR0RfZPS6oswxUVVeru y6kmMY+5IDYbkH+zzqXxyoXPPXtLaBlBDkrUZ8RUhtbdXp1PNKcKHg7vRQQyuPpBNigT 9cz8C468mVUb/e8nJVakSU7zCrejxjhxx+qTXuubAedN3poZwYcjzMV3Ym11ao7GI1Cp mjYg== X-Gm-Message-State: AGRZ1gJdRm/PG+uQArhW50ht5NMLd03k0+S0QZHNq3wFJI8VGPtSDJWR NAxl9pUJh59NUwnDVrW06f5iBcHnlfY= X-Google-Smtp-Source: AJdET5ddRfP0LuxewCLHTU2VlC9zzUWiY3vdyOcu6wj8W8Wt1T8PC8t/OfPVGRO4VFts78C0Q0TFfw== X-Received: by 2002:a17:906:4d41:: with SMTP id b1-v6mr6035514ejv.171.1541150149089; Fri, 02 Nov 2018 02:15:49 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id p19-v6sm5658103ejw.69.2018.11.02.02.15.47 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 02 Nov 2018 02:15:48 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Fri, 2 Nov 2018 10:15:27 +0100 Message-Id: <20181102091531.7775-8-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.19.1 In-Reply-To: <20181102091531.7775-1-daniel.vetter@ffwll.ch> References: <20181102091531.7775-1-daniel.vetter@ffwll.ch> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 08/12] dma-fence: cross-release annotations X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP dma-fence is a completion on steriods, which also allows hardware to directly sync work among each another. It's supposed to be deadlock free, so let's try to make sure that holds at least for the cpu-only interactions and waits. It's a bit much #ifdef, but I figured for the single case it's not worth it to pull it all out. Also, the single global lockdep map is intentional: dma_fences are supposed to be shared, we need all drivers to be compatible to each another in their locking hierarchy. v2: - handle #idef mess cleaner - also annotate dma_fence_signal() v3: - Add a dma_fence_might_wait() function to annotate fastpaths - Put it all into headers for other code to use Cc: Sumit Semwal Cc: Gustavo Padovan Cc: linux-media@vger.kernel.org Cc: linaro-mm-sig@lists.linaro.org Signed-off-by: Daniel Vetter --- drivers/dma-buf/dma-fence.c | 14 ++++++++++++++ include/linux/dma-fence.h | 28 ++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 1551ca7df394..7d56d8b48624 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -30,6 +30,13 @@ EXPORT_TRACEPOINT_SYMBOL(dma_fence_emit); EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal); + +#ifdef CONFIG_LOCKDEP_CROSSRELEASE +struct lockdep_map_cross dma_fence_wait_map = + STATIC_CROSS_LOCKDEP_MAP_INIT("dma_fence_signal", &dma_fence_wait_map); +EXPORT_SYMBOL(dma_fence_wait_map); +#endif + /* * fence context counter: each execution context should have its own * fence context, this allows checking if fences belong to the same @@ -106,6 +113,8 @@ int dma_fence_signal_locked(struct dma_fence *fence) lockdep_assert_held(fence->lock); + dma_fence_wait_release_commit(); + if (WARN_ON(!fence)) return -EINVAL; @@ -153,6 +162,8 @@ int dma_fence_signal(struct dma_fence *fence) if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) return -EINVAL; + dma_fence_wait_release_commit(); + fence->timestamp = ktime_get(); set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags); trace_dma_fence_signaled(fence); @@ -197,12 +208,15 @@ dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout) if (WARN_ON(timeout < 0)) return -EINVAL; + dma_fence_wait_acquire(); trace_dma_fence_wait_start(fence); if (fence->ops->wait) ret = fence->ops->wait(fence, intr, timeout); else ret = dma_fence_default_wait(fence, intr, timeout); trace_dma_fence_wait_end(fence); + dma_fence_wait_release(); + return ret; } EXPORT_SYMBOL(dma_fence_wait_timeout); diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 02dba8cd033d..8a045334a207 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -34,6 +34,34 @@ struct dma_fence; struct dma_fence_ops; struct dma_fence_cb; +#ifdef CONFIG_LOCKDEP_CROSSRELEASE +#include +extern struct lockdep_map_cross dma_fence_wait_map; + +static inline void dma_fence_wait_acquire(void) +{ + lock_acquire_exclusive(&dma_fence_wait_map.map, 0, 0, NULL, _RET_IP_); +} +static inline void dma_fence_wait_release(void) +{ + lock_release(&dma_fence_wait_map.map, 0, _RET_IP_); +} +static inline void dma_fence_wait_release_commit(void) +{ + lock_commit_crosslock(&dma_fence_wait_map.map); +} +#else +static inline void dma_fence_wait_acquire(void) {} +static inline void dma_fence_wait_release(void) {} +static inline void dma_fence_wait_release_commit(void) {} +#endif + +static inline void dma_fence_might_wait(void) +{ + dma_fence_wait_acquire(); + dma_fence_wait_release(); +} + /** * struct dma_fence - software synchronization primitive * @refcount: refcount for this fence From patchwork Fri Nov 2 09:15:28 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 10665195 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A147F15E9 for ; Fri, 2 Nov 2018 09:15:54 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8ECD52BA63 for ; Fri, 2 Nov 2018 09:15:54 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 832902BA84; Fri, 2 Nov 2018 09:15:54 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 48CB42BA63 for ; Fri, 2 Nov 2018 09:15:54 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 75DE26E558; Fri, 2 Nov 2018 09:15:53 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ed1-x542.google.com (mail-ed1-x542.google.com [IPv6:2a00:1450:4864:20::542]) by gabe.freedesktop.org (Postfix) with ESMTPS id DC9B06E550 for ; Fri, 2 Nov 2018 09:15:51 +0000 (UTC) Received: by mail-ed1-x542.google.com with SMTP id t10-v6so1259987eds.12 for ; Fri, 02 Nov 2018 02:15:51 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=HeZyi+pUwpeLs9Jmvlj3k+X5NVcF1gTKmQR3rzR90ZE=; b=TbMgj/EGIvkjnciLxKrh3c04/iaQOV6HJoRiBTtr8LHBxTCpzytLLi6cFZsbTwu1u/ eVfLkLyr5f8cLmqVJTs+JOmIaqT42mGtlpGBE8LKCkj7HrlsBwp/ohID4Kzz1v6FR77s Av51KMe+6XbBa+wNlzxUjZIpYbNf6f2QTaKCWNWFVNOv+p7d6AAdMrYM5pPK46e0Yt1+ 7cG0A1LoVbmqAdp52T2msQf/48QAqQxLKFHgwauPwT3EMX8/bSOqR01mjGbMi7Jd30zX B7ZQLuCXvVyT3An9XIY81AFlditZhaA9IsgCZ1h/ybTZHEL86PxyUqH2cbQwpwyh+ZE5 8uLw== X-Gm-Message-State: AGRZ1gI8vEIGcJpefKku32eeTLykPXHV5BYlR9/lF02qi7wivo1tG6bx dA1WZyZvxnjLNM/zJZsgH0QVQGIrM10= X-Google-Smtp-Source: AJdET5d9QfWfqjTypzfL6nOexd4haFt+KsfzBmV0YZo3mN3EOq73ERz3SOtWJ+kMLQ7ozFW6w9zk3A== X-Received: by 2002:a17:906:2ec8:: with SMTP id s8-v6mr6092230eji.192.1541150150098; Fri, 02 Nov 2018 02:15:50 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id p19-v6sm5658103ejw.69.2018.11.02.02.15.49 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 02 Nov 2018 02:15:49 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Fri, 2 Nov 2018 10:15:28 +0100 Message-Id: <20181102091531.7775-9-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.19.1 In-Reply-To: <20181102091531.7775-1-daniel.vetter@ffwll.ch> References: <20181102091531.7775-1-daniel.vetter@ffwll.ch> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 09/12] reservation: Annotate dma_fence waits X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP reservations have an optimized fastpath that bypasses dma_fence_wait(), make sure we still catch all dependencies in lockdep. To avoid lockdep complaining about recursion on the fake dma_fence_wait lock we use the dma_fence_might_sleep annotation. Cc: Sumit Semwal Cc: linux-media@vger.kernel.org Cc: linaro-mm-sig@lists.linaro.org Signed-off-by: Daniel Vetter --- drivers/dma-buf/reservation.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index c1618335ca99..599b88c75cd5 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -429,6 +429,8 @@ long reservation_object_wait_timeout_rcu(struct reservation_object *obj, long ret = timeout ? timeout : 1; int i; + dma_fence_might_wait(); + retry: shared_count = 0; seq = read_seqcount_begin(&obj->seq); From patchwork Fri Nov 2 09:15:29 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 10665199 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 0FBA317D5 for ; Fri, 2 Nov 2018 09:15:57 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id F1BF42BA63 for ; Fri, 2 Nov 2018 09:15:56 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E63D12BA84; Fri, 2 Nov 2018 09:15:56 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id A217D2BA63 for ; Fri, 2 Nov 2018 09:15:56 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id DE6BF6E55A; Fri, 2 Nov 2018 09:15:55 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ed1-x542.google.com (mail-ed1-x542.google.com [IPv6:2a00:1450:4864:20::542]) by gabe.freedesktop.org (Postfix) with ESMTPS id 05CDC6E542 for ; Fri, 2 Nov 2018 09:15:53 +0000 (UTC) Received: by mail-ed1-x542.google.com with SMTP id y20-v6so1268907eds.10 for ; Fri, 02 Nov 2018 02:15:52 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=q3IDMIBRwheka09D/N0RHTebGGKal8W73vQfBiBemM4=; b=q4UhqD4qKA3WkCZjKmI3dZvFQ+OcqWuOrHyuA2OeyCrphxIqY+RpA4Xihb/XYe8DQL IDCCXzqylA8lYp4Dek4XGNzH4z1IBvQ2nYT6uVTGay0dGDRAt4zR9icjElwjSQnjKgwX 01L4tDU/C53xbcGulgu7StMy8YaSyZ7VkHgI92Q7Kn1Vi63VRHncSwY9wcyWfkCdxr2x 5h2fel82ZoymbZ85lQ2mzcAyezM8/xPaAPZulzP9mkgVAlIIjZLtXgevGwEVSN3AxwcZ Xxzk+GXdaQvTJewysyDF7dGt1SAmvNGn20Kx8fjr77XTxG2kr8EfueShHK/KX7q3xMyk cE1w== X-Gm-Message-State: AGRZ1gK5J+epXqNUCfDWR+P/Ej8CjRQ4lzh+IgJq9UdV+NT/RUgA9sKR 92e6FZABAGhfkKNuSfz+daGL/QUjNBo= X-Google-Smtp-Source: AJdET5cmhtHZKLI/FYACzb0sYyERCgL1ENCAz6WZAKz1W1gEPMIGW723mILxBTFVtKZgIYT4mRKcyg== X-Received: by 2002:a50:9665:: with SMTP id y92-v6mr8165305eda.122.1541150151393; Fri, 02 Nov 2018 02:15:51 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id p19-v6sm5658103ejw.69.2018.11.02.02.15.50 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 02 Nov 2018 02:15:50 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Fri, 2 Nov 2018 10:15:29 +0100 Message-Id: <20181102091531.7775-10-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.19.1 In-Reply-To: <20181102091531.7775-1-daniel.vetter@ffwll.ch> References: <20181102091531.7775-1-daniel.vetter@ffwll.ch> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 10/12] drm/i915: Annotate dma_fence waits X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP i915_request_wait is simply our i915-optimized version of dma_fence_wait. They both use the exact same code. To help lockdep discovering all the dependencies, annotate it. Cc: Jani Nikula Cc: Joonas Lahtinen Cc: Rodrigo Vivi Cc: intel-gfx@lists.freedesktop.org Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_request.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 71107540581d..6b9722782d12 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -30,6 +30,10 @@ #include "i915_drv.h" +static long __i915_request_wait(struct i915_request *rq, + unsigned int flags, + long timeout); + static const char *i915_fence_get_driver_name(struct dma_fence *fence) { return "i915"; @@ -66,7 +70,7 @@ static signed long i915_fence_wait(struct dma_fence *fence, bool interruptible, signed long timeout) { - return i915_request_wait(to_request(fence), interruptible, timeout); + return __i915_request_wait(to_request(fence), interruptible, timeout); } static void i915_fence_release(struct dma_fence *fence) @@ -1201,6 +1205,19 @@ static bool __i915_wait_request_check_and_reset(struct i915_request *request) long i915_request_wait(struct i915_request *rq, unsigned int flags, long timeout) +{ + long ret; + + dma_fence_wait_acquire(); + ret = __i915_request_wait(rq, flags, timeout); + dma_fence_wait_release(); + + return ret; +} + +static long __i915_request_wait(struct i915_request *rq, + unsigned int flags, + long timeout) { const int state = flags & I915_WAIT_INTERRUPTIBLE ? TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE; From patchwork Fri Nov 2 09:15:30 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 10665197 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 6BC5115E9 for ; Fri, 2 Nov 2018 09:15:56 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5A1702BA63 for ; Fri, 2 Nov 2018 09:15:56 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4E66D2BA84; Fri, 2 Nov 2018 09:15:56 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id ECAB52BA63 for ; Fri, 2 Nov 2018 09:15:55 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 63A396E542; Fri, 2 Nov 2018 09:15:55 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ed1-x541.google.com (mail-ed1-x541.google.com [IPv6:2a00:1450:4864:20::541]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7144E6E55A for ; Fri, 2 Nov 2018 09:15:54 +0000 (UTC) Received: by mail-ed1-x541.google.com with SMTP id h21-v6so841652edq.9 for ; Fri, 02 Nov 2018 02:15:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=aEWm8L9oAmKCiEuVhjX5SVaG0yfzvl7KKuXr5g2E5SA=; b=Ua5OF7qfKwQwhTLGqZGzNybKQVAkGnV3KxTLpdqyfQEmT8cvKbxLuz+vleCgkSt7a2 QdcLmPNGSbDaNzIa5FqZx641eRocbL2WdYnVjB+M+l0dtBGB0W48cGnGhDUMmd/zrsDh dyP79YDmHn4Y0apUYKkPRngzw0r6ElmTk4bYXya+TXF9dsGfD00xtIxQFasFdINMbEpl CPzlmlzGdfW8yo2GOuivgWefPIVYhMfrNT4cHnlYaY2NRKmVQA4l4qejCdIjpV5VOan+ gnV27yEWME6lMXHkiTID6akhdGNmcGjAuavvggeoY99Pc8l9mSQovxctOQhPSpRfj8iI EDcw== X-Gm-Message-State: AGRZ1gL+XDCm3PUoR2eVf3hVChYIuNnBvGA37xFiPocTa7pBJdfxYWzw 21mrG2nB6fgl2MtvOfr8V+Zfa60ES50= X-Google-Smtp-Source: AJdET5emMgG02nGUdUN1QfNiBeakulY6Za2QdM7HkiPx++/QN+w4JSxlnn3itZRF/xqKTwQhgzo+TA== X-Received: by 2002:a17:906:1805:: with SMTP id v5-v6mr3381700eje.134.1541150152484; Fri, 02 Nov 2018 02:15:52 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id p19-v6sm5658103ejw.69.2018.11.02.02.15.51 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 02 Nov 2018 02:15:51 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Fri, 2 Nov 2018 10:15:30 +0100 Message-Id: <20181102091531.7775-11-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.19.1 In-Reply-To: <20181102091531.7775-1-daniel.vetter@ffwll.ch> References: <20181102091531.7775-1-daniel.vetter@ffwll.ch> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 11/12] drm/i915: annotate intel_atomic_commit_fence_wait X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP In i915 we also have i915_sw_fence, as some kind of super fence. Since those include dma-fences (it's the main use really, aside from chaining them for the scheduler) they're relevant for any lockdep cycles involving dma_fence. But i915_sw_fence is also really tough to properly annonate: - Most of the use (all of it in the scheduler) is entirely driven by callbacks. That creates dependency chains, but for all practical purposes we could treat that as part of the hw magic that executes requests for us. Eventually a dma_fence_signal() comes out of that, which we do annotate. - For dma-fences we only annotate cpu waits, and don't follow any of the chains going through the hardware. Since dma_fence are supposed to be ordered an always complete that should be good enough, as long as we don't accidentally deadlock on the cpu side. So treating i915_sw_fence as part of the hw magic shouldn't reduce our deadlock coverage, as long as i915_sw_fence itself doesn't deadlock. And there's lots of debug checks for that already. - There's one exception: intel_atomic_commit_fence_wait() is the only cpu wait on a i915_sw_fence we have. That one we should annotate, and annotate should be able to teach lockdep about our gpu reset vs modeset locking inversion. But in reality this is a dma-fence wait, and it using i915_sw_fence is really just an implementation detail. All other kms drivers use drm_atomic_helper_wait_for_fences() instead. - There is a i915_sw_fence_wait for normal cpu waits (the one in the modeset code is open-coded because it also waits for gpu reset wakups), but that's only used for selftests. So the simplest solution for annotating i915_sw_fence is therefore to annotate intel_atomic_commit_fence() as a dma_fence wait. Cc: Jani Nikula Cc: Joonas Lahtinen Cc: Rodrigo Vivi Cc: intel-gfx@lists.freedesktop.org Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 0aabc4b9854a..a2eee76da46b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12728,6 +12728,7 @@ static void intel_atomic_commit_fence_wait(struct intel_atomic_state *intel_stat struct wait_queue_entry wait_fence, wait_reset; struct drm_i915_private *dev_priv = to_i915(intel_state->base.dev); + dma_fence_wait_acquire(); init_wait_entry(&wait_fence, 0); init_wait_entry(&wait_reset, 0); for (;;) { @@ -12745,6 +12746,7 @@ static void intel_atomic_commit_fence_wait(struct intel_atomic_state *intel_stat } finish_wait(&intel_state->commit_ready.wait, &wait_fence); finish_wait(&dev_priv->gpu_error.wait_queue, &wait_reset); + dma_fence_wait_release(); } static void intel_atomic_cleanup_work(struct work_struct *work) From patchwork Fri Nov 2 09:15:31 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 10665201 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 660F613B5 for ; Fri, 2 Nov 2018 09:15:59 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 55C0C2BA84 for ; Fri, 2 Nov 2018 09:15:59 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4A0982BAA0; Fri, 2 Nov 2018 09:15:59 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 10DC12BA84 for ; Fri, 2 Nov 2018 09:15:59 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 794AA6E556; Fri, 2 Nov 2018 09:15:58 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ed1-x544.google.com (mail-ed1-x544.google.com [IPv6:2a00:1450:4864:20::544]) by gabe.freedesktop.org (Postfix) with ESMTPS id A1F496E556 for ; Fri, 2 Nov 2018 09:15:55 +0000 (UTC) Received: by mail-ed1-x544.google.com with SMTP id w19-v6so1312610eds.1 for ; Fri, 02 Nov 2018 02:15:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=jGodeTsiRCA1nY7YKOSPhCpu7lo/bkQw47+dNSehn84=; b=ZNpfoKHWshmmYt7/rF7umGSBv7peaWbAQVNc6jvv4osFH1u5ZKWQ3bVE/xf45eAlcx NL/lg/d1g6vHOg+Pd5wYYrxHh11EsrCLbUl9i7ejXPX1c4TFjFpo3/eajejii/HhTKmi /u06AcNV7RTnS7ftY2fKZmIP9p039sld7ayk/VNl/ecCR9+N26oa4SSD13CROxzctW6V T/BXnDOb2xl19kWE7+h4dxPYi/R26SAePWLxZu6oCxefgQ9n6XbW5h0NrqazBpdgtGzh dN7rFn91AB1sC3A2m9xT52bxb8DFzx6cssM/1XCTGxJvE97DclJAbPPBaS2jbbZwQySD WbBA== X-Gm-Message-State: AGRZ1gIdpkT8pOcXryxsjkZhXh6yUkfRld6mKgLRe1POtY6HjaoXGWY7 jh+IfR1VCKqpUyYPsTVas2CAFAdqNmo= X-Google-Smtp-Source: AJdET5e6VKKLA148qoBzJuEoO++wNIPRkMFHZlXbo23mombyQu4PBgvgtnB4YnMXDWggC6OR08QvNA== X-Received: by 2002:a50:c191:: with SMTP id m17-v6mr7710306edf.123.1541150153901; Fri, 02 Nov 2018 02:15:53 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id p19-v6sm5658103ejw.69.2018.11.02.02.15.52 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 02 Nov 2018 02:15:52 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Fri, 2 Nov 2018 10:15:31 +0100 Message-Id: <20181102091531.7775-12-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.19.1 In-Reply-To: <20181102091531.7775-1-daniel.vetter@ffwll.ch> References: <20181102091531.7775-1-daniel.vetter@ffwll.ch> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 12/12] HAX FOR CI: Enable cross-release X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP Only way to convince our CI to enable stuff that's new and defaulting to off. Obviously not for merging. Signed-off-by: Daniel Vetter --- lib/Kconfig.debug | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 7e504b7fbbe1..64cd6a43b397 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1095,6 +1095,7 @@ config LOCKDEP_CROSSRELEASE bool "Enable cross-release checking" depends on PROVE_LOCKING select LOCKDEP_COMPLETIONS + default y help This makes lockdep work for crosslock which is a lock allowed to be released in a different context from the acquisition context.