All of lore.kernel.org
 help / color / mirror / Atom feed
From: mturquette@linaro.org (Mike Turquette)
To: linux-arm-kernel@lists.infradead.org
Subject: [BUG] zynq | CCF | SRCU
Date: Fri, 31 May 2013 12:52:35 -0700	[thread overview]
Message-ID: <20130531195235.21525.64931@quantum> (raw)
In-Reply-To: <42b8bfd5-3012-4c49-b9ef-7a9beb5956f1@VA3EHSMHS041.ehs.local>

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 ;-)

Regards,
Mike

> 
>         Thanks,
>         S?ren

WARNING: multiple messages have this Message-ID (diff)
From: Mike Turquette <mturquette@linaro.org>
To: "Sören Brinkmann" <soren.brinkmann@xilinx.com>,
	"Michal Simek" <michal.simek@xilinx.com>,
	"Lai Jiangshan" <laijs@cn.fujitsu.com>,
	paulmck@linux.vnet.ibm.com
Cc: "Sören Brinkmann" <soren.brinkmann@xilinx.com>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, git@xilinx.com
Subject: Re: [BUG] zynq | CCF | SRCU
Date: Fri, 31 May 2013 12:52:35 -0700	[thread overview]
Message-ID: <20130531195235.21525.64931@quantum> (raw)
In-Reply-To: <42b8bfd5-3012-4c49-b9ef-7a9beb5956f1@VA3EHSMHS041.ehs.local>

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 ;-)

Regards,
Mike

> 
>         Thanks,
>         Sören

  reply	other threads:[~2013-05-31 19:52 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-31 19:12 [BUG] zynq | CCF | SRCU Sören Brinkmann
2013-05-31 19:12 ` Sören Brinkmann
2013-05-31 19:52 ` Mike Turquette [this message]
2013-05-31 19:52   ` Mike Turquette
2013-05-31 21:10   ` Sören Brinkmann
2013-05-31 21:10     ` Sören Brinkmann
2013-06-03  9:17 ` [PATCH] clk: remove the clk_notifier from clk_notifier_list before free it (was: Re: [BUG] zynq | CCF | SRCU) Lai Jiangshan
2013-06-03  9:17   ` Lai Jiangshan
2013-06-03 16:49   ` Sören Brinkmann
2013-06-03 16:49     ` Sören Brinkmann
2013-06-07  1:42   ` Mike Turquette
2013-06-07  1:42     ` Mike Turquette

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=20130531195235.21525.64931@quantum \
    --to=mturquette@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.