From: Alexandre Belloni <alexandre.belloni@bootlin.com>
To: Trent Piepho <tpiepho@impinj.com>
Cc: "linux-rtc@vger.kernel.org" <linux-rtc@vger.kernel.org>,
"anson.huang@nxp.com" <anson.huang@nxp.com>,
"a.zummo@towertech.it" <a.zummo@towertech.it>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"aisheng.dong@nxp.com" <aisheng.dong@nxp.com>,
"linux-imx@nxp.com" <linux-imx@nxp.com>
Subject: Re: [PATCH] rtc: snvs: fix possible race condition
Date: Sat, 20 Jul 2019 21:55:51 +0200 [thread overview]
Message-ID: <20190720195551.GB3271@piout.net> (raw)
In-Reply-To: <1563563060.2343.88.camel@impinj.com>
On 19/07/2019 19:04:21+0000, Trent Piepho wrote:
> On Fri, 2019-07-19 at 02:57 +0000, Anson Huang wrote:
> >
> > > I do worry that handling the irq before the rtc device is registered could still
> > > result in a crash. From what I saw, the irq path in snvs only uses driver state
> > > members that are fully initialized for the most part, and the allocated but
> > > unregistered data->rtc is only used in one call to rtc_update_irq(), which
> > > appears to be ok with this.
> > >
> > > But it is not that hard to imagine that something could go into the rtc core
> > > that assumes call like rtc_update_irq() are only made on registered devices.
> > >
> > > If there was a way to do it, I think allocating the irq in a masked state and
> > > then unmasking it as part of the final registration call to make the device go
> > > live would be a safer and more general pattern.
> >
> > It makes sense, I think we can just move the devm_request_irq() to after rtc_register_device(),
> > It will make sure everything is ready before IRQ is enabled. Will send out a V2 patch.
>
> That will mean registering the rtc, then unregistering it if the irq
> request fails. More of a pain to write this failure path.
>
> Alexandre, is it part of rtc core design that rtc_update_irq() might be
> called on a rtc device that is properly allocated, but not registered
> yet?
Yes, the main reason of the change of API was exactly to handle this.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
next prev parent reply other threads:[~2019-07-20 20:00 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-16 7:18 [PATCH] rtc: snvs: fix possible race condition Anson.Huang
2019-07-17 10:54 ` Aisheng Dong
2019-07-17 13:57 ` Anson Huang
2019-07-18 3:08 ` Aisheng Dong
2019-07-18 16:32 ` Trent Piepho
2019-07-19 2:57 ` Anson Huang
2019-07-19 19:04 ` Trent Piepho
2019-07-20 19:55 ` Alexandre Belloni [this message]
2019-08-13 9:22 ` Anson Huang
2019-08-29 15:39 ` Alexandre Belloni
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=20190720195551.GB3271@piout.net \
--to=alexandre.belloni@bootlin.com \
--cc=a.zummo@towertech.it \
--cc=aisheng.dong@nxp.com \
--cc=anson.huang@nxp.com \
--cc=linux-imx@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rtc@vger.kernel.org \
--cc=tpiepho@impinj.com \
/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.