Re: [PATCH net-next 2/3] connector/cn_proc: Kunit tests for threads hash table

2024-10-12 Thread Anjali Kulkarni 



From: Simon Horman 
Sent: Saturday, October 12, 2024 2:45 AM
To: Anjali Kulkarni
Cc: da...@davemloft.net; Liam Howlett; eduma...@google.com; k...@kernel.org; 
pab...@redhat.com; mi...@redhat.com; pet...@infradead.org; 
juri.le...@redhat.com; vincent.guit...@linaro.org; dietmar.eggem...@arm.com; 
rost...@goodmis.org; bseg...@google.com; mgor...@suse.de; vschn...@redhat.com; 
j...@resnulli.us; linux-kernel@vger.kernel.org; net...@vger.kernel.org; 
a...@linux-foundation.org; sh...@kernel.org; linux-kselft...@vger.kernel.org; 
Pei Li
Subject: Re: [PATCH net-next 2/3] connector/cn_proc: Kunit tests for threads 
hash table

On Fri, Oct 11, 2024 at 05:45:31PM -0700, Anjali Kulkarni wrote:
> Kunit tests to test hash table add, delete, duplicate add and delete.
> Add following configs and compile kernel code:
>
> CONFIG_CONNECTOR=y
> CONFIG_PROC_EVENTS=y
> CONFIG_NET=y
> CONFIG_KUNIT=m/y
> CONFIG_CN_HASH_KUNIT_TEST=m/y
>
> To run kunit tests:
> sudo modprobe cn_hash_test
>
> Output of kunit tests and hash table contents are displayed in
> /var/log/messages (at KERN_DEBUG level).
>
> Signed-off-by: Anjali Kulkarni 

...

> index ..2687492864ed
> --- /dev/null
> +++ b/lib/cn_hash_test.c
> @@ -0,0 +1,167 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test for the connector threads hashtable code.
> + *
> + * Copyright (c) 2024 Oracle and/or its affiliates.
> + * Author: Anjali Kulkarni 
> + */
> +#include 
> +
> +#include "cn_hash_test.h"
> +
> +#define ARR_SIZE 4
> +#define HASH_TABLE_LEN   1024
> +
> +struct add_data {
> + pid_t pid;
> + int exit_val;
> + int key;
> +};
> +
> +struct add_data adata[ARR_SIZE];
> +int key_display[HASH_TABLE_LEN];

Hi Anjali,

adata and key_display seem to only be used within this file.
Probably they should be static.

Anjali> Thanks! Yes, will do both changes you have suggested and send in new 
revision.

Anjali

...

Re: [PATCH net-next 2/3] connector/cn_proc: Kunit tests for threads hash table

2024-10-12 Thread Simon Horman 
On Fri, Oct 11, 2024 at 05:45:31PM -0700, Anjali Kulkarni wrote:
> Kunit tests to test hash table add, delete, duplicate add and delete.
> Add following configs and compile kernel code:
> 
> CONFIG_CONNECTOR=y
> CONFIG_PROC_EVENTS=y
> CONFIG_NET=y
> CONFIG_KUNIT=m/y
> CONFIG_CN_HASH_KUNIT_TEST=m/y
> 
> To run kunit tests:
> sudo modprobe cn_hash_test
> 
> Output of kunit tests and hash table contents are displayed in
> /var/log/messages (at KERN_DEBUG level).
> 
> Signed-off-by: Anjali Kulkarni 

...

> index ..2687492864ed
> --- /dev/null
> +++ b/lib/cn_hash_test.c
> @@ -0,0 +1,167 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test for the connector threads hashtable code.
> + *
> + * Copyright (c) 2024 Oracle and/or its affiliates.
> + * Author: Anjali Kulkarni 
> + */
> +#include 
> +
> +#include "cn_hash_test.h"
> +
> +#define ARR_SIZE 4
> +#define HASH_TABLE_LEN   1024
> +
> +struct add_data {
> + pid_t pid;
> + int exit_val;
> + int key;
> +};
> +
> +struct add_data adata[ARR_SIZE];
> +int key_display[HASH_TABLE_LEN];

Hi Anjali,

adata and key_display seem to only be used within this file.
Probably they should be static.

...

[PATCH net-next 2/3] connector/cn_proc: Kunit tests for threads hash table

2024-10-11 Thread Anjali Kulkarni 
Kunit tests to test hash table add, delete, duplicate add and delete.
Add following configs and compile kernel code:

CONFIG_CONNECTOR=y
CONFIG_PROC_EVENTS=y
CONFIG_NET=y
CONFIG_KUNIT=m/y
CONFIG_CN_HASH_KUNIT_TEST=m/y

To run kunit tests:
sudo modprobe cn_hash_test

Output of kunit tests and hash table contents are displayed in
/var/log/messages (at KERN_DEBUG level).

