Messages in this thread | | | Date | Mon, 11 Oct 2010 14:32:35 -0700 | From | Greg KH <> | Subject | Re: [PATCH 8/19]: SCST SYSFS interface implementation |
| |
On Mon, Oct 11, 2010 at 11:29:22PM +0400, Vladislav Bolkhovitin wrote: > Greg KH, on 10/10/2010 01:20 AM wrote: > > On Sat, Oct 02, 2010 at 01:46:21AM +0400, Vladislav Bolkhovitin wrote: > >> +static void scst_tgtt_release(struct kobject *kobj) > >> +{ > >> + struct scst_tgt_template *tgtt; > >> + > >> + tgtt = container_of(kobj, struct scst_tgt_template, tgtt_kobj); > >> + complete_all(&tgtt->tgtt_kobj_release_cmpl); > >> + return; > > > > Don't you also need to free the memory of your kobject here? > > > >> +static void scst_tgt_release(struct kobject *kobj) > >> +{ > >> + struct scst_tgt *tgt; > >> + > >> + tgt = container_of(kobj, struct scst_tgt, tgt_kobj); > >> + complete_all(&tgt->tgt_kobj_release_cmpl); > >> + return; > > > > Same here, no kfree? > > > >> +static void scst_acg_release(struct kobject *kobj) > >> +{ > >> + struct scst_acg *acg; > >> + > >> + acg = container_of(kobj, struct scst_acg, acg_kobj); > >> + complete_all(&acg->acg_kobj_release_cmpl); > > > > And here. > > Thanks for the review. In all those functions kobjects for simplicity > are embedded into the outer objects, so they will be freed as part of > the outer objects free. Hence, kfree() for the kobjects in the release > functions are not needed.
Sweet, you now have opened yourself up to public ridicule as per the documentation in the kernel for how to use kobjects!
Nice job :)
Seriously, you CAN NOT DO THIS! If you embed a kobject in a different structure, then you have to rely on the kobject to handle the reference counting for that larger structure. To do ANYTHING else is a bug and wrong.
Please read the kobject documentation and fix this code up before submitting it again.
thanks,
greg k-h
| |