| Message ID | 20241017181436.2047508-1-anjali.k.kulkarni@oracle.com |
|---|---|
| Headers | show |
| Series | Threads support in proc connector | expand |
On 10/17, 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 > CONFIG_CN_HASH_KUNIT_TEST=m > > 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 <anjali.k.kulkarni@oracle.com> > --- > drivers/connector/cn_hash.c | 40 ++++++++ > drivers/connector/connector.c | 12 +++ > include/linux/connector.h | 4 + > lib/Kconfig.debug | 17 ++++ > lib/Makefile | 1 + > lib/cn_hash_test.c | 167 ++++++++++++++++++++++++++++++++++ > lib/cn_hash_test.h | 10 ++ > 7 files changed, 251 insertions(+) > 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 a079e9bcea6d..40099b5908ac 100644 > --- a/drivers/connector/cn_hash.c > +++ b/drivers/connector/cn_hash.c > @@ -170,6 +170,46 @@ int 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) > { > bool is_empty; > diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c > index c1c0dcec53c0..2be2fe1adc12 100644 > --- a/drivers/connector/connector.c > +++ b/drivers/connector/connector.c > @@ -304,6 +304,18 @@ int 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 5384e4bb98e8..a75c3fcf182a 100644 > --- a/include/linux/connector.h > +++ b/include/linux/connector.h > @@ -168,4 +168,8 @@ int cn_get_exval(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 *key_display); > +int cn_hash_display_hlist(struct cn_hash_dev *hdev, pid_t pid, int max_len, > + int *hkey, int *key_display); > + > #endif /* __CONNECTOR_H */ > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index 7315f643817a..290cf0a6befa 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST > > If unsure, say N. > > +config CN_HASH_KUNIT_TEST > + tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS > + depends on KUNIT > + default KUNIT_ALL_TESTS > + help > + This builds the hashtable KUnit test suite. > + It tests the basic functionality of the API defined in > + drivers/connector/cn_hash.c. > + CONFIG_CONNECTOR=y, CONFIG_PROC_EVENTS=y and CONFIG_NET=y needs > + to be enabled along with CONFIG_CN_HASH_KUNIT_TEST=m and > + CONFIG_KUNIT=m in .config file to compile and then test as a kernel > + module with "modprobe cn_hash_test". > + For more information on KUnit and unit tests in general please > + refer to the KUnit documentation in Documentation/dev-tools/kunit/. > + > + If unsure, say N. > + Looks like this needs to depend on CONFIG_CONNECTOR? Otherwise, the existing kunit tester complains about the missing symbols (see below). Please also hold off reposting for a couple of days to give people some time to review. ERROR:root:ld: vmlinux.o: in function `cn_hash_test_dup_del': cn_hash_test.c:(.text+0x3e9dc3): undefined reference to `cn_del_get_exval' ld: cn_hash_test.c:(.text+0x3e9dee): undefined reference to `cn_del_get_exval' ld: cn_hash_test.c:(.text+0x3e9e22): undefined reference to `cn_table_empty' ld: vmlinux.o: in function `cn_display_htable': cn_hash_test.c:(.text+0x3e9f67): undefined reference to `cn_display_hlist' ld: vmlinux.o: in function `cn_hash_test_del_get_exval': cn_hash_test.c:(.text+0x3ea037): undefined reference to `cn_del_get_exval' ld: cn_hash_test.c:(.text+0x3ea088): undefined reference to `cn_table_empty' ld: vmlinux.o: in function `cn_hash_test_dup_add': cn_hash_test.c:(.text+0x3ea176): undefined reference to `cn_add_elem' ld: cn_hash_test.c:(.text+0x3ea19e): undefined reference to `cn_get_exval' ld: cn_hash_test.c:(.text+0x3ea1dc): undefined reference to `cn_add_elem' ld: cn_hash_test.c:(.text+0x3ea205): undefined reference to `cn_get_exval' ld: vmlinux.o: in function `cn_hash_test_del': cn_hash_test.c:(.text+0x3ea387): undefined reference to `cn_del_get_exval' ld: cn_hash_test.c:(.text+0x3ea3ab): undefined reference to `cn_get_exval' ld: cn_hash_test.c:(.text+0x3ea3fd): undefined reference to `cn_table_empty' ld: vmlinux.o: in function `cn_hash_test_add': cn_hash_test.c:(.text+0x3ea571): undefined reference to `cn_add_elem' ld: cn_hash_test.c:(.text+0x3ea591): undefined reference to `cn_get_exval' make[3]: *** [../scripts/Makefile.vmlinux:34: vmlinux] Error 1 make[2]: *** [/home/kunit/testing/Makefile:1166: vmlinux] Error 2 make[1]: *** [/home/kunit/testing/Makefile:224: __sub-make] Error 2 make: *** [Makefile:224: __sub-make] Error 2 --- pw-bot: cr
> On Oct 17, 2024, at 5:13 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote: > > On 10/17, 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 >> CONFIG_CN_HASH_KUNIT_TEST=m >> >> 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 <anjali.k.kulkarni@oracle.com> >> --- >> drivers/connector/cn_hash.c | 40 ++++++++ >> drivers/connector/connector.c | 12 +++ >> include/linux/connector.h | 4 + >> lib/Kconfig.debug | 17 ++++ >> lib/Makefile | 1 + >> lib/cn_hash_test.c | 167 ++++++++++++++++++++++++++++++++++ >> lib/cn_hash_test.h | 10 ++ >> 7 files changed, 251 insertions(+) >> 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 a079e9bcea6d..40099b5908ac 100644 >> --- a/drivers/connector/cn_hash.c >> +++ b/drivers/connector/cn_hash.c >> @@ -170,6 +170,46 @@ int 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) >> { >> bool is_empty; >> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c >> index c1c0dcec53c0..2be2fe1adc12 100644 >> --- a/drivers/connector/connector.c >> +++ b/drivers/connector/connector.c >> @@ -304,6 +304,18 @@ int 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 5384e4bb98e8..a75c3fcf182a 100644 >> --- a/include/linux/connector.h >> +++ b/include/linux/connector.h >> @@ -168,4 +168,8 @@ int cn_get_exval(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 *key_display); >> +int cn_hash_display_hlist(struct cn_hash_dev *hdev, pid_t pid, int max_len, >> + int *hkey, int *key_display); >> + >> #endif /* __CONNECTOR_H */ >> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug >> index 7315f643817a..290cf0a6befa 100644 >> --- a/lib/Kconfig.debug >> +++ b/lib/Kconfig.debug >> @@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST >> >> If unsure, say N. >> >> +config CN_HASH_KUNIT_TEST >> + tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS >> + depends on KUNIT >> + default KUNIT_ALL_TESTS >> + help >> + This builds the hashtable KUnit test suite. >> + It tests the basic functionality of the API defined in >> + drivers/connector/cn_hash.c. >> + CONFIG_CONNECTOR=y, CONFIG_PROC_EVENTS=y and CONFIG_NET=y needs >> + to be enabled along with CONFIG_CN_HASH_KUNIT_TEST=m and >> + CONFIG_KUNIT=m in .config file to compile and then test as a kernel >> + module with "modprobe cn_hash_test". >> + For more information on KUnit and unit tests in general please >> + refer to the KUnit documentation in Documentation/dev-tools/kunit/. >> + >> + If unsure, say N. >> + > > Looks like this needs to depend on CONFIG_CONNECTOR? Otherwise, the > existing kunit tester complains about the missing symbols (see below). > Please also hold off reposting for a couple of days to give people some > time to review. > > ERROR:root:ld: vmlinux.o: in function `cn_hash_test_dup_del': > cn_hash_test.c:(.text+0x3e9dc3): undefined reference to `cn_del_get_exval' > ld: cn_hash_test.c:(.text+0x3e9dee): undefined reference to `cn_del_get_exval' > ld: cn_hash_test.c:(.text+0x3e9e22): undefined reference to `cn_table_empty' > ld: vmlinux.o: in function `cn_display_htable': > cn_hash_test.c:(.text+0x3e9f67): undefined reference to `cn_display_hlist' > ld: vmlinux.o: in function `cn_hash_test_del_get_exval': > cn_hash_test.c:(.text+0x3ea037): undefined reference to `cn_del_get_exval' > ld: cn_hash_test.c:(.text+0x3ea088): undefined reference to `cn_table_empty' > ld: vmlinux.o: in function `cn_hash_test_dup_add': > cn_hash_test.c:(.text+0x3ea176): undefined reference to `cn_add_elem' > ld: cn_hash_test.c:(.text+0x3ea19e): undefined reference to `cn_get_exval' > ld: cn_hash_test.c:(.text+0x3ea1dc): undefined reference to `cn_add_elem' > ld: cn_hash_test.c:(.text+0x3ea205): undefined reference to `cn_get_exval' > ld: vmlinux.o: in function `cn_hash_test_del': > cn_hash_test.c:(.text+0x3ea387): undefined reference to `cn_del_get_exval' > ld: cn_hash_test.c:(.text+0x3ea3ab): undefined reference to `cn_get_exval' > ld: cn_hash_test.c:(.text+0x3ea3fd): undefined reference to `cn_table_empty' > ld: vmlinux.o: in function `cn_hash_test_add': > cn_hash_test.c:(.text+0x3ea571): undefined reference to `cn_add_elem' > ld: cn_hash_test.c:(.text+0x3ea591): undefined reference to `cn_get_exval' > make[3]: *** [../scripts/Makefile.vmlinux:34: vmlinux] Error 1 > make[2]: *** [/home/kunit/testing/Makefile:1166: vmlinux] Error 2 > make[1]: *** [/home/kunit/testing/Makefile:224: __sub-make] Error 2 > make: *** [Makefile:224: __sub-make] Error 2 Yes, I have added in the comments for CN_HASH_KUNIT_TEST, it depends on: CONFIG_CONNECTOR, CONFIG_PROC_EVENTS, CONFIG_NET. I didn’t realize I could add these to the “depends” field. So something like this: (let me know if you see any issues) tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS depends on KUNIT + depends on CONNECTOR && PROC_EVENTS + depends on NET default KUNIT_ALL_TESTS These are the configs I add to my .config file and compile it as a module and then do modprobe to test. Are you running the kunit tester with kunit.py? Thanks Anjali > > --- > pw-bot: cr
On 10/18, Anjali Kulkarni wrote: > > > > On Oct 17, 2024, at 5:13 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote: > > > > On 10/17, 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 > >> CONFIG_CN_HASH_KUNIT_TEST=m > >> > >> 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 <anjali.k.kulkarni@oracle.com> > >> --- > >> drivers/connector/cn_hash.c | 40 ++++++++ > >> drivers/connector/connector.c | 12 +++ > >> include/linux/connector.h | 4 + > >> lib/Kconfig.debug | 17 ++++ > >> lib/Makefile | 1 + > >> lib/cn_hash_test.c | 167 ++++++++++++++++++++++++++++++++++ > >> lib/cn_hash_test.h | 10 ++ > >> 7 files changed, 251 insertions(+) > >> 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 a079e9bcea6d..40099b5908ac 100644 > >> --- a/drivers/connector/cn_hash.c > >> +++ b/drivers/connector/cn_hash.c > >> @@ -170,6 +170,46 @@ int 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) > >> { > >> bool is_empty; > >> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c > >> index c1c0dcec53c0..2be2fe1adc12 100644 > >> --- a/drivers/connector/connector.c > >> +++ b/drivers/connector/connector.c > >> @@ -304,6 +304,18 @@ int 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 5384e4bb98e8..a75c3fcf182a 100644 > >> --- a/include/linux/connector.h > >> +++ b/include/linux/connector.h > >> @@ -168,4 +168,8 @@ int cn_get_exval(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 *key_display); > >> +int cn_hash_display_hlist(struct cn_hash_dev *hdev, pid_t pid, int max_len, > >> + int *hkey, int *key_display); > >> + > >> #endif /* __CONNECTOR_H */ > >> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > >> index 7315f643817a..290cf0a6befa 100644 > >> --- a/lib/Kconfig.debug > >> +++ b/lib/Kconfig.debug > >> @@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST > >> > >> If unsure, say N. > >> > >> +config CN_HASH_KUNIT_TEST > >> + tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS > >> + depends on KUNIT > >> + default KUNIT_ALL_TESTS > >> + help > >> + This builds the hashtable KUnit test suite. > >> + It tests the basic functionality of the API defined in > >> + drivers/connector/cn_hash.c. > >> + CONFIG_CONNECTOR=y, CONFIG_PROC_EVENTS=y and CONFIG_NET=y needs > >> + to be enabled along with CONFIG_CN_HASH_KUNIT_TEST=m and > >> + CONFIG_KUNIT=m in .config file to compile and then test as a kernel > >> + module with "modprobe cn_hash_test". > >> + For more information on KUnit and unit tests in general please > >> + refer to the KUnit documentation in Documentation/dev-tools/kunit/. > >> + > >> + If unsure, say N. > >> + > > > > Looks like this needs to depend on CONFIG_CONNECTOR? Otherwise, the > > existing kunit tester complains about the missing symbols (see below). > > Please also hold off reposting for a couple of days to give people some > > time to review. > > > > ERROR:root:ld: vmlinux.o: in function `cn_hash_test_dup_del': > > cn_hash_test.c:(.text+0x3e9dc3): undefined reference to `cn_del_get_exval' > > ld: cn_hash_test.c:(.text+0x3e9dee): undefined reference to `cn_del_get_exval' > > ld: cn_hash_test.c:(.text+0x3e9e22): undefined reference to `cn_table_empty' > > ld: vmlinux.o: in function `cn_display_htable': > > cn_hash_test.c:(.text+0x3e9f67): undefined reference to `cn_display_hlist' > > ld: vmlinux.o: in function `cn_hash_test_del_get_exval': > > cn_hash_test.c:(.text+0x3ea037): undefined reference to `cn_del_get_exval' > > ld: cn_hash_test.c:(.text+0x3ea088): undefined reference to `cn_table_empty' > > ld: vmlinux.o: in function `cn_hash_test_dup_add': > > cn_hash_test.c:(.text+0x3ea176): undefined reference to `cn_add_elem' > > ld: cn_hash_test.c:(.text+0x3ea19e): undefined reference to `cn_get_exval' > > ld: cn_hash_test.c:(.text+0x3ea1dc): undefined reference to `cn_add_elem' > > ld: cn_hash_test.c:(.text+0x3ea205): undefined reference to `cn_get_exval' > > ld: vmlinux.o: in function `cn_hash_test_del': > > cn_hash_test.c:(.text+0x3ea387): undefined reference to `cn_del_get_exval' > > ld: cn_hash_test.c:(.text+0x3ea3ab): undefined reference to `cn_get_exval' > > ld: cn_hash_test.c:(.text+0x3ea3fd): undefined reference to `cn_table_empty' > > ld: vmlinux.o: in function `cn_hash_test_add': > > cn_hash_test.c:(.text+0x3ea571): undefined reference to `cn_add_elem' > > ld: cn_hash_test.c:(.text+0x3ea591): undefined reference to `cn_get_exval' > > make[3]: *** [../scripts/Makefile.vmlinux:34: vmlinux] Error 1 > > make[2]: *** [/home/kunit/testing/Makefile:1166: vmlinux] Error 2 > > make[1]: *** [/home/kunit/testing/Makefile:224: __sub-make] Error 2 > > make: *** [Makefile:224: __sub-make] Error 2 > > Yes, I have added in the comments for CN_HASH_KUNIT_TEST, it depends on: > CONFIG_CONNECTOR, CONFIG_PROC_EVENTS, CONFIG_NET. I didn’t realize > I could add these to the “depends” field. > So something like this: (let me know if you see any issues) > > tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS > depends on KUNIT > + depends on CONNECTOR && PROC_EVENTS > + depends on NET > default KUNIT_ALL_TESTS > > These are the configs I add to my .config file and compile it as a module and then > do modprobe to test. [..] > Are you running the kunit tester with kunit.py? Yes, make sure all required options are picked up by "./tools/testing/kunit/kunit.py run" instead of manually adding options and doing modprobe.
> On Oct 17, 2024, at 5:55 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote: > > On 10/18, Anjali Kulkarni wrote: >> >> >>> On Oct 17, 2024, at 5:13 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote: >>> >>> On 10/17, 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 >>>> CONFIG_CN_HASH_KUNIT_TEST=m >>>> >>>> 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 <anjali.k.kulkarni@oracle.com> >>>> --- >>>> drivers/connector/cn_hash.c | 40 ++++++++ >>>> drivers/connector/connector.c | 12 +++ >>>> include/linux/connector.h | 4 + >>>> lib/Kconfig.debug | 17 ++++ >>>> lib/Makefile | 1 + >>>> lib/cn_hash_test.c | 167 ++++++++++++++++++++++++++++++++++ >>>> lib/cn_hash_test.h | 10 ++ >>>> 7 files changed, 251 insertions(+) >>>> 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 a079e9bcea6d..40099b5908ac 100644 >>>> --- a/drivers/connector/cn_hash.c >>>> +++ b/drivers/connector/cn_hash.c >>>> @@ -170,6 +170,46 @@ int 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) >>>> { >>>> bool is_empty; >>>> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c >>>> index c1c0dcec53c0..2be2fe1adc12 100644 >>>> --- a/drivers/connector/connector.c >>>> +++ b/drivers/connector/connector.c >>>> @@ -304,6 +304,18 @@ int 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 5384e4bb98e8..a75c3fcf182a 100644 >>>> --- a/include/linux/connector.h >>>> +++ b/include/linux/connector.h >>>> @@ -168,4 +168,8 @@ int cn_get_exval(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 *key_display); >>>> +int cn_hash_display_hlist(struct cn_hash_dev *hdev, pid_t pid, int max_len, >>>> + int *hkey, int *key_display); >>>> + >>>> #endif /* __CONNECTOR_H */ >>>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug >>>> index 7315f643817a..290cf0a6befa 100644 >>>> --- a/lib/Kconfig.debug >>>> +++ b/lib/Kconfig.debug >>>> @@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST >>>> >>>> If unsure, say N. >>>> >>>> +config CN_HASH_KUNIT_TEST >>>> + tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS >>>> + depends on KUNIT >>>> + default KUNIT_ALL_TESTS >>>> + help >>>> + This builds the hashtable KUnit test suite. >>>> + It tests the basic functionality of the API defined in >>>> + drivers/connector/cn_hash.c. >>>> + CONFIG_CONNECTOR=y, CONFIG_PROC_EVENTS=y and CONFIG_NET=y needs >>>> + to be enabled along with CONFIG_CN_HASH_KUNIT_TEST=m and >>>> + CONFIG_KUNIT=m in .config file to compile and then test as a kernel >>>> + module with "modprobe cn_hash_test". >>>> + For more information on KUnit and unit tests in general please >>>> + refer to the KUnit documentation in Documentation/dev-tools/kunit/. >>>> + >>>> + If unsure, say N. >>>> + >>> >>> Looks like this needs to depend on CONFIG_CONNECTOR? Otherwise, the >>> existing kunit tester complains about the missing symbols (see below). >>> Please also hold off reposting for a couple of days to give people some >>> time to review. >>> >>> ERROR:root:ld: vmlinux.o: in function `cn_hash_test_dup_del': >>> cn_hash_test.c:(.text+0x3e9dc3): undefined reference to `cn_del_get_exval' >>> ld: cn_hash_test.c:(.text+0x3e9dee): undefined reference to `cn_del_get_exval' >>> ld: cn_hash_test.c:(.text+0x3e9e22): undefined reference to `cn_table_empty' >>> ld: vmlinux.o: in function `cn_display_htable': >>> cn_hash_test.c:(.text+0x3e9f67): undefined reference to `cn_display_hlist' >>> ld: vmlinux.o: in function `cn_hash_test_del_get_exval': >>> cn_hash_test.c:(.text+0x3ea037): undefined reference to `cn_del_get_exval' >>> ld: cn_hash_test.c:(.text+0x3ea088): undefined reference to `cn_table_empty' >>> ld: vmlinux.o: in function `cn_hash_test_dup_add': >>> cn_hash_test.c:(.text+0x3ea176): undefined reference to `cn_add_elem' >>> ld: cn_hash_test.c:(.text+0x3ea19e): undefined reference to `cn_get_exval' >>> ld: cn_hash_test.c:(.text+0x3ea1dc): undefined reference to `cn_add_elem' >>> ld: cn_hash_test.c:(.text+0x3ea205): undefined reference to `cn_get_exval' >>> ld: vmlinux.o: in function `cn_hash_test_del': >>> cn_hash_test.c:(.text+0x3ea387): undefined reference to `cn_del_get_exval' >>> ld: cn_hash_test.c:(.text+0x3ea3ab): undefined reference to `cn_get_exval' >>> ld: cn_hash_test.c:(.text+0x3ea3fd): undefined reference to `cn_table_empty' >>> ld: vmlinux.o: in function `cn_hash_test_add': >>> cn_hash_test.c:(.text+0x3ea571): undefined reference to `cn_add_elem' >>> ld: cn_hash_test.c:(.text+0x3ea591): undefined reference to `cn_get_exval' >>> make[3]: *** [../scripts/Makefile.vmlinux:34: vmlinux] Error 1 >>> make[2]: *** [/home/kunit/testing/Makefile:1166: vmlinux] Error 2 >>> make[1]: *** [/home/kunit/testing/Makefile:224: __sub-make] Error 2 >>> make: *** [Makefile:224: __sub-make] Error 2 >> >> Yes, I have added in the comments for CN_HASH_KUNIT_TEST, it depends on: >> CONFIG_CONNECTOR, CONFIG_PROC_EVENTS, CONFIG_NET. I didn’t realize >> I could add these to the “depends” field. >> So something like this: (let me know if you see any issues) >> >> tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS >> depends on KUNIT >> + depends on CONNECTOR && PROC_EVENTS >> + depends on NET >> default KUNIT_ALL_TESTS >> >> These are the configs I add to my .config file and compile it as a module and then >> do modprobe to test. > > [..] > >> Are you running the kunit tester with kunit.py? > > Yes, make sure all required options are picked up by > "./tools/testing/kunit/kunit.py run" instead of manually adding options > and doing modprobe. I’m unable to run kunit.py, it runs into various issues like UML, permissions, other errors. I talked to the kunit guys about this and we have been debugging it for a while but unable to fix the environment issue. But the tests work fine. What kind of VM is this being run on? Like ubuntu etc.? I will try on a different OS and check if kunit.py works. Anjali
On Thu, Oct 17, 2024 at 11:14:33AM -0700, Anjali Kulkarni wrote: > Recently we committed a fix to allow processes to receive notifications for > non-zero exits via the process connector module. Commit is a4c9a56e6a2c. > > However, for threads, when it does a pthread_exit(&exit_status) call, the > kernel is not aware of the exit status with which pthread_exit is called. > It is sent by child thread to the parent process, if it is waiting in > pthread_join(). Hence, for a thread exiting abnormally, kernel cannot > send notifications to any listening processes. > > The exception to this is if the thread is sent a signal which it has not > handled, and dies along with it's process as a result; for eg. SIGSEGV or > SIGKILL. In this case, kernel is aware of the non-zero exit and sends a > notification for it. > > For our use case, we cannot have parent wait in pthread_join, one of the > main reasons for this being that we do not want to track normal > pthread_exit(), which could be a very large number. We only want to be > notified of any abnormal exits. Hence, threads are created with > pthread_attr_t set to PTHREAD_CREATE_DETACHED. > > To fix this problem, we add a new type PROC_CN_MCAST_NOTIFY to proc connector > API, which allows a thread to send it's exit status to kernel either when > it needs to call pthread_exit() with non-zero value to indicate some > error or from signal handler before pthread_exit(). > > We also need to filter packets with non-zero exit notifications futher > based on instances, which can be identified by task names. Hence, added a > comm field to the packet's struct proc_event, in which task->comm is > stored. As it seems that there will be another revision anyway, please run this patch-set through checkpatch with the following arguments. ./scripts/checkpatch.pl --strict --max-line-length=80 And please fix warnings about alignment and line length. But please do so in such a way that doesn't reduce readability, e.g. don't split strings over multiple lines.
On 10/18, Anjali Kulkarni wrote: > > > > On Oct 17, 2024, at 5:55 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote: > > > > On 10/18, Anjali Kulkarni wrote: > >> > >> > >>> On Oct 17, 2024, at 5:13 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote: > >>> > >>> On 10/17, 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 > >>>> CONFIG_CN_HASH_KUNIT_TEST=m > >>>> > >>>> 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 <anjali.k.kulkarni@oracle.com> > >>>> --- > >>>> drivers/connector/cn_hash.c | 40 ++++++++ > >>>> drivers/connector/connector.c | 12 +++ > >>>> include/linux/connector.h | 4 + > >>>> lib/Kconfig.debug | 17 ++++ > >>>> lib/Makefile | 1 + > >>>> lib/cn_hash_test.c | 167 ++++++++++++++++++++++++++++++++++ > >>>> lib/cn_hash_test.h | 10 ++ > >>>> 7 files changed, 251 insertions(+) > >>>> 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 a079e9bcea6d..40099b5908ac 100644 > >>>> --- a/drivers/connector/cn_hash.c > >>>> +++ b/drivers/connector/cn_hash.c > >>>> @@ -170,6 +170,46 @@ int 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) > >>>> { > >>>> bool is_empty; > >>>> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c > >>>> index c1c0dcec53c0..2be2fe1adc12 100644 > >>>> --- a/drivers/connector/connector.c > >>>> +++ b/drivers/connector/connector.c > >>>> @@ -304,6 +304,18 @@ int 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 5384e4bb98e8..a75c3fcf182a 100644 > >>>> --- a/include/linux/connector.h > >>>> +++ b/include/linux/connector.h > >>>> @@ -168,4 +168,8 @@ int cn_get_exval(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 *key_display); > >>>> +int cn_hash_display_hlist(struct cn_hash_dev *hdev, pid_t pid, int max_len, > >>>> + int *hkey, int *key_display); > >>>> + > >>>> #endif /* __CONNECTOR_H */ > >>>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > >>>> index 7315f643817a..290cf0a6befa 100644 > >>>> --- a/lib/Kconfig.debug > >>>> +++ b/lib/Kconfig.debug > >>>> @@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST > >>>> > >>>> If unsure, say N. > >>>> > >>>> +config CN_HASH_KUNIT_TEST > >>>> + tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS > >>>> + depends on KUNIT > >>>> + default KUNIT_ALL_TESTS > >>>> + help > >>>> + This builds the hashtable KUnit test suite. > >>>> + It tests the basic functionality of the API defined in > >>>> + drivers/connector/cn_hash.c. > >>>> + CONFIG_CONNECTOR=y, CONFIG_PROC_EVENTS=y and CONFIG_NET=y needs > >>>> + to be enabled along with CONFIG_CN_HASH_KUNIT_TEST=m and > >>>> + CONFIG_KUNIT=m in .config file to compile and then test as a kernel > >>>> + module with "modprobe cn_hash_test". > >>>> + For more information on KUnit and unit tests in general please > >>>> + refer to the KUnit documentation in Documentation/dev-tools/kunit/. > >>>> + > >>>> + If unsure, say N. > >>>> + > >>> > >>> Looks like this needs to depend on CONFIG_CONNECTOR? Otherwise, the > >>> existing kunit tester complains about the missing symbols (see below). > >>> Please also hold off reposting for a couple of days to give people some > >>> time to review. > >>> > >>> ERROR:root:ld: vmlinux.o: in function `cn_hash_test_dup_del': > >>> cn_hash_test.c:(.text+0x3e9dc3): undefined reference to `cn_del_get_exval' > >>> ld: cn_hash_test.c:(.text+0x3e9dee): undefined reference to `cn_del_get_exval' > >>> ld: cn_hash_test.c:(.text+0x3e9e22): undefined reference to `cn_table_empty' > >>> ld: vmlinux.o: in function `cn_display_htable': > >>> cn_hash_test.c:(.text+0x3e9f67): undefined reference to `cn_display_hlist' > >>> ld: vmlinux.o: in function `cn_hash_test_del_get_exval': > >>> cn_hash_test.c:(.text+0x3ea037): undefined reference to `cn_del_get_exval' > >>> ld: cn_hash_test.c:(.text+0x3ea088): undefined reference to `cn_table_empty' > >>> ld: vmlinux.o: in function `cn_hash_test_dup_add': > >>> cn_hash_test.c:(.text+0x3ea176): undefined reference to `cn_add_elem' > >>> ld: cn_hash_test.c:(.text+0x3ea19e): undefined reference to `cn_get_exval' > >>> ld: cn_hash_test.c:(.text+0x3ea1dc): undefined reference to `cn_add_elem' > >>> ld: cn_hash_test.c:(.text+0x3ea205): undefined reference to `cn_get_exval' > >>> ld: vmlinux.o: in function `cn_hash_test_del': > >>> cn_hash_test.c:(.text+0x3ea387): undefined reference to `cn_del_get_exval' > >>> ld: cn_hash_test.c:(.text+0x3ea3ab): undefined reference to `cn_get_exval' > >>> ld: cn_hash_test.c:(.text+0x3ea3fd): undefined reference to `cn_table_empty' > >>> ld: vmlinux.o: in function `cn_hash_test_add': > >>> cn_hash_test.c:(.text+0x3ea571): undefined reference to `cn_add_elem' > >>> ld: cn_hash_test.c:(.text+0x3ea591): undefined reference to `cn_get_exval' > >>> make[3]: *** [../scripts/Makefile.vmlinux:34: vmlinux] Error 1 > >>> make[2]: *** [/home/kunit/testing/Makefile:1166: vmlinux] Error 2 > >>> make[1]: *** [/home/kunit/testing/Makefile:224: __sub-make] Error 2 > >>> make: *** [Makefile:224: __sub-make] Error 2 > >> > >> Yes, I have added in the comments for CN_HASH_KUNIT_TEST, it depends on: > >> CONFIG_CONNECTOR, CONFIG_PROC_EVENTS, CONFIG_NET. I didn’t realize > >> I could add these to the “depends” field. > >> So something like this: (let me know if you see any issues) > >> > >> tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS > >> depends on KUNIT > >> + depends on CONNECTOR && PROC_EVENTS > >> + depends on NET > >> default KUNIT_ALL_TESTS > >> > >> These are the configs I add to my .config file and compile it as a module and then > >> do modprobe to test. > > > > [..] > > > >> Are you running the kunit tester with kunit.py? > > > > Yes, make sure all required options are picked up by > > "./tools/testing/kunit/kunit.py run" instead of manually adding options > > and doing modprobe. > > I’m unable to run kunit.py, it runs into various issues like UML, permissions, other > errors. I talked to the kunit guys about this and we have been debugging it for a > while but unable to fix the environment issue. But the tests work fine. > > What kind of VM is this being run on? Like ubuntu etc.? I will try on a different > OS and check if kunit.py works. It's running on fedora.
> On Oct 18, 2024, at 2:49 AM, Simon Horman <horms@kernel.org> wrote: > > On Thu, Oct 17, 2024 at 11:14:33AM -0700, Anjali Kulkarni wrote: >> Recently we committed a fix to allow processes to receive notifications for >> non-zero exits via the process connector module. Commit is a4c9a56e6a2c. >> >> However, for threads, when it does a pthread_exit(&exit_status) call, the >> kernel is not aware of the exit status with which pthread_exit is called. >> It is sent by child thread to the parent process, if it is waiting in >> pthread_join(). Hence, for a thread exiting abnormally, kernel cannot >> send notifications to any listening processes. >> >> The exception to this is if the thread is sent a signal which it has not >> handled, and dies along with it's process as a result; for eg. SIGSEGV or >> SIGKILL. In this case, kernel is aware of the non-zero exit and sends a >> notification for it. >> >> For our use case, we cannot have parent wait in pthread_join, one of the >> main reasons for this being that we do not want to track normal >> pthread_exit(), which could be a very large number. We only want to be >> notified of any abnormal exits. Hence, threads are created with >> pthread_attr_t set to PTHREAD_CREATE_DETACHED. >> >> To fix this problem, we add a new type PROC_CN_MCAST_NOTIFY to proc connector >> API, which allows a thread to send it's exit status to kernel either when >> it needs to call pthread_exit() with non-zero value to indicate some >> error or from signal handler before pthread_exit(). >> >> We also need to filter packets with non-zero exit notifications futher >> based on instances, which can be identified by task names. Hence, added a >> comm field to the packet's struct proc_event, in which task->comm is >> stored. > > As it seems that there will be another revision anyway, > please run this patch-set through checkpatch with the following arguments. > > ./scripts/checkpatch.pl --strict --max-line-length=80 > > And please fix warnings about alignment and line length. > But please do so in such a way that doesn't reduce readability, > e.g. don't split strings over multiple lines. Ok thanks, will do. Anjali
> On Oct 17, 2024, at 5:55 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote: > > On 10/18, Anjali Kulkarni wrote: >> >> >>> On Oct 17, 2024, at 5:13 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote: >>> >>> On 10/17, 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 >>>> CONFIG_CN_HASH_KUNIT_TEST=m >>>> >>>> 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 <anjali.k.kulkarni@oracle.com> >>>> --- >>>> drivers/connector/cn_hash.c | 40 ++++++++ >>>> drivers/connector/connector.c | 12 +++ >>>> include/linux/connector.h | 4 + >>>> lib/Kconfig.debug | 17 ++++ >>>> lib/Makefile | 1 + >>>> lib/cn_hash_test.c | 167 ++++++++++++++++++++++++++++++++++ >>>> lib/cn_hash_test.h | 10 ++ >>>> 7 files changed, 251 insertions(+) >>>> 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 a079e9bcea6d..40099b5908ac 100644 >>>> --- a/drivers/connector/cn_hash.c >>>> +++ b/drivers/connector/cn_hash.c >>>> @@ -170,6 +170,46 @@ int 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) >>>> { >>>> bool is_empty; >>>> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c >>>> index c1c0dcec53c0..2be2fe1adc12 100644 >>>> --- a/drivers/connector/connector.c >>>> +++ b/drivers/connector/connector.c >>>> @@ -304,6 +304,18 @@ int 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 5384e4bb98e8..a75c3fcf182a 100644 >>>> --- a/include/linux/connector.h >>>> +++ b/include/linux/connector.h >>>> @@ -168,4 +168,8 @@ int cn_get_exval(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 *key_display); >>>> +int cn_hash_display_hlist(struct cn_hash_dev *hdev, pid_t pid, int max_len, >>>> + int *hkey, int *key_display); >>>> + >>>> #endif /* __CONNECTOR_H */ >>>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug >>>> index 7315f643817a..290cf0a6befa 100644 >>>> --- a/lib/Kconfig.debug >>>> +++ b/lib/Kconfig.debug >>>> @@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST >>>> >>>> If unsure, say N. >>>> >>>> +config CN_HASH_KUNIT_TEST >>>> + tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS >>>> + depends on KUNIT >>>> + default KUNIT_ALL_TESTS >>>> + help >>>> + This builds the hashtable KUnit test suite. >>>> + It tests the basic functionality of the API defined in >>>> + drivers/connector/cn_hash.c. >>>> + CONFIG_CONNECTOR=y, CONFIG_PROC_EVENTS=y and CONFIG_NET=y needs >>>> + to be enabled along with CONFIG_CN_HASH_KUNIT_TEST=m and >>>> + CONFIG_KUNIT=m in .config file to compile and then test as a kernel >>>> + module with "modprobe cn_hash_test". >>>> + For more information on KUnit and unit tests in general please >>>> + refer to the KUnit documentation in Documentation/dev-tools/kunit/. >>>> + >>>> + If unsure, say N. >>>> + >>> >>> Looks like this needs to depend on CONFIG_CONNECTOR? Otherwise, the >>> existing kunit tester complains about the missing symbols (see below). >>> Please also hold off reposting for a couple of days to give people some >>> time to review. >>> >>> ERROR:root:ld: vmlinux.o: in function `cn_hash_test_dup_del': >>> cn_hash_test.c:(.text+0x3e9dc3): undefined reference to `cn_del_get_exval' >>> ld: cn_hash_test.c:(.text+0x3e9dee): undefined reference to `cn_del_get_exval' >>> ld: cn_hash_test.c:(.text+0x3e9e22): undefined reference to `cn_table_empty' >>> ld: vmlinux.o: in function `cn_display_htable': >>> cn_hash_test.c:(.text+0x3e9f67): undefined reference to `cn_display_hlist' >>> ld: vmlinux.o: in function `cn_hash_test_del_get_exval': >>> cn_hash_test.c:(.text+0x3ea037): undefined reference to `cn_del_get_exval' >>> ld: cn_hash_test.c:(.text+0x3ea088): undefined reference to `cn_table_empty' >>> ld: vmlinux.o: in function `cn_hash_test_dup_add': >>> cn_hash_test.c:(.text+0x3ea176): undefined reference to `cn_add_elem' >>> ld: cn_hash_test.c:(.text+0x3ea19e): undefined reference to `cn_get_exval' >>> ld: cn_hash_test.c:(.text+0x3ea1dc): undefined reference to `cn_add_elem' >>> ld: cn_hash_test.c:(.text+0x3ea205): undefined reference to `cn_get_exval' >>> ld: vmlinux.o: in function `cn_hash_test_del': >>> cn_hash_test.c:(.text+0x3ea387): undefined reference to `cn_del_get_exval' >>> ld: cn_hash_test.c:(.text+0x3ea3ab): undefined reference to `cn_get_exval' >>> ld: cn_hash_test.c:(.text+0x3ea3fd): undefined reference to `cn_table_empty' >>> ld: vmlinux.o: in function `cn_hash_test_add': >>> cn_hash_test.c:(.text+0x3ea571): undefined reference to `cn_add_elem' >>> ld: cn_hash_test.c:(.text+0x3ea591): undefined reference to `cn_get_exval' >>> make[3]: *** [../scripts/Makefile.vmlinux:34: vmlinux] Error 1 >>> make[2]: *** [/home/kunit/testing/Makefile:1166: vmlinux] Error 2 >>> make[1]: *** [/home/kunit/testing/Makefile:224: __sub-make] Error 2 >>> make: *** [Makefile:224: __sub-make] Error 2 >> >> Yes, I have added in the comments for CN_HASH_KUNIT_TEST, it depends on: >> CONFIG_CONNECTOR, CONFIG_PROC_EVENTS, CONFIG_NET. I didn’t realize >> I could add these to the “depends” field. >> So something like this: (let me know if you see any issues) >> >> tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS >> depends on KUNIT >> + depends on CONNECTOR && PROC_EVENTS >> + depends on NET >> default KUNIT_ALL_TESTS >> >> These are the configs I add to my .config file and compile it as a module and then >> do modprobe to test. > > [..] > >> Are you running the kunit tester with kunit.py? > > Yes, make sure all required options are picked up by > "./tools/testing/kunit/kunit.py run" instead of manually adding options > and doing modprobe. The environment issues are resolved and I am able to run kunit.py, but my tests are not invoked without giving options via —kconfig-add. Other tests are also not invoked. Running with the manual options runs 413 tests, and with just kunit.py runs 389 tests. (I have added 6). Any idea how I can make it run my tests?
On 10/22, Anjali Kulkarni wrote: > > > > On Oct 17, 2024, at 5:55 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote: > > > > On 10/18, Anjali Kulkarni wrote: > >> > >> > >>> On Oct 17, 2024, at 5:13 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote: > >>> > >>> On 10/17, 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 > >>>> CONFIG_CN_HASH_KUNIT_TEST=m > >>>> > >>>> 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 <anjali.k.kulkarni@oracle.com> > >>>> --- > >>>> drivers/connector/cn_hash.c | 40 ++++++++ > >>>> drivers/connector/connector.c | 12 +++ > >>>> include/linux/connector.h | 4 + > >>>> lib/Kconfig.debug | 17 ++++ > >>>> lib/Makefile | 1 + > >>>> lib/cn_hash_test.c | 167 ++++++++++++++++++++++++++++++++++ > >>>> lib/cn_hash_test.h | 10 ++ > >>>> 7 files changed, 251 insertions(+) > >>>> 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 a079e9bcea6d..40099b5908ac 100644 > >>>> --- a/drivers/connector/cn_hash.c > >>>> +++ b/drivers/connector/cn_hash.c > >>>> @@ -170,6 +170,46 @@ int 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) > >>>> { > >>>> bool is_empty; > >>>> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c > >>>> index c1c0dcec53c0..2be2fe1adc12 100644 > >>>> --- a/drivers/connector/connector.c > >>>> +++ b/drivers/connector/connector.c > >>>> @@ -304,6 +304,18 @@ int 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 5384e4bb98e8..a75c3fcf182a 100644 > >>>> --- a/include/linux/connector.h > >>>> +++ b/include/linux/connector.h > >>>> @@ -168,4 +168,8 @@ int cn_get_exval(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 *key_display); > >>>> +int cn_hash_display_hlist(struct cn_hash_dev *hdev, pid_t pid, int max_len, > >>>> + int *hkey, int *key_display); > >>>> + > >>>> #endif /* __CONNECTOR_H */ > >>>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > >>>> index 7315f643817a..290cf0a6befa 100644 > >>>> --- a/lib/Kconfig.debug > >>>> +++ b/lib/Kconfig.debug > >>>> @@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST > >>>> > >>>> If unsure, say N. > >>>> > >>>> +config CN_HASH_KUNIT_TEST > >>>> + tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS > >>>> + depends on KUNIT > >>>> + default KUNIT_ALL_TESTS > >>>> + help > >>>> + This builds the hashtable KUnit test suite. > >>>> + It tests the basic functionality of the API defined in > >>>> + drivers/connector/cn_hash.c. > >>>> + CONFIG_CONNECTOR=y, CONFIG_PROC_EVENTS=y and CONFIG_NET=y needs > >>>> + to be enabled along with CONFIG_CN_HASH_KUNIT_TEST=m and > >>>> + CONFIG_KUNIT=m in .config file to compile and then test as a kernel > >>>> + module with "modprobe cn_hash_test". > >>>> + For more information on KUnit and unit tests in general please > >>>> + refer to the KUnit documentation in Documentation/dev-tools/kunit/. > >>>> + > >>>> + If unsure, say N. > >>>> + > >>> > >>> Looks like this needs to depend on CONFIG_CONNECTOR? Otherwise, the > >>> existing kunit tester complains about the missing symbols (see below). > >>> Please also hold off reposting for a couple of days to give people some > >>> time to review. > >>> > >>> ERROR:root:ld: vmlinux.o: in function `cn_hash_test_dup_del': > >>> cn_hash_test.c:(.text+0x3e9dc3): undefined reference to `cn_del_get_exval' > >>> ld: cn_hash_test.c:(.text+0x3e9dee): undefined reference to `cn_del_get_exval' > >>> ld: cn_hash_test.c:(.text+0x3e9e22): undefined reference to `cn_table_empty' > >>> ld: vmlinux.o: in function `cn_display_htable': > >>> cn_hash_test.c:(.text+0x3e9f67): undefined reference to `cn_display_hlist' > >>> ld: vmlinux.o: in function `cn_hash_test_del_get_exval': > >>> cn_hash_test.c:(.text+0x3ea037): undefined reference to `cn_del_get_exval' > >>> ld: cn_hash_test.c:(.text+0x3ea088): undefined reference to `cn_table_empty' > >>> ld: vmlinux.o: in function `cn_hash_test_dup_add': > >>> cn_hash_test.c:(.text+0x3ea176): undefined reference to `cn_add_elem' > >>> ld: cn_hash_test.c:(.text+0x3ea19e): undefined reference to `cn_get_exval' > >>> ld: cn_hash_test.c:(.text+0x3ea1dc): undefined reference to `cn_add_elem' > >>> ld: cn_hash_test.c:(.text+0x3ea205): undefined reference to `cn_get_exval' > >>> ld: vmlinux.o: in function `cn_hash_test_del': > >>> cn_hash_test.c:(.text+0x3ea387): undefined reference to `cn_del_get_exval' > >>> ld: cn_hash_test.c:(.text+0x3ea3ab): undefined reference to `cn_get_exval' > >>> ld: cn_hash_test.c:(.text+0x3ea3fd): undefined reference to `cn_table_empty' > >>> ld: vmlinux.o: in function `cn_hash_test_add': > >>> cn_hash_test.c:(.text+0x3ea571): undefined reference to `cn_add_elem' > >>> ld: cn_hash_test.c:(.text+0x3ea591): undefined reference to `cn_get_exval' > >>> make[3]: *** [../scripts/Makefile.vmlinux:34: vmlinux] Error 1 > >>> make[2]: *** [/home/kunit/testing/Makefile:1166: vmlinux] Error 2 > >>> make[1]: *** [/home/kunit/testing/Makefile:224: __sub-make] Error 2 > >>> make: *** [Makefile:224: __sub-make] Error 2 > >> > >> Yes, I have added in the comments for CN_HASH_KUNIT_TEST, it depends on: > >> CONFIG_CONNECTOR, CONFIG_PROC_EVENTS, CONFIG_NET. I didn’t realize > >> I could add these to the “depends” field. > >> So something like this: (let me know if you see any issues) > >> > >> tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS > >> depends on KUNIT > >> + depends on CONNECTOR && PROC_EVENTS > >> + depends on NET > >> default KUNIT_ALL_TESTS > >> > >> These are the configs I add to my .config file and compile it as a module and then > >> do modprobe to test. > > > > [..] > > > >> Are you running the kunit tester with kunit.py? > > > > Yes, make sure all required options are picked up by > > "./tools/testing/kunit/kunit.py run" instead of manually adding options > > and doing modprobe. > > The environment issues are resolved and I am able to run kunit.py, but my tests > are not invoked without giving options via —kconfig-add. Other tests are also not > invoked. Running with the manual options runs 413 tests, and with just kunit.py > runs 389 tests. (I have added 6). Any idea how I can make it run my tests? The runner does: ./tools/testing/kunit/kunit.py run --alltests Is it not enough in your case? What options do you pass via --kconfig-add? Is it because CONNECTOR stuff is disabled by default?
> On Oct 22, 2024, at 4:50 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote: > > On 10/22, Anjali Kulkarni wrote: >> >> >>> On Oct 17, 2024, at 5:55 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote: >>> >>> On 10/18, Anjali Kulkarni wrote: >>>> >>>> >>>>> On Oct 17, 2024, at 5:13 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote: >>>>> >>>>> On 10/17, 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 >>>>>> CONFIG_CN_HASH_KUNIT_TEST=m >>>>>> >>>>>> 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 <anjali.k.kulkarni@oracle.com> >>>>>> --- >>>>>> drivers/connector/cn_hash.c | 40 ++++++++ >>>>>> drivers/connector/connector.c | 12 +++ >>>>>> include/linux/connector.h | 4 + >>>>>> lib/Kconfig.debug | 17 ++++ >>>>>> lib/Makefile | 1 + >>>>>> lib/cn_hash_test.c | 167 ++++++++++++++++++++++++++++++++++ >>>>>> lib/cn_hash_test.h | 10 ++ >>>>>> 7 files changed, 251 insertions(+) >>>>>> 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 a079e9bcea6d..40099b5908ac 100644 >>>>>> --- a/drivers/connector/cn_hash.c >>>>>> +++ b/drivers/connector/cn_hash.c >>>>>> @@ -170,6 +170,46 @@ int 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) >>>>>> { >>>>>> bool is_empty; >>>>>> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c >>>>>> index c1c0dcec53c0..2be2fe1adc12 100644 >>>>>> --- a/drivers/connector/connector.c >>>>>> +++ b/drivers/connector/connector.c >>>>>> @@ -304,6 +304,18 @@ int 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 5384e4bb98e8..a75c3fcf182a 100644 >>>>>> --- a/include/linux/connector.h >>>>>> +++ b/include/linux/connector.h >>>>>> @@ -168,4 +168,8 @@ int cn_get_exval(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 *key_display); >>>>>> +int cn_hash_display_hlist(struct cn_hash_dev *hdev, pid_t pid, int max_len, >>>>>> + int *hkey, int *key_display); >>>>>> + >>>>>> #endif /* __CONNECTOR_H */ >>>>>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug >>>>>> index 7315f643817a..290cf0a6befa 100644 >>>>>> --- a/lib/Kconfig.debug >>>>>> +++ b/lib/Kconfig.debug >>>>>> @@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST >>>>>> >>>>>> If unsure, say N. >>>>>> >>>>>> +config CN_HASH_KUNIT_TEST >>>>>> + tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS >>>>>> + depends on KUNIT >>>>>> + default KUNIT_ALL_TESTS >>>>>> + help >>>>>> + This builds the hashtable KUnit test suite. >>>>>> + It tests the basic functionality of the API defined in >>>>>> + drivers/connector/cn_hash.c. >>>>>> + CONFIG_CONNECTOR=y, CONFIG_PROC_EVENTS=y and CONFIG_NET=y needs >>>>>> + to be enabled along with CONFIG_CN_HASH_KUNIT_TEST=m and >>>>>> + CONFIG_KUNIT=m in .config file to compile and then test as a kernel >>>>>> + module with "modprobe cn_hash_test". >>>>>> + For more information on KUnit and unit tests in general please >>>>>> + refer to the KUnit documentation in Documentation/dev-tools/kunit/. >>>>>> + >>>>>> + If unsure, say N. >>>>>> + >>>>> >>>>> Looks like this needs to depend on CONFIG_CONNECTOR? Otherwise, the >>>>> existing kunit tester complains about the missing symbols (see below). >>>>> Please also hold off reposting for a couple of days to give people some >>>>> time to review. >>>>> >>>>> ERROR:root:ld: vmlinux.o: in function `cn_hash_test_dup_del': >>>>> cn_hash_test.c:(.text+0x3e9dc3): undefined reference to `cn_del_get_exval' >>>>> ld: cn_hash_test.c:(.text+0x3e9dee): undefined reference to `cn_del_get_exval' >>>>> ld: cn_hash_test.c:(.text+0x3e9e22): undefined reference to `cn_table_empty' >>>>> ld: vmlinux.o: in function `cn_display_htable': >>>>> cn_hash_test.c:(.text+0x3e9f67): undefined reference to `cn_display_hlist' >>>>> ld: vmlinux.o: in function `cn_hash_test_del_get_exval': >>>>> cn_hash_test.c:(.text+0x3ea037): undefined reference to `cn_del_get_exval' >>>>> ld: cn_hash_test.c:(.text+0x3ea088): undefined reference to `cn_table_empty' >>>>> ld: vmlinux.o: in function `cn_hash_test_dup_add': >>>>> cn_hash_test.c:(.text+0x3ea176): undefined reference to `cn_add_elem' >>>>> ld: cn_hash_test.c:(.text+0x3ea19e): undefined reference to `cn_get_exval' >>>>> ld: cn_hash_test.c:(.text+0x3ea1dc): undefined reference to `cn_add_elem' >>>>> ld: cn_hash_test.c:(.text+0x3ea205): undefined reference to `cn_get_exval' >>>>> ld: vmlinux.o: in function `cn_hash_test_del': >>>>> cn_hash_test.c:(.text+0x3ea387): undefined reference to `cn_del_get_exval' >>>>> ld: cn_hash_test.c:(.text+0x3ea3ab): undefined reference to `cn_get_exval' >>>>> ld: cn_hash_test.c:(.text+0x3ea3fd): undefined reference to `cn_table_empty' >>>>> ld: vmlinux.o: in function `cn_hash_test_add': >>>>> cn_hash_test.c:(.text+0x3ea571): undefined reference to `cn_add_elem' >>>>> ld: cn_hash_test.c:(.text+0x3ea591): undefined reference to `cn_get_exval' >>>>> make[3]: *** [../scripts/Makefile.vmlinux:34: vmlinux] Error 1 >>>>> make[2]: *** [/home/kunit/testing/Makefile:1166: vmlinux] Error 2 >>>>> make[1]: *** [/home/kunit/testing/Makefile:224: __sub-make] Error 2 >>>>> make: *** [Makefile:224: __sub-make] Error 2 >>>> >>>> Yes, I have added in the comments for CN_HASH_KUNIT_TEST, it depends on: >>>> CONFIG_CONNECTOR, CONFIG_PROC_EVENTS, CONFIG_NET. I didn’t realize >>>> I could add these to the “depends” field. >>>> So something like this: (let me know if you see any issues) >>>> >>>> tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS >>>> depends on KUNIT >>>> + depends on CONNECTOR && PROC_EVENTS >>>> + depends on NET >>>> default KUNIT_ALL_TESTS >>>> >>>> These are the configs I add to my .config file and compile it as a module and then >>>> do modprobe to test. >>> >>> [..] >>> >>>> Are you running the kunit tester with kunit.py? >>> >>> Yes, make sure all required options are picked up by >>> "./tools/testing/kunit/kunit.py run" instead of manually adding options >>> and doing modprobe. >> >> The environment issues are resolved and I am able to run kunit.py, but my tests >> are not invoked without giving options via —kconfig-add. Other tests are also not >> invoked. Running with the manual options runs 413 tests, and with just kunit.py >> runs 389 tests. (I have added 6). Any idea how I can make it run my tests? > > The runner does: ./tools/testing/kunit/kunit.py run --alltests > Is it not enough in your case? What options do you pass via > --kconfig-add? Is it because CONNECTOR stuff is disabled by default? No, it still does not run. However, I added to tools/testing/kunit/configs/all_tests.config: CONFIG_CONNECTOR=y CONFIG_PROC_EVENTS=y CONFIG_NET=y CONFIG_CN_HASH_KUNIT_TEST=y And now it does run. Should I make the change above? I will also check with the kunit guys. But I do not understand how it ran for you(and run into the error), or did it just try to compile? Anjali
> On Oct 23, 2024, at 8:05 AM, Stanislav Fomichev <stfomichev@gmail.com> wrote: > > On 10/23, Anjali Kulkarni wrote: >> […snip…] >> >>>>>> >>>>>> Yes, make sure all required options are picked up by >>>>>> "./tools/testing/kunit/kunit.py run" instead of manually adding options >>>>>> and doing modprobe. >>>>> >>>>> The environment issues are resolved and I am able to run kunit.py, but my tests >>>>> are not invoked without giving options via —kconfig-add. Other tests are also not >>>>> invoked. Running with the manual options runs 413 tests, and with just kunit.py >>>>> runs 389 tests. (I have added 6). Any idea how I can make it run my tests? >>>> >>>> The runner does: ./tools/testing/kunit/kunit.py run --alltests >>>> Is it not enough in your case? What options do you pass via >>>> --kconfig-add? Is it because CONNECTOR stuff is disabled by default? >>> >>> No, it still does not run. >>> However, I added to tools/testing/kunit/configs/all_tests.config: >>> >>> CONFIG_CONNECTOR=y >>> CONFIG_PROC_EVENTS=y >>> CONFIG_NET=y >>> CONFIG_CN_HASH_KUNIT_TEST=y >>> >>> And now it does run. >>> Should I make the change above? I will also check with the kunit guys. >>> But I do not understand how it ran for you(and run into the error), or did >>> it just try to compile? >> >> I see this in comments on top of all_tests.config. >> >> # The config is manually maintained, though it uses KUNIT_ALL_TESTS=y to enable >> # any tests whose dependencies are already satisfied. Please feel free to add >> # more options if they any new tests. >> >> So I suppose if a test needs more dependencies, it needs to be added here. > > Let's try and CC a bunch of kunit people to confirm :-) Ok! Will send out a new patch and cc the kunit folks on it. Hopefully it works:)