From patchwork Wed Apr 12 02:10:20 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Ellerman X-Patchwork-Id: 749703 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3w2nTl3Gfdz9sNr for ; Wed, 12 Apr 2017 12:11:23 +1000 (AEST) Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 3w2nTl2FsVzDqK3 for ; Wed, 12 Apr 2017 12:11:23 +1000 (AEST) X-Original-To: linuxppc-dev@lists.ozlabs.org Delivered-To: linuxppc-dev@lists.ozlabs.org Received: from ozlabs.org (ozlabs.org [IPv6:2401:3900:2:1::2]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3w2nSY2gDRzDq5W for ; Wed, 12 Apr 2017 12:10:21 +1000 (AEST) Received: by ozlabs.org (Postfix) id 3w2nSY1n19z9sNt; Wed, 12 Apr 2017 12:10:21 +1000 (AEST) Delivered-To: linuxppc-dev@ozlabs.org Received: from authenticated.ozlabs.org (localhost [127.0.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPSA id 3w2nSY0tb5z9sNr; Wed, 12 Apr 2017 12:10:21 +1000 (AEST) From: Michael Ellerman To: Tyrel Datwyler , Tyrel Datwyler , Sachin Sant , linuxppc-dev@ozlabs.org Subject: Re: WARN @lib/refcount.c:128 during hot unplug of I/O adapter. In-Reply-To: References: <8760ig983f.fsf@concordia.ellerman.id.au> <89aec36c-e352-e055-5e80-1235449762ce@linux.vnet.ibm.com> <871sszwc87.fsf@concordia.ellerman.id.au> User-Agent: Notmuch/0.21 (https://notmuchmail.org) Date: Wed, 12 Apr 2017 12:10:20 +1000 Message-ID: <87efwyv0j7.fsf@concordia.ellerman.id.au> MIME-Version: 1.0 X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Nathan Fontenot , LKML Errors-To: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org Sender: "Linuxppc-dev" Tyrel Datwyler writes: > On 04/11/2017 02:00 AM, Michael Ellerman wrote: >> Tyrel Datwyler writes: >>> I started looking at it when Bharata submitted a patch trying to fix the >>> issue for CPUs, but got side tracked by other things. I suspect that >>> this underflow has actually been an issue for quite some time, and we >>> are just now becoming aware of it thanks to the recount_t patchset being >>> merged. >> >> Yes I agree. Which means it might be broken in existing distros. > > Definitely. I did some profiling last night, and I understand the > hotplug case. It turns out to be as I suggested in the original thread > about CPUs. When the devicetree code was worked to move the tree out of > proc and into sysfs the sysfs detach code added a of_node_put to remove > the original of_init reference. pSeries Being the sole original > *dynamic* device tree user we had always issued a of_node_put in our > dlpar specific detach function to achieve that end. So, this should be a > pretty straight forward trivial fix. Excellent, thanks. > However, for the case where devices are present at boot it appears we a > leaking a lot of references resulting in the device nodes never actually > being released/freed after a dlpar remove. In the CPU case after boot I > count 8 more references taken than the hotplug case, and corresponding > of_node_put's are not called at dlpar remove time either. That will take > some time to track them down, review and clean up. Yes that is a perennial problem unfortunately which we've never come up with a good solution for. The (old) patch below might help track some of them down. I remember having a script to process the output of the trace and find mismatches, but I can't find it right now - but I'm sure you can hack up something :) cheers diff --git a/arch/powerpc/include/asm/trace.h b/arch/powerpc/include/asm/trace.h index 32e36b16773f..ad32365082a0 100644 --- a/arch/powerpc/include/asm/trace.h +++ b/arch/powerpc/include/asm/trace.h @@ -168,6 +168,44 @@ TRACE_EVENT(hash_fault, __entry->addr, __entry->access, __entry->trap) ); +TRACE_EVENT(of_node_get, + + TP_PROTO(struct device_node *dn, int val), + + TP_ARGS(dn, val), + + TP_STRUCT__entry( + __field(struct device_node *, dn) + __field(int, val) + ), + + TP_fast_assign( + __entry->dn = dn; + __entry->val = val; + ), + + TP_printk("get %d -> %d %s", __entry->val - 1, __entry->val, __entry->dn->full_name) +); + +TRACE_EVENT(of_node_put, + + TP_PROTO(struct device_node *dn, int val), + + TP_ARGS(dn, val), + + TP_STRUCT__entry( + __field(struct device_node *, dn) + __field(int, val) + ), + + TP_fast_assign( + __entry->dn = dn; + __entry->val = val; + ), + + TP_printk("put %d -> %d %s", __entry->val + 1, __entry->val, __entry->dn->full_name) +); + #endif /* _TRACE_POWERPC_H */ #undef TRACE_INCLUDE_PATH diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index c647bd1b6903..f5c3d761f3cd 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -14,6 +14,8 @@ #include "of_private.h" +#include + /** * of_node_get() - Increment refcount of a node * @node: Node to inc refcount, NULL is supported to simplify writing of @@ -23,8 +25,12 @@ */ struct device_node *of_node_get(struct device_node *node) { - if (node) + if (node) { kobject_get(&node->kobj); + + trace_of_node_get(node, atomic_read(&node->kobj.kref.refcount)); + } + return node; } EXPORT_SYMBOL(of_node_get); @@ -36,8 +42,10 @@ EXPORT_SYMBOL(of_node_get); */ void of_node_put(struct device_node *node) { - if (node) + if (node) { kobject_put(&node->kobj); + trace_of_node_put(node, atomic_read(&node->kobj.kref.refcount)); + } } EXPORT_SYMBOL(of_node_put);