[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <09ee3179-5100-4c7c-bf3d-0a659a3e7633@DB8EHSMHS026.ehs.local>
Date: Fri, 31 May 2013 14:10:05 -0700
From: Sören Brinkmann <soren.brinkmann@...inx.com>
To: Mike Turquette <mturquette@...aro.org>
CC: Michal Simek <michal.simek@...inx.com>,
Lai Jiangshan <laijs@...fujitsu.com>,
<paulmck@...ux.vnet.ibm.com>, <linux-kernel@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>, <git@...inx.com>
Subject: Re: [BUG] zynq | CCF | SRCU
Hi Mike,
On Fri, May 31, 2013 at 12:52:35PM -0700, Mike Turquette wrote:
> Quoting Sören Brinkmann (2013-05-31 12:12:07)
> > Hi,
> >
> > we recently encountered some kernel panics when we compiled one of our
> > drivers as module and tested inserting/removing the module.
> > Trying to debug this issue, I could reproduce it on the mainline kernel
> > with a dummy module.
> >
> > What happens is, that when on driver remove clk_notifier_unregister() is
> > called and no other notifier for that clock is registered, the kernel
> > panics.
> > I'm not sure what is going wrong here. If there is a bug (and if where)
> > or I'm just using the infrastructure the wrong way,... So, any hint is
> > appreciated.
> >
> > I attach the output from the crashing system. The stacktrace indicates a
> > crash in 'srcu_readers_seq_idx()'.
> > I also attach the module I used to trigger the issue and a patch on top
> > of mainline commit a93cb29acaa8f75618c3f202d1cf43c231984644 which has
> > the DT modifications I need to make the module find its clock and boot
> > with my initramfs.
> >
>
> Soren,
>
> I only took a quick look at this so the following is a shot in the dark.
> notifier_block->next should be protected by an RCU lock, and the way you
> open-code the initialization struck me as a bit weird. Can you change
> your code to the following and let me know if it makes any difference?
>
> static struct notifier_block nb = {
> .notifier_call = clk_notif_dbg_cb;
> };
>
> static int clk_notif_dbg_cb(struct notifier_block *nb,
> unsigned long event, void *data)
> {
> pr_info("clk_notif_dbg_cb\n");
>
> return NOTIFY_OK;
> }
>
> static int clk_notif_dbg_probe(struct platform_device *pdev)
> {
> ...
> if (clk_notifier_register(clk, &nb))
> dev_warn(&pdev->dev, "clk_notifier_register failed\n");
> ...
>
>
> That is a small difference, but that style of initializing the
> notifier_block has always worked for me when using clk rate-change
> notifiers. However I'm sure the bug you mention is far more evil and
> nefarious than that ;-)
I changed the module like this (I hope this is what you suggested):
8,9d7
< static struct notifier_block nb;
<
17a16,19
> static struct notifier_block nb = {
> .notifier_call = clk_notif_dbg_cb
> };
>
20,22d21
< nb.next = NULL;
< nb.notifier_call = clk_notif_dbg_cb;
<
The result is the same. It crashes in srcu_readers_seq_idx().
Thanks,
Sören
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists