From: Tejun Heo <tj@kernel.org>
To: Andre Przywara <andre.przywara@arm.com>
Cc: Icenowy Zheng <icenowy@aosc.xyz>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Adam Borowski <kilobyte@angband.pl>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: sun50i-a64-pinctrl WARN_ON drivers/base/dd.c:349
Date: Fri, 28 Apr 2017 18:03:14 -0400 [thread overview]
Message-ID: <20170428220314.GI22354@htj.duckdns.org> (raw)
In-Reply-To: <3fda46eb-7fcc-4c27-98b6-7d822cce1405@arm.com>
Hello,
On Tue, Apr 18, 2017 at 10:12:16AM +0100, Andre Przywara wrote:
> Yeah, so I stack-dumped on the zero allocations and indeed they are
> called from cleanup functions:
> drivers/pinctrl/pinmux.c:pinmux_generic_free_functions():
> devm_kzalloc(sizeof(*indices) * pctldev->num_functions, ...)
> (and another one I don't know from the top of the my head, logs at home)
>
> So my hunch was that once EPROBE_DEFER triggers the devres cleanup, it
> uses some reverse list traversal to release all allocated resources
> (backwards!), so missing those which get (appended) during the process.
> But I don't think that would not work with the locking.
> So I have to dig deeper tonight in my logs.
If this is a valid use case, we can change devm to repeat till empty
but it's a weird thing to do to allocate from a release function.
So, something like this. Only compile tested.
Thanks.
diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index 71d5770..d2a9f34 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -509,13 +509,21 @@ static int release_nodes(struct device *dev, struct list_head *first,
int devres_release_all(struct device *dev)
{
unsigned long flags;
+ int cnt = 0, ret;
/* Looks like an uninitialized device structure */
if (WARN_ON(dev->devres_head.next == NULL))
return -ENODEV;
- spin_lock_irqsave(&dev->devres_lock, flags);
- return release_nodes(dev, dev->devres_head.next, &dev->devres_head,
- flags);
+
+ /* Release callbacks may create new nodes, repeat till empty */
+ do {
+ spin_lock_irqsave(&dev->devres_lock, flags);
+ ret = release_nodes(dev, dev->devres_head.next,
+ &dev->devres_head, flags);
+ cnt += ret;
+ } while (ret);
+
+ return cnt;
}
/**
next prev parent reply other threads:[~2017-04-28 22:03 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20170315161406.smd4na25two55jjh@angband.pl>
[not found] ` <197431489595078@web8g.yandex.ru>
2017-03-16 1:06 ` sun50i-a64-pinctrl WARN_ON drivers/base/dd.c:349 Greg Kroah-Hartman
2017-03-17 14:08 ` Tejun Heo
[not found] ` <785901489760914@web50g.yandex.ru>
2017-03-17 14:44 ` Tejun Heo
2017-03-17 16:32 ` Adam Borowski
2017-04-02 23:48 ` André Przywara
2017-04-18 7:25 ` Tejun Heo
2017-04-18 9:12 ` Andre Przywara
2017-04-28 22:03 ` Tejun Heo [this message]
2017-04-29 21:28 ` Adam Borowski
2017-04-29 23:34 ` André Przywara
2017-05-01 19:22 ` Tejun Heo
2017-04-18 10:51 ` Icenowy Zheng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170428220314.GI22354@htj.duckdns.org \
--to=tj@kernel.org \
--cc=andre.przywara@arm.com \
--cc=gregkh@linuxfoundation.org \
--cc=icenowy@aosc.xyz \
--cc=kilobyte@angband.pl \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).