Prev: [PATCH] dmi-id: fix a memory leak in dmi_id_init error path
Next: [git pull] drm + agp + one fb (v2)
From: Greg KH on 15 Jul 2010 13:30
On Thu, Jul 15, 2010 at 06:10:29PM +0100, Russell King - ARM Linux wrote:
> On Thu, Jul 15, 2010 at 09:51:41AM -0700, Greg KH wrote:
> > On Thu, Jul 15, 2010 at 05:35:26PM +0100, Russell King - ARM Linux wrote:
> > > On Thu, Jul 15, 2010 at 11:16:30PM +0800, Conke Hu wrote:
> > > > ever notice to the following kernel log?
> > > > "Device 's3c2440-nand' does not have a release() function, it is
> > > > broken and must."
> > > > release() function should be implemented in the platform_device.
> > >
> > > That's not telling you to provide a release function. The warning is
> > > telling you that a device is being unregistered which doesn't have a
> > > release function.
> > >
> > > Consider this point - maybe it doesn't have a release function because
> > > it's not supposed to be unregistered?
> >
> > Heh, well, it is being unregistered, and at that point in time, the
> > kernel complains.
>
> The unregistration occurs because platform_register_devices() (which is
> used by arch code to register a block of platform devices) undoes its
> work if one of the devices fails to register.
>
> I've long since thought, since I created that function, that this probably
> isn't desirable behaviour, and it should continue to register as many
> devices as it possibly can.
>
> > All kobjects need a release function that actually frees it. If not,
> > that is a logic bug. Please see the Documentation/kobject.txt file for
> > details.
>
> How do you free a statically declared platform device?

You never unregister it :)

Otherwise, you will have problems with the reference counting logic, and
you should make it a dynamic device. I really want to move to forcing
all devices to be created dynamically, but the platform devices are the
biggest static device users, so I've not been pushing it very hard, due
to other more pressing issues...

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Russell King - ARM Linux on 15 Jul 2010 13:40
On Thu, Jul 15, 2010 at 10:22:18AM -0700, Greg KH wrote:
> On Thu, Jul 15, 2010 at 06:10:29PM +0100, Russell King - ARM Linux wrote:
> > On Thu, Jul 15, 2010 at 09:51:41AM -0700, Greg KH wrote:
> > > On Thu, Jul 15, 2010 at 05:35:26PM +0100, Russell King - ARM Linux wrote:
> > > > On Thu, Jul 15, 2010 at 11:16:30PM +0800, Conke Hu wrote:
> > > > > ever notice to the following kernel log?
> > > > > "Device 's3c2440-nand' does not have a release() function, it is
> > > > > broken and must."
> > > > > release() function should be implemented in the platform_device.
> > > >
> > > > That's not telling you to provide a release function. The warning is
> > > > telling you that a device is being unregistered which doesn't have a
> > > > release function.
> > > >
> > > > Consider this point - maybe it doesn't have a release function because
> > > > it's not supposed to be unregistered?
> > >
> > > Heh, well, it is being unregistered, and at that point in time, the
> > > kernel complains.
> >
> > The unregistration occurs because platform_register_devices() (which is
> > used by arch code to register a block of platform devices) undoes its
> > work if one of the devices fails to register.
> >
> > I've long since thought, since I created that function, that this probably
> > isn't desirable behaviour, and it should continue to register as many
> > devices as it possibly can.
> >
> > > All kobjects need a release function that actually frees it. If not,
> > > that is a logic bug. Please see the Documentation/kobject.txt file for
> > > details.
> >
> > How do you free a statically declared platform device?
>
> You never unregister it :)

