Namespace: Optimize the namespace code

[PATCH v1] Namespace: Optimize the namespace code

Posted by Xavier 1 year, 6 months ago

Modify variable names in the function clone_time_ns to enhance
readability, and optimize the memory release operation by placing
it outside of the spin lock in alloc_ucounts.

Signed-off-by: Xavier <xavier_qy@163.com>
---
 kernel/time/namespace.c | 30 +++++++++++++++---------------
 kernel/ucount.c         |  9 +++++----
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/kernel/time/namespace.c b/kernel/time/namespace.c
index 0775b9ec952a..4c0836fb845b 100644
--- a/kernel/time/namespace.c
+++ b/kernel/time/namespace.c
@@ -78,7 +78,7 @@ static void dec_time_namespaces(struct ucounts *ucounts)
 static struct time_namespace *clone_time_ns(struct user_namespace *user_ns,
 					  struct time_namespace *old_ns)
 {
-	struct time_namespace *ns;
+	struct time_namespace *time_ns;
 	struct ucounts *ucounts;
 	int err;
 
@@ -88,31 +88,31 @@ static struct time_namespace *clone_time_ns(struct user_namespace *user_ns,
 		goto fail;
 
 	err = -ENOMEM;
-	ns = kmalloc(sizeof(*ns), GFP_KERNEL_ACCOUNT);
-	if (!ns)
+	time_ns = kmalloc(sizeof(*time_ns), GFP_KERNEL_ACCOUNT);
+	if (!time_ns)
 		goto fail_dec;
 
-	refcount_set(&ns->ns.count, 1);
+	refcount_set(&time_ns->ns.count, 1);
 
-	ns->vvar_page = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO);
-	if (!ns->vvar_page)
+	time_ns->vvar_page = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO);
+	if (!time_ns->vvar_page)
 		goto fail_free;
 
-	err = ns_alloc_inum(&ns->ns);
+	err = ns_alloc_inum(&time_ns->ns);
 	if (err)
 		goto fail_free_page;
 
-	ns->ucounts = ucounts;
-	ns->ns.ops = &timens_operations;
-	ns->user_ns = get_user_ns(user_ns);
-	ns->offsets = old_ns->offsets;
-	ns->frozen_offsets = false;
-	return ns;
+	time_ns->ucounts = ucounts;
+	time_ns->ns.ops = &timens_operations;
+	time_ns->user_ns = get_user_ns(user_ns);
+	time_ns->offsets = old_ns->offsets;
+	time_ns->frozen_offsets = false;
+	return time_ns;
 
 fail_free_page:
-	__free_page(ns->vvar_page);
+	__free_page(time_ns->vvar_page);
 fail_free:
-	kfree(ns);
+	kfree(time_ns);
 fail_dec:
 	dec_time_namespaces(ucounts);
 fail:
