Messages in this thread | | | Date | Tue, 29 Oct 2013 09:34:28 -0700 | Subject | Re: [PATCH v3 1/3] msi: free msi_desc entry only after we've released the kobject | From | Linus Torvalds <> |
| |
On Tue, Oct 29, 2013 at 3:30 AM, Veaceslav Falico <vfalico@redhat.com> wrote: > /* > * Its possible that we get into this path > * When populate_msi_sysfs fails, which means the entries > * were not registered with sysfs. In that case don't > - * unregister them. > + * unregister them, and just free. Otherwise the > + * kobject->release will take care of freeing the entry via > + * msi_kobj_release(). > */ > if (entry->kobj.parent) { > kobject_del(&entry->kobj); > kobject_put(&entry->kobj); > + } else { > + kfree(entry); > } > - > - list_del(&entry->list); > - kfree(entry);
So this code sequence still makes me very unhappy.
Why does not just a simple unconditional
kobject_del(&entry->kobj); kobject_put(&entry->kobj);
work for the "not registered with sysfs" case? And if the sysfs code really gets confused, why not
if (entry->kobj.parent) kobject_del(&entry->kobj); kobject_put(&entry->kobj);
(btw, looking at the sysfs code, this looks *very* suspicious in sysfs_remove_dir():
struct sysfs_dirent *sd = kobj->sd;
spin_lock(&sysfs_assoc_lock); kobj->sd = NULL; spin_unlock(&sysfs_assoc_lock);
and I would suggest that "sd = kobj->sd" should be done under the lock, because otherwise the lock is kind of pointless..)
Greg?
Linus
| |