Signed-off-by: Anjali Kulkarni 
---
 drivers/connector/cn_hash.c   |  49 +-
 drivers/connector/connector.c |  15 ++-
 include/linux/connector.h |   8 +-
 lib/Kconfig.debug |  17 
 lib/Makefile  |   1 +
 lib/cn_hash_test.c| 167 ++
 lib/cn_hash_test.h|  12 +++
 7 files changed, 264 insertions(+), 5 deletions(-)
 create mode 100644 lib/cn_hash_test.c
 create mode 100644 lib/cn_hash_test.h

diff --git a/drivers/connector/cn_hash.c b/drivers/connector/cn_hash.c
index a0211cd99132..8f0eb6acb158 100644
--- a/drivers/connector/cn_hash.c
+++ b/drivers/connector/cn_hash.c
@@ -166,7 +166,7 @@ __u32 cn_hash_del_get_exval(struct cn_hash_dev *hdev, pid_t 
pid)
return 0;
 }
 
-__u32 cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t pid)
+int cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t pid)
 {
struct uexit_pid_hnode *hnode;
__u32 excde;
@@ -189,7 +189,52 @@ __u32 cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t 
pid)
return -EINVAL;
 }
 
+int cn_hash_display_hlist(struct cn_hash_dev *hdev, pid_t pid, int max_len,
+   int *hkey, int *key_display)
+{
+   struct uexit_pid_hnode *hnode;
+   int key, count = 0;
+
+   mutex_lock(&hdev->uexit_hash_lock);
+   key = hash_min(pid, HASH_BITS(hdev->uexit_pid_htable));
+   pr_debug("Bucket: %d\n", key);
+
+   hlist_for_each_entry(hnode,
+   &hdev->uexit_pid_htable[key],
+   uexit_pid_hlist) {
+   if (key_display[key] != 1) {
+   if (hnode->uexit_pid_hlist.next == NULL)
+   pr_debug("pid %d ", hnode->pid);
+   else
+   pr_debug("pid %d --> ", hnode->pid);
+   }
+   count++;
+   }
+
+   mutex_unlock(&hdev->uexit_hash_lock);
+
+   if ((key_display[key] != 1) && !count)
+   pr_debug("(empty)\n");
+
+   pr_debug("\n");
+
+   *hkey = key;
+
+   if (count > max_len) {
+   pr_err("%d entries in hlist for key %d, expected %d\n",
+   count, key, max_len);
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
 bool cn_hash_table_empty(struct cn_hash_dev *hdev)
 {
-   return hash_empty(hdev->uexit_pid_htable);
+   bool is_empty;
+
+   is_empty = hash_empty(hdev->uexit_pid_htable);
+   pr_debug("Hash table is %s\n", (is_empty ? "empty" : "not empty"));
+
+   return is_empty;
 }
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index 506e3cbedf85..28e60c8b0fdf 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -308,7 +308,7 @@ int cn_add_elem(__u32 uexit_code, pid_t pid)
 }
 EXPORT_SYMBOL_GPL(cn_add_elem);
 
-__u32 cn_get_exval(pid_t pid)
+int cn_get_exval(pid_t pid)
 {
struct cn_dev *dev = &cdev;
__u32 exval;
@@ -321,6 +321,19 @@ __u32 cn_get_exval(pid_t pid)
 }
 EXPORT_SYMBOL_GPL(cn_get_exval);
 
+int cn_display_hlist(pid_t pid, int max_len, int *hkey,
+   int *key_display)
+{
+   struct cn_dev *dev = &cdev;
+
+   if (!cn_already_initialized)
+   return 0;
+
+   return cn_hash_display_hlist(dev->hdev, pid, max_len,
+   hkey, key_display);
+}
+EXPORT_SYMBOL_GPL(cn_display_hlist);
+
 bool cn_table_empty(void)
 {
struct cn_dev *dev = &cdev;
diff --git a/include/linux/connector.h b/include/linux/connector.h
index 094e1730a4f6..af801c5005e8 100644
--- a/include/linux/connector.h
+++ b/include/linux/connector.h
@@ -165,7 +165,7 @@ __u32 cn_hash_del_get_exval(struct cn_hash_dev *hdev, pid_t 
pid);
 int cn_add_elem(__u32 uexit_code, pid_t pid);
 int cn_del_elem(pid_t pid);
 __u32 cn_del_get_exval(pid_t pid);
-__u32 cn_get_exval(pid_t pid);
+int cn_get_exval(pid_t pid);
 
 struct cn_hash_dev *cn_hash_alloc_dev(const char *name);
 void cn_hash_free_dev(struct cn_hash_dev *hdev);
@@ -175,9 +175,13 @@ void cn_hash_free_elem(struct uexit_pid_hnode *elem);
 int cn_hash_add_elem(struct cn_hash_dev *hdev, __u32 uexit_code, pid_t pid);
 int cn_hash_del_elem(struct cn_hash_dev *hdev, pid_t pid);
 __u32 cn_hash_del_get_exval(struct cn_hash_dev *hdev, pid_t pid);
-__u32 cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t pid);
+int cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t pid);
 
 bool cn_table_empty(void);
 bool cn_hash_table_empty(struct cn_hash_dev *hdev);
 
+int cn_display_hlist(pid_t pid, int max_len, int *hkey, int *ke