| Message ID | 20230322165830.55071-1-frank.li@vivo.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | [v3,01/10] kobject: introduce kobject_del_and_put() | expand |
On 3/23/23 01:58, Yangtao Li wrote: > There are plenty of using kobject_del() and kobject_put() together > in the kernel tree. This patch wraps these two calls in a single helper. > > Signed-off-by: Yangtao Li <frank.li@vivo.com> > --- > v3: > -convert to inline helper > v2: > -add kobject_del_and_put() users > include/linux/kobject.h | 13 +++++++++++++ > lib/kobject.c | 3 +-- > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/include/linux/kobject.h b/include/linux/kobject.h > index bdab370a24f4..e21b7c22e355 100644 > --- a/include/linux/kobject.h > +++ b/include/linux/kobject.h > @@ -112,6 +112,19 @@ extern struct kobject * __must_check kobject_get_unless_zero( > struct kobject *kobj); > extern void kobject_put(struct kobject *kobj); > > +/** > + * kobject_del_and_put() - Delete kobject. > + * @kobj: object. > + * > + * Unlink kobject from hierarchy and decrement the refcount. Unlink kobject from hierarchy and decrement its refcount. > + * If refcount is 0, call kobject_cleanup(). That is done by kobject_put() and not explicitly done directly in this helper. So I would not mention this to avoid confusion as you otherwise have a description that does not match the code we can see here. With that fixed, this looks OK to me, so feel free to add: Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> > + */ > +static inline void kobject_del_and_put(struct kobject *kobj) > +{ > + kobject_del(kobj); > + kobject_put(kobj); > +} > + > extern const void *kobject_namespace(const struct kobject *kobj); > extern void kobject_get_ownership(const struct kobject *kobj, > kuid_t *uid, kgid_t *gid); > diff --git a/lib/kobject.c b/lib/kobject.c > index f79a434e1231..e6c5a3ff1c53 100644 > --- a/lib/kobject.c > +++ b/lib/kobject.c > @@ -876,8 +876,7 @@ void kset_unregister(struct kset *k) > { > if (!k) > return; > - kobject_del(&k->kobj); > - kobject_put(&k->kobj); > + kobject_del_and_put(&k->kobj); Nit: You could simplify this one to be: if (k) kobject_del_and_put(&k->kobj); and drop the return line. > } > EXPORT_SYMBOL(kset_unregister); >
On Thu, Mar 23, 2023 at 12:58:30AM +0800, Yangtao Li wrote: > There are plenty of using kobject_del() and kobject_put() together > in the kernel tree. This patch wraps these two calls in a single helper. > > Signed-off-by: Yangtao Li <frank.li@vivo.com> > --- > v3: > -convert to inline helper > v2: > -add kobject_del_and_put() users > include/linux/kobject.h | 13 +++++++++++++ > lib/kobject.c | 3 +-- > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/include/linux/kobject.h b/include/linux/kobject.h > index bdab370a24f4..e21b7c22e355 100644 > --- a/include/linux/kobject.h > +++ b/include/linux/kobject.h > @@ -112,6 +112,19 @@ extern struct kobject * __must_check kobject_get_unless_zero( > struct kobject *kobj); > extern void kobject_put(struct kobject *kobj); > > +/** > + * kobject_del_and_put() - Delete kobject. > + * @kobj: object. > + * > + * Unlink kobject from hierarchy and decrement the refcount. > + * If refcount is 0, call kobject_cleanup(). > + */ > +static inline void kobject_del_and_put(struct kobject *kobj) > +{ > + kobject_del(kobj); > + kobject_put(kobj); > +} kobject_put() actually covers kobject removal automatically, which is single stage removal. So if you see the two called together, it is safe to kill kobject_del() directly. thanks, Ming
On Thu, Mar 23, 2023 at 12:58:30AM +0800, Yangtao Li wrote: > There are plenty of using kobject_del() and kobject_put() together > in the kernel tree. This patch wraps these two calls in a single helper. > > Signed-off-by: Yangtao Li <frank.li@vivo.com> > --- > v3: > -convert to inline helper > v2: > -add kobject_del_and_put() users > include/linux/kobject.h | 13 +++++++++++++ > lib/kobject.c | 3 +-- > 2 files changed, 14 insertions(+), 2 deletions(-) Meta-comment, something is wrong with this email as it is not linked to the rest of the series. You can see that by looking at this message in lore.kernel.org: https://lore.kernel.org/r/20230322165830.55071-1-frank.li@vivo.com No 2-10 patches linked there (they show up as a separate series.) So even if I wanted to take this series now, we can't as our tools can't find them... thanks, greg k-h
> kobject_put() actually covers kobject removal automatically, which is > single stage removal. So if you see the two called together, it is > safe to kill kobject_del() directly. If the reference count is not abnormal, kobject_put() does contain what kobject_del() does. Thx, Yangtao
diff --git a/include/linux/kobject.h b/include/linux/kobject.h index bdab370a24f4..e21b7c22e355 100644 --- a/include/linux/kobject.h +++ b/include/linux/kobject.h @@ -112,6 +112,19 @@ extern struct kobject * __must_check kobject_get_unless_zero( struct kobject *kobj); extern void kobject_put(struct kobject *kobj); +/** + * kobject_del_and_put() - Delete kobject. + * @kobj: object. + * + * Unlink kobject from hierarchy and decrement the refcount. + * If refcount is 0, call kobject_cleanup(). + */ +static inline void kobject_del_and_put(struct kobject *kobj) +{ + kobject_del(kobj); + kobject_put(kobj); +} + extern const void *kobject_namespace(const struct kobject *kobj); extern void kobject_get_ownership(const struct kobject *kobj, kuid_t *uid, kgid_t *gid); diff --git a/lib/kobject.c b/lib/kobject.c index f79a434e1231..e6c5a3ff1c53 100644 --- a/lib/kobject.c +++ b/lib/kobject.c @@ -876,8 +876,7 @@ void kset_unregister(struct kset *k) { if (!k) return; - kobject_del(&k->kobj); - kobject_put(&k->kobj); + kobject_del_and_put(&k->kobj); } EXPORT_SYMBOL(kset_unregister);
There are plenty of using kobject_del() and kobject_put() together in the kernel tree. This patch wraps these two calls in a single helper. Signed-off-by: Yangtao Li <frank.li@vivo.com> --- v3: -convert to inline helper v2: -add kobject_del_and_put() users include/linux/kobject.h | 13 +++++++++++++ lib/kobject.c | 3 +-- 2 files changed, 14 insertions(+), 2 deletions(-)