From: gregkh@suse•de (Greg KH)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 3/6] i2c/pxa2xx: Add PCI support for PXA I2C controller
Date: Thu, 6 Jan 2011 13:57:26 -0800 [thread overview]
Message-ID: <20110106215726.GB30799@suse.de> (raw)
In-Reply-To: <20110106092044.GT8638@n2100.arm.linux.org.uk>
On Thu, Jan 06, 2011 at 09:20:44AM +0000, Russell King - ARM Linux wrote:
> On Wed, Jan 05, 2011 at 03:08:34PM -0800, Greg KH wrote:
> > On Wed, Jan 05, 2011 at 11:03:42PM +0000, Russell King - ARM Linux wrote:
> > > On Wed, Jan 05, 2011 at 05:51:00PM +0100, Sebastian Andrzej Siewior wrote:
> > > > +static void plat_dev_release(struct device *dev)
> > > > +{
> > > > + struct ce4100_i2c_device *sd = container_of(dev,
> > > > + struct ce4100_i2c_device, pdev.dev);
> > > > +
> > > > + of_device_node_put(&sd->pdev.dev);
> > > > +}
> > > > +static int add_i2c_device(struct pci_dev *dev, int bar,
> > > > + struct ce4100_i2c_device *sd)
> > > > +{
> > > > + struct platform_device *pdev = &sd->pdev;
> > > > + struct i2c_pxa_platform_data *pdata = &sd->pdata;
> > > ...
> > > > + pdev->name = "ce4100-i2c";
> > > > + pdev->dev.release = plat_dev_release;
> > > > + pdev->dev.parent = &dev->dev;
> > > > +
> > > > + pdev->dev.platform_data = pdata;
> > > > + pdev->resource = sd->res;
> > > ...
> > > > +static int __devinit ce4100_i2c_probe(struct pci_dev *dev,
> > > > + const struct pci_device_id *ent)
> > > > +{
> > > > + sds = kzalloc(sizeof(*sds), GFP_KERNEL);
> > > > + if (!sds)
> > > > + goto err_mem;
> > > > +
> > > > + pci_set_drvdata(dev, sds);
> > > > +
> > > > + for (i = 0; i < ARRAY_SIZE(sds->sd); i++) {
> > > > + ret = add_i2c_device(dev, i, &sds->sd[i]);
> > > > + if (ret) {
> > > > + while (--i >= 0)
> > > > + platform_device_unregister(&sds->sd[i].pdev);
> > > ...
> > > > +static void __devexit ce4100_i2c_remove(struct pci_dev *dev)
> > > ...
> > > > + for (i = 0; i < ARRAY_SIZE(sds->sd); i++)
> > > > + platform_device_unregister(&sds->sd[i].pdev);
> > > > +
> > > > + pci_disable_device(dev);
> > > > + kfree(sds);
> > > > +}
> > >
> > > I see we're still repeating the same mistakes with lifetime rules of
> > > sysfs objects.
> > >
> > > Any struct device which has been registered into the device model can
> > > remain indefinitely live after it's been unregistered (by, eg, if
> > > userspace holds a reference to it via sysfs.)
> >
> > Actually this race is almost not possible these days with the rework
> > that Tejun did on sysfs, so it's not easy to test for this anymore.
>
> Is it wise to make such a problematical bug harder to trigger without
> completely preventing it triggering?
>
> A different approach was taken with IRQ handling where people were
> registering handlers before the driver was ready for it to be called -
> request_irq() would explicitly call the handler as soon as it was
> registered to provoke bugs.
>
> Surely for these lifetime violations a similar approach should be taken.
> Make the kernel more likely to oops should a violation occur before the
> developer can get the code out the door. One way I can think of doing
> that is when devices are unregistered but not yet released, place them
> on a list which is periodically scanned, and not only is the device
> dereferenced by also the release function.
>
> When the use count drops to zero, don't immediately release, but wait
> a number of polls.
>
> If either goes away before the device has been released, then we
> predictably oops, and the developer gets to know about his violation
> of the rules immediately.
Yes, that would be a good way to resolve this, patches are glady welcome
:)
thanks,
greg k-h
next prev parent reply other threads:[~2011-01-06 21:57 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-01-05 16:50 I2C support for CE4100, v3 Sebastian Andrzej Siewior
2011-01-05 16:50 ` [PATCH 1/6] i2c/pxa: use dynamic register layout Sebastian Andrzej Siewior
2011-01-05 16:50 ` [PATCH 2/6] arm/pxa2xx: reorganize I2C files Sebastian Andrzej Siewior
2011-01-05 16:51 ` [PATCH 3/6] i2c/pxa2xx: Add PCI support for PXA I2C controller Sebastian Andrzej Siewior
2011-01-05 20:21 ` Ben Dooks
2011-01-05 22:18 ` Sebastian Andrzej Siewior
2011-01-14 14:31 ` Sebastian Andrzej Siewior
2011-02-07 17:56 ` Sebastian Andrzej Siewior
2011-02-23 1:14 ` Ben Dooks
2011-01-05 23:03 ` Russell King - ARM Linux
2011-01-05 23:08 ` Greg KH
2011-01-06 9:20 ` Russell King - ARM Linux
2011-01-06 21:57 ` Greg KH [this message]
2011-01-07 12:31 ` [PATCH] i2c-pxa2xx: " Sebastian Andrzej Siewior
2011-01-06 10:50 ` [PATCH 3/6] i2c/pxa2xx: " Sebastian Andrzej Siewior
2011-01-06 11:12 ` Russell King - ARM Linux
2011-01-06 11:50 ` Sebastian Andrzej Siewior
2011-01-07 15:57 ` Grant Likely
2011-01-05 16:51 ` [PATCH 4/6] i2c/pxa2xx: add support for shared IRQ handler Sebastian Andrzej Siewior
2011-01-05 16:51 ` [PATCH 5/6] i2c/pxa2xx: check timeout correctly Sebastian Andrzej Siewior
2011-01-05 16:51 ` [PATCH 6/6] i2c/pxa2xx: pass of_node from platform driver to adapter and publish Sebastian Andrzej Siewior
2011-01-21 19:32 ` Grant Likely
2011-01-05 21:51 ` I2C support for CE4100, v3 Ben Dooks
2011-01-07 11:20 ` Sebastian Andrzej Siewior
-- strict thread matches above, loose matches on Subject: below --
2010-12-02 20:09 I2C support for CE4100, v2 Sebastian Andrzej Siewior
2010-12-02 20:09 ` [PATCH 3/6] i2c/pxa2xx: Add PCI support for PXA I2C controller Sebastian Andrzej Siewior
2010-12-14 14:35 ` Florian Fainelli
2011-01-05 17:26 ` Sebastian Andrzej Siewior
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=20110106215726.GB30799@suse.de \
--to=gregkh@suse$(echo .)de \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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