dm: sysfs add empty release function to avoid debug warning
in [Kernel]
|
Prev: [PATCH] OneNAND: Fix test of unsigned in onenand_otp_walk()
Next: cgroup: implement eventfd-based generic API for notifications
From: Greg KH on 15 Dec 2009 19:50 On Tue, Dec 15, 2009 at 05:35:08PM +0000, James Bottomley wrote: > commit: d2bb7df8cac647b92f51fb84ae735771e7adbfa7 > From: Milan Broz <mbroz(a)redhat.com> > Date: Thu, 10 Dec 2009 23:51:53 +0000 > Subject: [PATCH] dm: sysfs add empty release function to avoid debug warning > > This patch just removes an unnecessary warning: > kobject: 'dm': does not have a release() function, > it is broken and must be fixed. > > The kobject is embedded in mapped device struct, so > code does not need to release memory explicitly here. Oh no you did not! Come on people, do you think that the kernel is just randomly spitting out warnings because it fricken feels like it! No, please revert this patch now before someone does it for you. Please, this is totally and completly wrong. And if you feel that it is needed, then your design is wrong and it needs to be fixed. And note, as per the Documentation/kobject.txt file, you were warned about this public mocking, next time it will be harsher. Heh, and you tried to get this into a -stable kernel release, as if. Patch left below so that others can mock. bah, greg k-h > > Cc: stable(a)kernel.org > Signed-off-by: Milan Broz <mbroz(a)redhat.com> > Signed-off-by: Alasdair G Kergon <agk(a)redhat.com> > --- > drivers/md/dm-sysfs.c | 8 ++++++++ > 1 files changed, 8 insertions(+), 0 deletions(-) > > diff --git a/drivers/md/dm-sysfs.c b/drivers/md/dm-sysfs.c > index 4b04590..b000de3 100644 > --- a/drivers/md/dm-sysfs.c > +++ b/drivers/md/dm-sysfs.c > @@ -80,12 +80,20 @@ static struct sysfs_ops dm_sysfs_ops = { > }; > > /* > + * The sysfs structure is embedded in md struct, nothing to do here > + */ > +static void dm_sysfs_release(struct kobject *kobj) > +{ > +} > + > +/* > * dm kobject is embedded in mapped_device structure > * no need to define release function here > */ > static struct kobj_type dm_ktype = { > .sysfs_ops = &dm_sysfs_ops, > .default_attrs = dm_attrs, > + .release = dm_sysfs_release > }; > > /* > > _______________________________________________ > stable mailing list > stable(a)linux.kernel.org > http://linux.kernel.org/mailman/listinfo/stable -- 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: Milan Broz on 16 Dec 2009 04:50 On 12/16/2009 01:47 AM, Greg KH wrote: > On Tue, Dec 15, 2009 at 05:35:08PM +0000, James Bottomley wrote: >> commit: d2bb7df8cac647b92f51fb84ae735771e7adbfa7 >> From: Milan Broz <mbroz(a)redhat.com> >> Date: Thu, 10 Dec 2009 23:51:53 +0000 >> Subject: [PATCH] dm: sysfs add empty release function to avoid debug warning >> >> This patch just removes an unnecessary warning: >> kobject: 'dm': does not have a release() function, >> it is broken and must be fixed. >> >> The kobject is embedded in mapped device struct, so >> code does not need to release memory explicitly here. > > Please, this is totally and completly wrong. And if you feel that it is > needed, then your design is wrong and it needs to be fixed. There are several places in kernel, where kobject have not defined release method. Yes, something is wrong here. So quietly ignoring warning is ok? Why is not there BUG_ON(!release) then? The sysfs attributes here just represents attributes of block device object, this device is always removed before release here is called. So if there is preferred to do another alloc/free, no problem. > And note, as per the Documentation/kobject.txt file, you were warned > about this public mocking, next time it will be harsher. > > Heh, and you tried to get this into a -stable kernel release, as if. btw I did'n added stable@, someone added that later. But it is not relevant. > Patch left below so that others can mock. Enjoy;-) Milan -- 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 16 Dec 2009 09:00
On Wed, Dec 16, 2009 at 10:44:41AM +0100, Milan Broz wrote: > On 12/16/2009 01:47 AM, Greg KH wrote: > > On Tue, Dec 15, 2009 at 05:35:08PM +0000, James Bottomley wrote: > >> commit: d2bb7df8cac647b92f51fb84ae735771e7adbfa7 > >> From: Milan Broz <mbroz(a)redhat.com> > >> Date: Thu, 10 Dec 2009 23:51:53 +0000 > >> Subject: [PATCH] dm: sysfs add empty release function to avoid debug warning > >> > >> This patch just removes an unnecessary warning: > >> kobject: 'dm': does not have a release() function, > >> it is broken and must be fixed. > >> > >> The kobject is embedded in mapped device struct, so > >> code does not need to release memory explicitly here. > > > > > Please, this is totally and completly wrong. And if you feel that it is > > needed, then your design is wrong and it needs to be fixed. > > There are several places in kernel, where kobject have not defined release > method. Yes, something is wrong here. Where are those instances, becides the use of static kobjects, which is being worked on? That still does not make this change acceptable, it is incorrect. > So quietly ignoring warning is ok? Why is not there BUG_ON(!release) then? I was trying to be nice and not crash your machine and give you the opportunity to fix it easily. > The sysfs attributes here just represents attributes of block device object, > this device is always removed before release here is called. Then why use a kobject at all? Your reference counting will be all wrong if you use it incorrectly like this. > So if there is preferred to do another alloc/free, no problem. Please do. Also, with the addition of this patch, your comments for the kobject are now incorrect as well. 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/ |