diff --git a/kernel/ucount.c b/kernel/ucount.c
index 8c07714ff27d..87a773e6ff15 100644
--- a/kernel/ucount.c
+++ b/kernel/ucount.c
@@ -164,7 +164,7 @@ struct ucounts *get_ucounts(struct ucounts *ucounts)
 struct ucounts *alloc_ucounts(struct user_namespace *ns, kuid_t uid)
 {
 	struct hlist_head *hashent = ucounts_hashentry(ns, uid);
-	struct ucounts *ucounts, *new;
+	struct ucounts *ucounts, *new = NULL;
 	bool wrapped;
 
 	spin_lock_irq(&ucounts_lock);
@@ -182,9 +182,7 @@ struct ucounts *alloc_ucounts(struct user_namespace *ns, kuid_t uid)
 
 		spin_lock_irq(&ucounts_lock);
 		ucounts = find_ucounts(ns, uid, hashent);
-		if (ucounts) {
-			kfree(new);
-		} else {
+		if (!ucounts) {
 			hlist_add_head(&new->node, hashent);
 			get_user_ns(new->ns);
 			spin_unlock_irq(&ucounts_lock);
@@ -193,6 +191,9 @@ struct ucounts *alloc_ucounts(struct user_namespace *ns, kuid_t uid)
 	}
 	wrapped = !get_ucounts_or_wrap(ucounts);
 	spin_unlock_irq(&ucounts_lock);
+	if (new)
+		kfree(new);
+
 	if (wrapped) {
 		put_ucounts(ucounts);
 		return NULL;
-- 
2.45.2

Re: [PATCH v1] Namespace: Optimize the namespace code

Posted by Thomas Gleixner 1 year, 6 months ago

On Fri, Aug 09 2024 at 20:17, Xavier wrote:
> Modify variable names in the function clone_time_ns to enhance
> readability, and optimize the memory release operation by placing
> it outside of the spin lock in alloc_ucounts.

Renaming variables is not an optimization. Don't do two things at once.

Also the rename is not adding any value. This code is about time
namespace so 'ns' is perfectly understandable.

Thanks,

        tglx

[PATCH v2] Ucount: Optimize the ucount code

Posted by Xavier 1 year, 5 months ago

Optimize the memory release operation by placing it outside of the
spin lock in alloc_ucounts.

Signed-off-by: Xavier <xavier_qy@163.com>
---
 kernel/ucount.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/kernel/ucount.c b/kernel/ucount.c
index 8c07714ff27d..87a773e6ff15 100644
--- a/kernel/ucount.c
+++ b/kernel/ucount.c
@@ -164,7 +164,7 @@ struct ucounts *get_ucounts(struct ucounts *ucounts)
 struct ucounts *alloc_ucounts(struct user_namespace *ns, kuid_t uid)
 {
 	struct hlist_head *hashent = ucounts_hashentry(ns, uid);
-	struct ucounts *ucounts, *new;
+	struct ucounts *ucounts, *new = NULL;
 	bool wrapped;
 
 	spin_lock_irq(&ucounts_lock);
@@ -182,9 +182,7 @@ struct ucounts *alloc_ucounts(struct user_namespace *ns, kuid_t uid)
 
 		spin_lock_irq(&ucounts_lock);
 		ucounts = find_ucounts(ns, uid, hashent);
-		if (ucounts) {
-			kfree(new);
-		} else {
+		if (!ucounts) {
 			hlist_add_head(&new->node, hashent);
 			get_user_ns(new->ns);
 			spin_unlock_irq(&ucounts_lock);
@@ -193,6 +191,9 @@ struct ucounts *alloc_ucounts(struct user_namespace *ns, kuid_t uid)
 	}
 	wrapped = !get_ucounts_or_wrap(ucounts);
 	spin_unlock_irq(&ucounts_lock);
+	if (new)
+		kfree(new);
+
 	if (wrapped) {
 		put_ucounts(ucounts);
 		return NULL;
-- 
2.45.2

Re:[PATCH v2] Ucount: Optimize the ucount code

Posted by Xavier 1 year, 5 months ago

Hi,

Just a reminder, does anyone have any comments or feedback on the patch?

--
Thanks,
Xavier




At 2024-08-12 16:48:23, "Xavier" <xavier_qy@163.com> wrote:
>Optimize the memory release operation by placing it outside of the
>spin lock in alloc_ucounts.
>
>Signed-off-by: Xavier <xavier_qy@163.com>
>---
> kernel/ucount.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
>diff --git a/kernel/ucount.c b/kernel/ucount.c
>index 8c07714ff27d..87a773e6ff15 100644
>--- a/kernel/ucount.c
>+++ b/kernel/ucount.c
>@@ -164,7 +164,7 @@ struct ucounts *get_ucounts(struct ucounts *ucounts)
> struct ucounts *alloc_ucounts(struct user_namespace *ns, kuid_t uid)
> {
> 	struct hlist_head *hashent = ucounts_hashentry(ns, uid);
>-	struct ucounts *ucounts, *new;
>+	struct ucounts *ucounts, *new = NULL;
> 	bool wrapped;
> 
> 	spin_lock_irq(&ucounts_lock);
>@@ -182,9 +182,7 @@ struct ucounts *alloc_ucounts(struct user_namespace *ns, kuid_t uid)
> 
> 		spin_lock_irq(&ucounts_lock);
> 		ucounts = find_ucounts(ns, uid, hashent);
>-		if (ucounts) {
>-			kfree(new);
>-		} else {
>+		if (!ucounts) {
> 			hlist_add_head(&new->node, hashent);
> 			get_user_ns(new->ns);
> 			spin_unlock_irq(&ucounts_lock);
>@@ -193,6 +191,9 @@ struct ucounts *alloc_ucounts(struct user_namespace *ns, kuid_t uid)
> 	}
> 	wrapped = !get_ucounts_or_wrap(ucounts);
> 	spin_unlock_irq(&ucounts_lock);
>+	if (new)
>+		kfree(new);
>+
> 	if (wrapped) {
> 		put_ucounts(ucounts);
> 		return NULL;
>-- 
>2.45.2