* [PATCH] lib/kobject: Simplify checking of parameters in kobj_ns_type_register.
@ 2022-06-17 8:38 wuchi
2022-06-17 8:49 ` Greg KH
0 siblings, 1 reply; 3+ messages in thread
From: wuchi @ 2022-06-17 8:38 UTC (permalink / raw)
To: gregkh, rafael; +Cc: linux-kernel
1. Merge checking of following code:
if (type >= KOBJ_NS_TYPES)
...
if (type <= KOBJ_NS_TYPE_NONE)
2. Move the checking of parameters out of critical section, there is
no need to check that with spinlock.
Signed-off-by: wuchi <wuchi.zero@gmail.com>
---
lib/kobject.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/lib/kobject.c b/lib/kobject.c
index 5f0e71ab292c..6a8b009682b8 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -996,19 +996,13 @@ static const struct kobj_ns_type_operations *kobj_ns_ops_tbl[KOBJ_NS_TYPES];
int kobj_ns_type_register(const struct kobj_ns_type_operations *ops)
{
enum kobj_ns_type type = ops->type;
- int error;
-
- spin_lock(&kobj_ns_type_lock);
+ int error = -EBUSY;
- error = -EINVAL;
- if (type >= KOBJ_NS_TYPES)
- goto out;
+ if (unlikely(type >= KOBJ_NS_TYPES || type <= KOBJ_NS_TYPE_NONE))
+ return -EINVAL;
- error = -EINVAL;
- if (type <= KOBJ_NS_TYPE_NONE)
- goto out;
+ spin_lock(&kobj_ns_type_lock);
- error = -EBUSY;
if (kobj_ns_ops_tbl[type])
goto out;
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] lib/kobject: Simplify checking of parameters in kobj_ns_type_register.
2022-06-17 8:38 [PATCH] lib/kobject: Simplify checking of parameters in kobj_ns_type_register wuchi
@ 2022-06-17 8:49 ` Greg KH
2022-06-17 9:55 ` chi wu
0 siblings, 1 reply; 3+ messages in thread
From: Greg KH @ 2022-06-17 8:49 UTC (permalink / raw)
To: wuchi; +Cc: rafael, linux-kernel
On Fri, Jun 17, 2022 at 04:38:32PM +0800, wuchi wrote:
> 1. Merge checking of following code:
> if (type >= KOBJ_NS_TYPES)
> ...
> if (type <= KOBJ_NS_TYPE_NONE)
Why?
> 2. Move the checking of parameters out of critical section, there is
> no need to check that with spinlock.
Why does it matter?
>
> Signed-off-by: wuchi <wuchi.zero@gmail.com>
> ---
> lib/kobject.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/lib/kobject.c b/lib/kobject.c
> index 5f0e71ab292c..6a8b009682b8 100644
> --- a/lib/kobject.c
> +++ b/lib/kobject.c
> @@ -996,19 +996,13 @@ static const struct kobj_ns_type_operations *kobj_ns_ops_tbl[KOBJ_NS_TYPES];
> int kobj_ns_type_register(const struct kobj_ns_type_operations *ops)
> {
> enum kobj_ns_type type = ops->type;
> - int error;
> -
> - spin_lock(&kobj_ns_type_lock);
> + int error = -EBUSY;
>
> - error = -EINVAL;
> - if (type >= KOBJ_NS_TYPES)
> - goto out;
> + if (unlikely(type >= KOBJ_NS_TYPES || type <= KOBJ_NS_TYPE_NONE))
Why add unlikely? Did you measure the performance benifit? If not,
please never add likely/unlikely.
> + return -EINVAL;
>
> - error = -EINVAL;
> - if (type <= KOBJ_NS_TYPE_NONE)
> - goto out;
> + spin_lock(&kobj_ns_type_lock);
>
> - error = -EBUSY;
How did you test this? What is the benefit?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] lib/kobject: Simplify checking of parameters in kobj_ns_type_register.
2022-06-17 8:49 ` Greg KH
@ 2022-06-17 9:55 ` chi wu
0 siblings, 0 replies; 3+ messages in thread
From: chi wu @ 2022-06-17 9:55 UTC (permalink / raw)
To: Greg KH; +Cc: rafael, linux-kernel
Greg KH <gregkh@linuxfoundation.org> 于2022年6月17日周五 16:49写道:
>
> On Fri, Jun 17, 2022 at 04:38:32PM +0800, wuchi wrote:
> > 1. Merge checking of following code:
> > if (type >= KOBJ_NS_TYPES)
> > ...
> > if (type <= KOBJ_NS_TYPE_NONE)
>
> Why?
As lib/kobjecct.c following code in kobj_ns_type_register():
L998: enum kobj_ns_type type = ops->type;
L999: int error;
L996: int kobj_ns_type_register(const struct kobj_ns_type_operations *ops)
L1001: spin_lock(&kobj_ns_type_lock);
L1003: error = -EINVAL;
L1004: if (type >= KOBJ_NS_TYPES)
L1005: goto out;
L1007: error = -EINVAL;
L1008: if (type <= KOBJ_NS_TYPE_NONE)
L1009: goto out;
L1011: error = -EBUSY;
...
L1018: out:
L1019: spin_unlock(&kobj_ns_type_lock);
L1003~L1005 and L1007~1009 set twice vaule and do the similar checking.
it is redundant code, So just simplify that.
>
> > 2. Move the checking of parameters out of critical section, there is
> > no need to check that with spinlock.
>
> Why does it matter?
>
[2]:
The result is ok, but just want to reduce the critical section code,
as @ops->type is input arg and KOBJ_NS_{TYPES|TYPE_NONE} is enum, so it
seems to be better to put the checking out of spinlock.
>
> >
> > Signed-off-by: wuchi <wuchi.zero@gmail.com>
> > ---
> > lib/kobject.c | 14 ++++----------
> > 1 file changed, 4 insertions(+), 10 deletions(-)
> >
> > diff --git a/lib/kobject.c b/lib/kobject.c
> > index 5f0e71ab292c..6a8b009682b8 100644
> > --- a/lib/kobject.c
> > +++ b/lib/kobject.c
> > @@ -996,19 +996,13 @@ static const struct kobj_ns_type_operations *kobj_ns_ops_tbl[KOBJ_NS_TYPES];
> > int kobj_ns_type_register(const struct kobj_ns_type_operations *ops)
> > {
> > enum kobj_ns_type type = ops->type;
> > - int error;
> > -
> > - spin_lock(&kobj_ns_type_lock);
> > + int error = -EBUSY;
> >
> > - error = -EINVAL;
> > - if (type >= KOBJ_NS_TYPES)
> > - goto out;
> > + if (unlikely(type >= KOBJ_NS_TYPES || type <= KOBJ_NS_TYPE_NONE))
>
> Why add unlikely? Did you measure the performance benifit? If not,
> please never add likely/unlikely.
[3]
Yes, just want to get some performance benifit, but i am sorry for that I didn't
do the test, but just see the only call in netdev_kobject_init from
net/core/net-sysfs.c:
kobj_ns_type_register(&net_ns_type_operations);
and the type of net_ns_type_operations is KOBJ_NS_TYPE_NET which is valid.
>
> > + return -EINVAL;
> >
> > - error = -EINVAL;
> > - if (type <= KOBJ_NS_TYPE_NONE)
> > - goto out;
> > + spin_lock(&kobj_ns_type_lock);
> >
> > - error = -EBUSY;
>
>
> How did you test this? What is the benefit?
As the reply [2] [3] snippet, and just for a little bit of possible
performance to reduce the
preempt_disable time though it just called when system is bootting.
thanks for greg k-h's time
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-06-17 9:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-17 8:38 [PATCH] lib/kobject: Simplify checking of parameters in kobj_ns_type_register wuchi
2022-06-17 8:49 ` Greg KH
2022-06-17 9:55 ` chi wu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.