Indeed, and one way to do that is to fix the double-registration of
dm9000.0. Another way to avoid the other complaints is to remove
the unregistration in platform_register_devices().
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on 15 Jul 2010 17:00
On Thu, Jul 15, 2010 at 06:33:22PM +0100, Russell King - ARM Linux wrote:
> On Thu, Jul 15, 2010 at 10:22:18AM -0700, Greg KH wrote:
> > On Thu, Jul 15, 2010 at 06:10:29PM +0100, Russell King - ARM Linux wrote:
> > > On Thu, Jul 15, 2010 at 09:51:41AM -0700, Greg KH wrote:
> > > > On Thu, Jul 15, 2010 at 05:35:26PM +0100, Russell King - ARM Linux wrote:
> > > > > On Thu, Jul 15, 2010 at 11:16:30PM +0800, Conke Hu wrote:
> > > > > > ever notice to the following kernel log?
> > > > > > "Device 's3c2440-nand' does not have a release() function, it is
> > > > > > broken and must."
> > > > > > release() function should be implemented in the platform_device.
> > > > >
> > > > > That's not telling you to provide a release function. The warning is
> > > > > telling you that a device is being unregistered which doesn't have a
> > > > > release function.
> > > > >
> > > > > Consider this point - maybe it doesn't have a release function because
> > > > > it's not supposed to be unregistered?
> > > >
> > > > Heh, well, it is being unregistered, and at that point in time, the
> > > > kernel complains.
> > >
> > > The unregistration occurs because platform_register_devices() (which is
> > > used by arch code to register a block of platform devices) undoes its
> > > work if one of the devices fails to register.
> > >
> > > I've long since thought, since I created that function, that this probably
> > > isn't desirable behaviour, and it should continue to register as many
> > > devices as it possibly can.
> > >
> > > > All kobjects need a release function that actually frees it. If not,
> > > > that is a logic bug. Please see the Documentation/kobject.txt file for
> > > > details.
> > >
> > > How do you free a statically declared platform device?
> >
> > You never unregister it :)
>
> Indeed, and one way to do that is to fix the double-registration of
> dm9000.0.

I agree.

> Another way to avoid the other complaints is to remove
> the unregistration in platform_register_devices().

No, because you could have created a platform device with a call to
platform_device_alloc() and then called platform_device_register() and
then later, platform_device_unregister(), right?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Russell King - ARM Linux on 15 Jul 2010 17:20
On Thu, Jul 15, 2010 at 01:51:53PM -0700, Greg KH wrote:
> On Thu, Jul 15, 2010 at 06:33:22PM +0100, Russell King - ARM Linux wrote:
> > Another way to avoid the other complaints is to remove
> > the unregistration in platform_register_devices().
>
> No, because you could have created a platform device with a call to
> platform_device_alloc() and then called platform_device_register() and
> then later, platform_device_unregister(), right?

No. Sorry, platform_register_devices should've been platform_add_devices().
Please look at that function.

It's used to register an array of static platform devices from architecture
code, as I've said in this thread once already.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Conke Hu on 16 Jul 2010 01:50
On Fri, Jul 16, 2010 at 12:35 AM, Russell King - ARM Linux
<linux(a)arm.linux.org.uk> wrote:
> On Thu, Jul 15, 2010 at 11:16:30PM +0800, Conke Hu wrote:
>> ever notice to the following kernel log?
>> "Device 's3c2440-nand' does not have a release() function, it is
>> broken and must."
>> release() function should be implemented �in the platform_device.
>
> That's not telling you to provide a release function. �The warning is
> telling you that a device is being unregistered which doesn't have a
> release function.
>
> Consider this point - maybe it doesn't have a release function because
> it's not supposed to be unregistered?
>
>> > ------------[ cut here ]------------
>> > WARNING: at fs/sysfs/dir.c:463 sysfs_add_one+0x30/0x44()
>> > sysfs: duplicate filename 'dm9000.0' can not be created
>
> This is the cause of the problem. �You're registering two dm9000.0
> devices. �That's illegal, and it then causes all these:
>
>> > WARNING: at drivers/base/core.c:122 device_release+0x6c/0x78()
>> > Device 's3c24xx_led.3' does not have a release() function, it is broken and mus.
>> > WARNING: at drivers/base/core.c:122 device_release+0x6c/0x78()
>> > Device 's3c24xx_led.2' does not have a release() function, it is broken and mus.
>> > WARNING: at drivers/base/core.c:122 device_release+0x6c/0x78()
>> > Device 's3c24xx_led.1' does not have a release() function, it is broken and mus.
>> > WARNING: at drivers/base/core.c:122 device_release+0x6c/0x78()
>> > Device 's3c24xx_led.0' does not have a release() function, it is broken and mus.
>> > WARNING: at drivers/base/core.c:122 device_release+0x6c/0x78()
>> > Device 's3c2440-nand' does not have a release() function, it is broken and must.
> ...
>
> Fix the first problem (which is the real one) and you won't have these
> other problems (which aren't actually a problem.)
>

Right:)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
First  |  Prev  |  Next  |  Last
Pages: 1 2 3
Prev: [PATCH] dmi-id: fix a memory leak in dmi_id_init error path
Next: [git pull] drm + agp + one fb (v2)