* [PATCH net-next 1/3] connector/cn_proc: Add hash table for threads
2024-10-12 0:45 [PATCH net-next 0/3] Threads support in proc connector Anjali Kulkarni
@ 2024-10-12 0:45 ` Anjali Kulkarni
2024-10-12 9:43 ` Simon Horman
2024-10-12 0:45 ` [PATCH net-next 2/3] connector/cn_proc: Kunit tests for threads hash table Anjali Kulkarni
2024-10-12 0:45 ` [PATCH net-next 3/3] connector/cn_proc: Selftest for threads Anjali Kulkarni
2 siblings, 1 reply; 7+ messages in thread
From: Anjali Kulkarni @ 2024-10-12 0:45 UTC (permalink / raw)
To: davem, Liam.Howlett
Cc: edumazet, kuba, pabeni, mingo, peterz, juri.lelli,
vincent.guittot, dietmar.eggemann, rostedt, bsegall, mgorman,
vschneid, jiri, linux-kernel, netdev, akpm, shuah,
linux-kselftest, anjali.k.kulkarni, peili.io
Add a new type PROC_CN_MCAST_NOTIFY to proc connector API, which allows a
thread to notify the kernel that it has exited abnormally. Thread can also
send the exit status code it wants returned in the notification with it.
Exiting thread can call this either when it wants to call pthread_exit()
with non-zero value or from signal handler.
Add a new file cn_hash.c which implements a hash table storing the exit
codes of abnormally exiting threads, received by the system call above.
The key used for the hash table is the pid of the thread, so when the
thread actually exits, we lookup it's pid in the hash table and retrieve
the exit code sent by user. If the exit code in struct task is 0, we
then replace it with the user supplied non-zero exit code.
cn_hash.c implements the hash table add, delete, lookup operations.
mutex_lock() and mutex_unlock() operations are used to safeguard the
integrity of the hash table while adding or deleting elements.
connector.c has the API calls, called from cn_proc.c, as well as calls
to allocate, initialize and free the hash table.
Add a new flag in PF_* flags of task_struct - EXIT_NOTIFY. This flag is
set when user sends the exit code via PROC_CN_MCAST_NOTIFY. While
exiting, this flag is checked and the hash table add or delete calls
are only made if this flag is set.
A refcount field hrefcnt is added in struct cn_hash_dev, to keep track
of number of threads which have added an entry in hash table. Before
freeing the struct cn_hash_dev, this value must be 0.
Signed-off-by: Anjali Kulkarni <anjali.k.kulkarni@oracle.com>
---
drivers/connector/Makefile | 2 +-
drivers/connector/cn_hash.c | 195 ++++++++++++++++++++++++++++++++++
drivers/connector/cn_proc.c | 59 +++++++++-
drivers/connector/connector.c | 83 ++++++++++++++-
include/linux/connector.h | 43 ++++++++
include/linux/sched.h | 2 +-
include/uapi/linux/cn_proc.h | 4 +-
7 files changed, 379 insertions(+), 9 deletions(-)
create mode 100644 drivers/connector/cn_hash.c
diff --git a/drivers/connector/Makefile b/drivers/connector/Makefile
index 1bf67d3df97d..cb1dcdf067ad 100644
--- a/drivers/connector/Makefile
+++ b/drivers/connector/Makefile
@@ -2,4 +2,4 @@
obj-$(CONFIG_CONNECTOR) += cn.o
obj-$(CONFIG_PROC_EVENTS) += cn_proc.o
-cn-y += cn_queue.o connector.o
+cn-y += cn_hash.o cn_queue.o connector.o
diff --git a/drivers/connector/cn_hash.c b/drivers/connector/cn_hash.c
new file mode 100644
index 000000000000..a0211cd99132
--- /dev/null
+++ b/drivers/connector/cn_hash.c
@@ -0,0 +1,195 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Author: Anjali Kulkarni <anjali.k.kulkarni@oracle.com>
+ *
+ * Copyright (c) 2024 Oracle and/or its affiliates.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/connector.h>
+#include <linux/mutex.h>
+#include <linux/pid_namespace.h>
+
+#include <linux/cn_proc.h>
+
+struct cn_hash_dev *cn_hash_alloc_dev(const char *name)
+{
+ struct cn_hash_dev *hdev;
+
+ hdev = kzalloc(sizeof(*hdev), GFP_KERNEL);
+ if (!hdev)
+ return NULL;
+
+ snprintf(hdev->name, sizeof(hdev->name), "%s", name);
+ atomic_set(&hdev->hrefcnt, 0);
+ mutex_init(&hdev->uexit_hash_lock);
+ hash_init(hdev->uexit_pid_htable);
+ return hdev;
+}
+
+void cn_hash_free_dev(struct cn_hash_dev *hdev)
+{
+ struct uexit_pid_hnode *hnode;
+ struct hlist_node *tmp;
+ int bucket;
+
+ pr_debug("%s: Freeing entire hdev %p\n", __func__, hdev);
+
+ mutex_lock(&hdev->uexit_hash_lock);
+ hash_for_each_safe(hdev->uexit_pid_htable, bucket, tmp,
+ hnode, uexit_pid_hlist) {
+ hash_del(&hnode->uexit_pid_hlist);
+ pr_debug("%s: Freeing node for pid %d\n",
+ __func__, hnode->pid);
+ kfree(hnode);
+ }
+
+ mutex_unlock(&hdev->uexit_hash_lock);
+ mutex_destroy(&hdev->uexit_hash_lock);
+
+ while (atomic_read(&hdev->hrefcnt)) {
+ pr_info("Waiting for %s to become free: refcnt=%d\n",
+ hdev->name, atomic_read(&hdev->hrefcnt));
+ msleep(1000);
+ }
+
+ kfree(hdev);
+}
+
+static struct uexit_pid_hnode *cn_hash_alloc_elem(__u32 uexit_code, pid_t pid)
+{
+ struct uexit_pid_hnode *elem;
+
+ elem = kzalloc(sizeof(*elem), GFP_KERNEL);
+ if (!elem)
+ return NULL;
+
+ INIT_HLIST_NODE(&elem->uexit_pid_hlist);
+ elem->uexit_code = uexit_code;
+ elem->pid = pid;
+ return elem;
+}
+
+void cn_hash_free_elem(struct uexit_pid_hnode *elem)
+{
+ kfree(elem);
+}
+
+int cn_hash_add_elem(struct cn_hash_dev *hdev, __u32 uexit_code, pid_t pid)
+{
+ struct uexit_pid_hnode *elem, *hnode;
+
+ elem = cn_hash_alloc_elem(uexit_code, pid);
+ if (!elem) {
+ pr_err("%s: cn_hash_alloc_elem() returned NULL pid %d\n",
+ __func__, pid);
+ return -ENOMEM;
+ }
+
+ mutex_lock(&hdev->uexit_hash_lock);
+ /*
+ * Check if an entry for the same pid already exists
+ */
+ hash_for_each_possible(hdev->uexit_pid_htable,
+ hnode, uexit_pid_hlist, pid) {
+ if (hnode->pid == pid) {
+ mutex_unlock(&hdev->uexit_hash_lock);
+ cn_hash_free_elem(elem);
+ pr_debug("%s: pid %d already exists in hash table\n",
+ __func__, pid);
+ return -EEXIST;
+ }
+ }
+
+ hash_add(hdev->uexit_pid_htable, &elem->uexit_pid_hlist, pid);
+ mutex_unlock(&hdev->uexit_hash_lock);
+
+ atomic_inc(&hdev->hrefcnt);
+
+ pr_debug("%s: After hash_add of pid %d elem %p hrefcnt %d\n",
+ __func__, pid, elem, atomic_read(&hdev->hrefcnt));
+ return 0;
+}
+
+int cn_hash_del_elem(struct cn_hash_dev *hdev, pid_t pid)
+{
+ struct uexit_pid_hnode *hnode;
+ struct hlist_node *tmp;
+
+ mutex_lock(&hdev->uexit_hash_lock);
+ hash_for_each_possible_safe(hdev->uexit_pid_htable,
+ hnode, tmp, uexit_pid_hlist, pid) {
+ if (hnode && hnode->pid == pid) {
+ hash_del(&hnode->uexit_pid_hlist);
+ mutex_unlock(&hdev->uexit_hash_lock);
+ kfree(hnode);
+ atomic_dec(&hdev->hrefcnt);
+ pr_debug("%s: After hash_del of pid %d, hrefcnt %d\n",
+ __func__, pid,
+ atomic_read(&hdev->hrefcnt));
+ return 0;
+ }
+ }
+
+ mutex_unlock(&hdev->uexit_hash_lock);
+ pr_err("%s: pid %d not found in hash table\n",
+ __func__, pid);
+ return -EINVAL;
+}
+
+__u32 cn_hash_del_get_exval(struct cn_hash_dev *hdev, pid_t pid)
+{
+ struct uexit_pid_hnode *hnode;
+ struct hlist_node *tmp;
+ __u32 excde;
+
+ mutex_lock(&hdev->uexit_hash_lock);
+ hash_for_each_possible_safe(hdev->uexit_pid_htable,
+ hnode, tmp, uexit_pid_hlist, pid) {
+ if (hnode->pid == pid) {
+ excde = hnode->uexit_code;
+ hash_del(&hnode->uexit_pid_hlist);
+ mutex_unlock(&hdev->uexit_hash_lock);
+ kfree(hnode);
+ atomic_dec(&hdev->hrefcnt);
+ pr_debug("%s: After hash_del of pid %d, found exit code %u hrefcnt %d\n",
+ __func__, pid, excde,
+ atomic_read(&hdev->hrefcnt));
+ return excde;
+ }
+ }
+
+ mutex_unlock(&hdev->uexit_hash_lock);
+ pr_err("%s: pid %d not found in hash table\n",
+ __func__, pid);
+ return 0;
+}
+
+__u32 cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t pid)
+{
+ struct uexit_pid_hnode *hnode;
+ __u32 excde;
+
+ mutex_lock(&hdev->uexit_hash_lock);
+ hash_for_each_possible(hdev->uexit_pid_htable,
+ hnode, uexit_pid_hlist, pid) {
+ if (hnode->pid == pid) {
+ excde = hnode->uexit_code;
+ mutex_unlock(&hdev->uexit_hash_lock);
+ pr_debug("%s: Found exit code %u for pid %d\n",
+ __func__, excde, pid);
+ return excde;
+ }
+ }
+
+ mutex_unlock(&hdev->uexit_hash_lock);
+ pr_debug("%s: pid %d not found in hash table\n",
+ __func__, pid);
+ return -EINVAL;
+}
+
+bool cn_hash_table_empty(struct cn_hash_dev *hdev)
+{
+ return hash_empty(hdev->uexit_pid_htable);
+}
diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index 44b19e696176..8c6e002069d9 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -69,6 +69,8 @@ static int cn_filter(struct sock *dsk, struct sk_buff *skb, void *data)
if ((__u32)val == PROC_EVENT_ALL)
return 0;
+ pr_debug("%s: val %lx, what %x\n", __func__, val, what);
+
/*
* Drop packet if we have to report only non-zero exit status
* (PROC_EVENT_NONZERO_EXIT) and exit status is 0
@@ -326,9 +328,16 @@ void proc_exit_connector(struct task_struct *task)
struct proc_event *ev;
struct task_struct *parent;
__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);
+ __u32 uexit_code;
+ int err;
- if (atomic_read(&proc_event_num_listeners) < 1)
+ if (atomic_read(&proc_event_num_listeners) < 1) {
+ if (likely(!(task->flags & PF_EXIT_NOTIFY)))
+ return;
+
+ err = cn_del_elem(task->pid);
return;
+ }
msg = buffer_to_cn_msg(buffer);
ev = (struct proc_event *)msg->data;
@@ -337,7 +346,26 @@ void proc_exit_connector(struct task_struct *task)
ev->what = PROC_EVENT_EXIT;
ev->event_data.exit.process_pid = task->pid;
ev->event_data.exit.process_tgid = task->tgid;
- ev->event_data.exit.exit_code = task->exit_code;
+ if (unlikely(task->flags & PF_EXIT_NOTIFY)) {
+ task_lock(task);
+ task->flags &= ~PF_EXIT_NOTIFY;
+ task_unlock(task);
+
+ uexit_code = cn_del_get_exval(task->pid);
+ if (uexit_code == 0) {
+ pr_debug("%s: Returning with task's exit code %u\n",
+ __func__, task->exit_code);
+ ev->event_data.exit.exit_code = task->exit_code;
+ } else {
+ ev->event_data.exit.exit_code = uexit_code;
+ pr_debug("%s: Reset PF_EXIT_NOTIFY & retrieved exit code %u from hash table, pid %d\n",
+ __func__, task->pid,
+ ev->event_data.exit.exit_code);
+ }
+ } else {
+ ev->event_data.exit.exit_code = task->exit_code;
+ }
+
ev->event_data.exit.exit_signal = task->exit_signal;
rcu_read_lock();
@@ -413,6 +441,15 @@ static void cn_proc_mcast_ctl(struct cn_msg *msg,
if (msg->len == sizeof(*pinput)) {
pinput = (struct proc_input *)msg->data;
mc_op = pinput->mcast_op;
+ if (mc_op == PROC_CN_MCAST_NOTIFY) {
+ pr_debug("%s: Received PROC_CN_MCAST_NOTIFY, pid %d\n",
+ __func__, current->pid);
+ task_lock(current);
+ current->flags |= PF_EXIT_NOTIFY;
+ task_unlock(current);
+ err = cn_add_elem(pinput->uexit_code, current->pid);
+ return;
+ }
ev_type = pinput->event_type;
} else if (msg->len == sizeof(mc_op)) {
mc_op = *((enum proc_cn_mcast_op *)msg->data);
@@ -432,6 +469,8 @@ static void cn_proc_mcast_ctl(struct cn_msg *msg,
sk->sk_user_data = kzalloc(sizeof(struct proc_input),
GFP_KERNEL);
if (sk->sk_user_data == NULL) {
+ pr_err("%s: ENOMEM for sk_user_data, pid %d\n",
+ __func__, current->pid);
err = ENOMEM;
goto out;
}
@@ -442,21 +481,33 @@ static void cn_proc_mcast_ctl(struct cn_msg *msg,
}
((struct proc_input *)(sk->sk_user_data))->event_type =
ev_type;
+ pr_debug("%s: sk: %p pid: %d event_type: %x\n",
+ __func__, sk, current->pid, ev_type);
((struct proc_input *)(sk->sk_user_data))->mcast_op = mc_op;
}
switch (mc_op) {
case PROC_CN_MCAST_LISTEN:
- if (initial || (prev_mc_op != PROC_CN_MCAST_LISTEN))
+ if (initial || (prev_mc_op != PROC_CN_MCAST_LISTEN)) {
atomic_inc(&proc_event_num_listeners);
+ pr_debug("%s: PROC_CN_MCAST_LISTEN pid %d: Incremented listeners to %d\n",
+ __func__, current->pid,
+ atomic_read(&proc_event_num_listeners));
+ }
break;
case PROC_CN_MCAST_IGNORE:
- if (!initial && (prev_mc_op != PROC_CN_MCAST_IGNORE))
+ if (!initial && (prev_mc_op != PROC_CN_MCAST_IGNORE)) {
atomic_dec(&proc_event_num_listeners);
+ pr_debug("%s: PROC_CN_MCAST_IGNORE pid %d: Decremented listeners to %d\n",
+ __func__, current->pid,
+ atomic_read(&proc_event_num_listeners));
+ }
((struct proc_input *)(sk->sk_user_data))->event_type =
PROC_EVENT_NONE;
break;
default:
+ pr_warn("%s: Invalid value for mc_op %d\n",
+ __func__, mc_op);
err = EINVAL;
break;
}
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index 4028e8eeba82..506e3cbedf85 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -271,6 +271,67 @@ static int __maybe_unused cn_proc_show(struct seq_file *m, void *v)
return 0;
}
+__u32 cn_del_get_exval(pid_t pid)
+{
+ struct cn_dev *dev = &cdev;
+ __u32 exval;
+
+ if (!cn_already_initialized)
+ return 0;
+
+ exval = cn_hash_del_get_exval(dev->hdev, pid);
+ return exval;
+}
+EXPORT_SYMBOL_GPL(cn_del_get_exval);
+
+int cn_del_elem(pid_t pid)
+{
+ struct cn_dev *dev = &cdev;
+ int ret;
+
+ if (!cn_already_initialized)
+ return 0;
+
+ ret = cn_hash_del_elem(dev->hdev, pid);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(cn_del_elem);
+
+int cn_add_elem(__u32 uexit_code, pid_t pid)
+{
+ struct cn_dev *dev = &cdev;
+
+ if (!cn_already_initialized)
+ return 0;
+
+ return cn_hash_add_elem(dev->hdev, uexit_code, pid);
+}
+EXPORT_SYMBOL_GPL(cn_add_elem);
+
+__u32 cn_get_exval(pid_t pid)
+{
+ struct cn_dev *dev = &cdev;
+ __u32 exval;
+
+ if (!cn_already_initialized)
+ return 0;
+
+ exval = cn_hash_get_exval(dev->hdev, pid);
+ return exval;
+}
+EXPORT_SYMBOL_GPL(cn_get_exval);
+
+bool cn_table_empty(void)
+{
+ struct cn_dev *dev = &cdev;
+
+ if (!cn_already_initialized)
+ return 0;
+
+ return cn_hash_table_empty(dev->hdev);
+}
+EXPORT_SYMBOL_GPL(cn_table_empty);
+
static int cn_init(void)
{
struct cn_dev *dev = &cdev;
@@ -283,18 +344,35 @@ static int cn_init(void)
};
dev->nls = netlink_kernel_create(&init_net, NETLINK_CONNECTOR, &cfg);
- if (!dev->nls)
+ if (!dev->nls) {
+ pr_err("%s: netlink_kernel_create failed, connector not initialized\n",
+ __func__);
return -EIO;
+ }
dev->cbdev = cn_queue_alloc_dev("cqueue", dev->nls);
if (!dev->cbdev) {
+ pr_err("%s: Allocation of dev->cbdev failed, connector not initialized\n",
+ __func__);
netlink_kernel_release(dev->nls);
return -EINVAL;
}
+ dev->hdev = cn_hash_alloc_dev("pid hash table");
+ if (!dev->hdev) {
+ pr_err("%s: Allocation of dev->hdev failed, connector not initialized\n",
+ __func__);
+ netlink_kernel_release(dev->nls);
+ cn_queue_free_dev(dev->cbdev);
+ return -ENOMEM;
+ }
+
+ pr_debug("Connector initialized, allocated hdev %p\n", dev->hdev);
+
cn_already_initialized = 1;
- proc_create_single("connector", S_IRUGO, init_net.proc_net, cn_proc_show);
+ proc_create_single("connector", S_IRUGO, init_net.proc_net,
+ cn_proc_show);
return 0;
}
@@ -308,6 +386,7 @@ static void cn_fini(void)
remove_proc_entry("connector", init_net.proc_net);
cn_queue_free_dev(dev->cbdev);
+ cn_hash_free_dev(dev->hdev);
netlink_kernel_release(dev->nls);
}
diff --git a/include/linux/connector.h b/include/linux/connector.h
index 70bc1160f3d8..094e1730a4f6 100644
--- a/include/linux/connector.h
+++ b/include/linux/connector.h
@@ -18,6 +18,8 @@
#include <uapi/linux/connector.h>
#define CN_CBQ_NAMELEN 32
+#define HASHT_NAMELEN 32
+#define PID_HASH_TABLE_BITS 10
struct cn_queue_dev {
atomic_t refcnt;
@@ -45,6 +47,19 @@ struct cn_callback_entry {
u32 seq, group;
};
+struct uexit_pid_hnode {
+ __u32 uexit_code;
+ pid_t pid;
+ struct hlist_node uexit_pid_hlist;
+};
+
+struct cn_hash_dev {
+ atomic_t hrefcnt;
+ unsigned char name[HASHT_NAMELEN];
+ struct mutex uexit_hash_lock;
+ DECLARE_HASHTABLE(uexit_pid_htable, PID_HASH_TABLE_BITS);
+};
+
struct cn_dev {
struct cb_id id;
@@ -52,6 +67,7 @@ struct cn_dev {
struct sock *nls;
struct cn_queue_dev *cbdev;
+ struct cn_hash_dev *hdev;
};
/**
@@ -137,4 +153,31 @@ void cn_queue_free_dev(struct cn_queue_dev *dev);
int cn_cb_equal(const struct cb_id *, const struct cb_id *);
+struct cn_hash_dev *cn_hash_alloc_dev(const char *name);
+void cn_hash_free_dev(struct cn_hash_dev *hdev);
+struct uexit_pid_hnode *cn_hash_find_pid_node(struct cn_hash_dev *hdev,
+ pid_t pid);
+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);
+
+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);
+
+struct cn_hash_dev *cn_hash_alloc_dev(const char *name);
+void cn_hash_free_dev(struct cn_hash_dev *hdev);
+struct uexit_pid_hnode *cn_hash_find_pid_node(struct cn_hash_dev *hdev,
+ pid_t pid);
+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);
+
+bool cn_table_empty(void);
+bool cn_hash_table_empty(struct cn_hash_dev *hdev);
+
#endif /* __CONNECTOR_H */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index e6ee4258169a..a2339ae6208b 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1673,7 +1673,7 @@ extern struct pid *cad_pid;
#define PF_USED_MATH 0x00002000 /* If unset the fpu must be initialized before use */
#define PF_USER_WORKER 0x00004000 /* Kernel thread cloned from userspace thread */
#define PF_NOFREEZE 0x00008000 /* This thread should not be frozen */
-#define PF__HOLE__00010000 0x00010000
+#define PF_EXIT_NOTIFY 0x00010000 /* This thread has sent an exit value to be sent as a notification to listening processes */
#define PF_KSWAPD 0x00020000 /* I am kswapd */
#define PF_MEMALLOC_NOFS 0x00040000 /* All allocations inherit GFP_NOFS. See memalloc_nfs_save() */
#define PF_MEMALLOC_NOIO 0x00080000 /* All allocations inherit GFP_NOIO. See memalloc_noio_save() */
diff --git a/include/uapi/linux/cn_proc.h b/include/uapi/linux/cn_proc.h
index 18e3745b86cd..2b12a24e4651 100644
--- a/include/uapi/linux/cn_proc.h
+++ b/include/uapi/linux/cn_proc.h
@@ -27,7 +27,8 @@
*/
enum proc_cn_mcast_op {
PROC_CN_MCAST_LISTEN = 1,
- PROC_CN_MCAST_IGNORE = 2
+ PROC_CN_MCAST_IGNORE = 2,
+ PROC_CN_MCAST_NOTIFY = 3
};
#define PROC_EVENT_ALL (PROC_EVENT_FORK | PROC_EVENT_EXEC | PROC_EVENT_UID | \
@@ -65,6 +66,7 @@ enum proc_cn_event {
struct proc_input {
enum proc_cn_mcast_op mcast_op;
enum proc_cn_event event_type;
+ __u32 uexit_code;
};
static inline enum proc_cn_event valid_event(enum proc_cn_event ev_type)
--
2.46.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH net-next 1/3] connector/cn_proc: Add hash table for threads
2024-10-12 0:45 ` [PATCH net-next 1/3] connector/cn_proc: Add hash table for threads Anjali Kulkarni
@ 2024-10-12 9:43 ` Simon Horman
0 siblings, 0 replies; 7+ messages in thread
From: Simon Horman @ 2024-10-12 9:43 UTC (permalink / raw)
To: Anjali Kulkarni
Cc: davem, Liam.Howlett, edumazet, kuba, pabeni, mingo, peterz,
juri.lelli, vincent.guittot, dietmar.eggemann, rostedt, bsegall,
mgorman, vschneid, jiri, linux-kernel, netdev, akpm, shuah,
linux-kselftest, peili.io
On Fri, Oct 11, 2024 at 05:45:30PM -0700, Anjali Kulkarni wrote:
> Add a new type PROC_CN_MCAST_NOTIFY to proc connector API, which allows a
> thread to notify the kernel that it has exited abnormally. Thread can also
> send the exit status code it wants returned in the notification with it.
> Exiting thread can call this either when it wants to call pthread_exit()
> with non-zero value or from signal handler.
>
> Add a new file cn_hash.c which implements a hash table storing the exit
> codes of abnormally exiting threads, received by the system call above.
> The key used for the hash table is the pid of the thread, so when the
> thread actually exits, we lookup it's pid in the hash table and retrieve
> the exit code sent by user. If the exit code in struct task is 0, we
> then replace it with the user supplied non-zero exit code.
>
> cn_hash.c implements the hash table add, delete, lookup operations.
> mutex_lock() and mutex_unlock() operations are used to safeguard the
> integrity of the hash table while adding or deleting elements.
> connector.c has the API calls, called from cn_proc.c, as well as calls
> to allocate, initialize and free the hash table.
>
> Add a new flag in PF_* flags of task_struct - EXIT_NOTIFY. This flag is
> set when user sends the exit code via PROC_CN_MCAST_NOTIFY. While
> exiting, this flag is checked and the hash table add or delete calls
> are only made if this flag is set.
>
> A refcount field hrefcnt is added in struct cn_hash_dev, to keep track
> of number of threads which have added an entry in hash table. Before
> freeing the struct cn_hash_dev, this value must be 0.
>
> Signed-off-by: Anjali Kulkarni <anjali.k.kulkarni@oracle.com>
...
> diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
> index 44b19e696176..8c6e002069d9 100644
> --- a/drivers/connector/cn_proc.c
> +++ b/drivers/connector/cn_proc.c
...
> @@ -326,9 +328,16 @@ void proc_exit_connector(struct task_struct *task)
> struct proc_event *ev;
> struct task_struct *parent;
> __u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);
> + __u32 uexit_code;
> + int err;
>
> - if (atomic_read(&proc_event_num_listeners) < 1)
> + if (atomic_read(&proc_event_num_listeners) < 1) {
> + if (likely(!(task->flags & PF_EXIT_NOTIFY)))
> + return;
> +
> + err = cn_del_elem(task->pid);
Hi Anjali,
err is set but otherwise unused in this function; probably it can be removed.
> return;
> + }
>
> msg = buffer_to_cn_msg(buffer);
> ev = (struct proc_event *)msg->data;
...
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next 2/3] connector/cn_proc: Kunit tests for threads hash table
2024-10-12 0:45 [PATCH net-next 0/3] Threads support in proc connector Anjali Kulkarni
2024-10-12 0:45 ` [PATCH net-next 1/3] connector/cn_proc: Add hash table for threads Anjali Kulkarni
@ 2024-10-12 0:45 ` Anjali Kulkarni
2024-10-12 9:45 ` Simon Horman
2024-10-12 0:45 ` [PATCH net-next 3/3] connector/cn_proc: Selftest for threads Anjali Kulkarni
2 siblings, 1 reply; 7+ messages in thread
From: Anjali Kulkarni @ 2024-10-12 0:45 UTC (permalink / raw)
To: davem, Liam.Howlett
Cc: edumazet, kuba, pabeni, mingo, peterz, juri.lelli,
vincent.guittot, dietmar.eggemann, rostedt, bsegall, mgorman,
vschneid, jiri, linux-kernel, netdev, akpm, shuah,
linux-kselftest, anjali.k.kulkarni, peili.io
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 <anjali.k.kulkarni@oracle.com>
---
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 *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..23599beb9bec 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST
If unsure, say N.
+config 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 as well as 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.
+
config LINEAR_RANGES_TEST
tristate "KUnit test for linear_ranges"
depends on KUNIT
diff --git a/lib/Makefile b/lib/Makefile
index 811ba12c8cd0..2c59c82b0b18 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -379,6 +379,7 @@ obj-$(CONFIG_CMDLINE_KUNIT_TEST) += cmdline_kunit.o
obj-$(CONFIG_SLUB_KUNIT_TEST) += slub_kunit.o
obj-$(CONFIG_MEMCPY_KUNIT_TEST) += memcpy_kunit.o
obj-$(CONFIG_IS_SIGNED_TYPE_KUNIT_TEST) += is_signed_type_kunit.o
+obj-$(CONFIG_CN_HASH_KUNIT_TEST) += cn_hash_test.o
CFLAGS_overflow_kunit.o = $(call cc-disable-warning, tautological-constant-out-of-range-compare)
obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow_kunit.o
CFLAGS_stackinit_kunit.o += $(call cc-disable-warning, switch-unreachable)
diff --git a/lib/cn_hash_test.c b/lib/cn_hash_test.c
new file mode 100644
index 000000000000..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 <anjali.k.kulkarni@oracle.com>
+ */
+#include <kunit/test.h>
+
+#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];
+
+static int cn_hash_init(struct kunit *test)
+{
+ for (int i = 0; i < HASH_TABLE_LEN; i++)
+ key_display[i] = 0;
+
+ return 0;
+}
+
+static void cn_display_htable(struct kunit *test, int len)
+{
+ int i, err;
+
+ cn_hash_init(test);
+
+ pr_debug("\n");
+ pr_debug("Displaying hash table:\n");
+
+ for (i = 0; i < len; i++) {
+ err = cn_display_hlist(adata[i].pid, len, &adata[i].key,
+ key_display);
+ key_display[adata[i].key] = 1;
+ KUNIT_EXPECT_EQ(test, err, 0);
+ }
+}
+
+static void cn_hash_test_add(struct kunit *test)
+{
+ int err, i;
+ int exit_val;
+
+ adata[0].pid = 1;
+ adata[0].exit_val = 45;
+
+ adata[1].pid = 2;
+ adata[1].exit_val = 13;
+
+ adata[2].pid = 1024;
+ adata[2].exit_val = 16;
+
+ adata[3].pid = 1023;
+ adata[3].exit_val = 71;
+
+ for (i = 0; i < ARRAY_SIZE(adata); i++) {
+ err = cn_add_elem(adata[i].exit_val, adata[i].pid);
+ KUNIT_EXPECT_EQ_MSG(test, 0, err,
+ "Adding pid %d returned err %d",
+ adata[i].pid, err);
+
+ exit_val = cn_get_exval(adata[i].pid);
+ KUNIT_EXPECT_EQ(test, adata[i].exit_val, exit_val);
+ }
+
+ cn_display_htable(test, ARRAY_SIZE(adata));
+}
+
+static void cn_hash_test_del(struct kunit *test)
+{
+ int i, err;
+ int exit_val;
+
+ for (i = 0; i < ARRAY_SIZE(adata); i++) {
+ err = cn_del_elem(adata[i].pid);
+ KUNIT_EXPECT_EQ_MSG(test, 0, err,
+ "Deleting pid %d returned err %d",
+ adata[i].pid, err);
+
+ exit_val = cn_get_exval(adata[i].pid);
+ KUNIT_EXPECT_EQ(test, -EINVAL, exit_val);
+ }
+
+ cn_display_htable(test, ARRAY_SIZE(adata));
+ KUNIT_EXPECT_TRUE(test, cn_table_empty());
+}
+
+static void cn_hash_test_del_get_exval(struct kunit *test)
+{
+ int i, exval;
+
+ for (i = 0; i < ARRAY_SIZE(adata); i++) {
+ exval = cn_del_get_exval(adata[i].pid);
+ KUNIT_EXPECT_EQ(test, adata[i].exit_val, exval);
+
+ cn_display_htable(test, ARRAY_SIZE(adata));
+ }
+
+ KUNIT_EXPECT_TRUE(test, cn_table_empty());
+}
+static void cn_hash_test_dup_add(struct kunit *test)
+{
+ int err, exit_val;
+
+ adata[0].pid = 10;
+ adata[0].exit_val = 21;
+
+ err = cn_add_elem(adata[0].exit_val, adata[0].pid);
+ KUNIT_EXPECT_EQ(test, 0, err);
+
+ exit_val = cn_get_exval(adata[0].pid);
+ KUNIT_EXPECT_EQ(test, 21, exit_val);
+
+ adata[1].pid = 10;
+ adata[1].exit_val = 12;
+
+ err = cn_add_elem(adata[1].exit_val, adata[1].pid);
+ KUNIT_EXPECT_EQ(test, -EEXIST, err);
+
+ exit_val = cn_get_exval(adata[1].pid);
+ KUNIT_EXPECT_EQ(test, 21, exit_val);
+
+ cn_display_htable(test, 1);
+}
+
+static void cn_hash_test_dup_del(struct kunit *test)
+{
+ int err;
+
+ err = cn_del_elem(adata[0].pid);
+ KUNIT_EXPECT_EQ(test, 0, err);
+
+ err = cn_del_elem(adata[0].pid);
+ KUNIT_EXPECT_EQ(test, -EINVAL, err);
+
+ KUNIT_EXPECT_TRUE(test, cn_table_empty());
+}
+
+static struct kunit_case cn_hashtable_test_cases[] = {
+ KUNIT_CASE(cn_hash_test_add),
+ KUNIT_CASE(cn_hash_test_del),
+ KUNIT_CASE(cn_hash_test_dup_add),
+ KUNIT_CASE(cn_hash_test_dup_del),
+ KUNIT_CASE(cn_hash_test_add),
+ KUNIT_CASE(cn_hash_test_del_get_exval),
+ {},
+};
+
+static struct kunit_suite cn_hashtable_test_module = {
+ .name = "cn_hashtable",
+ .init = cn_hash_init,
+ .test_cases = cn_hashtable_test_cases,
+};
+kunit_test_suite(cn_hashtable_test_module);
+
+MODULE_DESCRIPTION("KUnit test for the connector threads hashtable code");
+MODULE_LICENSE("GPL");
diff --git a/lib/cn_hash_test.h b/lib/cn_hash_test.h
new file mode 100644
index 000000000000..46fcda31b25c
--- /dev/null
+++ b/lib/cn_hash_test.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2024 Oracle and/or its affiliates.
+ * Author: Anjali Kulkarni <anjali.k.kulkarni@oracle.com>
+ */
+extern int cn_display_hlist(pid_t pid, int max_len, int *hkey,
+ int *key_display);
+extern int cn_del_elem(pid_t pid);
+extern int cn_add_elem(__u32 uexit_code, pid_t pid);
+extern __u32 cn_del_get_exval(pid_t pid);
+extern int cn_get_exval(pid_t pid);
+extern bool cn_table_empty(void);
--
2.46.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH net-next 2/3] connector/cn_proc: Kunit tests for threads hash table
2024-10-12 0:45 ` [PATCH net-next 2/3] connector/cn_proc: Kunit tests for threads hash table Anjali Kulkarni
@ 2024-10-12 9:45 ` Simon Horman
2024-10-13 3:06 ` Anjali Kulkarni
0 siblings, 1 reply; 7+ messages in thread
From: Simon Horman @ 2024-10-12 9:45 UTC (permalink / raw)
To: Anjali Kulkarni
Cc: davem, Liam.Howlett, edumazet, kuba, pabeni, mingo, peterz,
juri.lelli, vincent.guittot, dietmar.eggemann, rostedt, bsegall,
mgorman, vschneid, jiri, linux-kernel, netdev, akpm, shuah,
linux-kselftest, peili.io
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 <anjali.k.kulkarni@oracle.com>
...
> index 000000000000..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 <anjali.k.kulkarni@oracle.com>
> + */
> +#include <kunit/test.h>
> +
> +#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.
...
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net-next 2/3] connector/cn_proc: Kunit tests for threads hash table
2024-10-12 9:45 ` Simon Horman
@ 2024-10-13 3:06 ` Anjali Kulkarni
0 siblings, 0 replies; 7+ messages in thread
From: Anjali Kulkarni @ 2024-10-13 3:06 UTC (permalink / raw)
To: Simon Horman
Cc: davem@davemloft.net, Liam Howlett, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, mingo@redhat.com,
peterz@infradead.org, juri.lelli@redhat.com,
vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
vschneid@redhat.com, jiri@resnulli.us,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
akpm@linux-foundation.org, shuah@kernel.org,
linux-kselftest@vger.kernel.org, Pei Li
________________________________________
From: Simon Horman <horms@kernel.org>
Sent: Saturday, October 12, 2024 2:45 AM
To: Anjali Kulkarni
Cc: davem@davemloft.net; Liam Howlett; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; mingo@redhat.com; peterz@infradead.org; juri.lelli@redhat.com; vincent.guittot@linaro.org; dietmar.eggemann@arm.com; rostedt@goodmis.org; bsegall@google.com; mgorman@suse.de; vschneid@redhat.com; jiri@resnulli.us; linux-kernel@vger.kernel.org; netdev@vger.kernel.org; akpm@linux-foundation.org; shuah@kernel.org; linux-kselftest@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 <anjali.k.kulkarni@oracle.com>
...
> index 000000000000..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 <anjali.k.kulkarni@oracle.com>
> + */
> +#include <kunit/test.h>
> +
> +#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
...
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next 3/3] connector/cn_proc: Selftest for threads
2024-10-12 0:45 [PATCH net-next 0/3] Threads support in proc connector Anjali Kulkarni
2024-10-12 0:45 ` [PATCH net-next 1/3] connector/cn_proc: Add hash table for threads Anjali Kulkarni
2024-10-12 0:45 ` [PATCH net-next 2/3] connector/cn_proc: Kunit tests for threads hash table Anjali Kulkarni
@ 2024-10-12 0:45 ` Anjali Kulkarni
2 siblings, 0 replies; 7+ messages in thread
From: Anjali Kulkarni @ 2024-10-12 0:45 UTC (permalink / raw)
To: davem, Liam.Howlett
Cc: edumazet, kuba, pabeni, mingo, peterz, juri.lelli,
vincent.guittot, dietmar.eggemann, rostedt, bsegall, mgorman,
vschneid, jiri, linux-kernel, netdev, akpm, shuah,
linux-kselftest, anjali.k.kulkarni, peili.io
Test to check if setting PROC_CN_MCAST_NOTIFY in proc connector API, allows
a thread's non-zero exit status to be returned to proc_filter.
The threads.c program creates 2 child threads. 1st thread handles signal
SIGSEGV, and 2nd thread needs to indicate some error condition (value 1)
to the kernel, instead of using pthread_exit() with 1.
In both cases, child sends notify_netlink_thread_exit(exit_code) to kernel,
to let kernel know it has exited abnormally with exit_code.
Compile:
make thread
make proc_filter
To see non-zero exit notifications, run:
./proc_filter -f
Run threads code in another window:
./threads
Note the 2 child thread IDs reported above
Send SIGSEGV signal to the child handling SIGSEGV:
kill -11 <child1-tid>
Watch the child 1 tid being notified with exit code 11 to proc_filter
Watch child 2 tid being notified with exit code 1 (value defined in code)
to proc_filter
Signed-off-by: Anjali Kulkarni <anjali.k.kulkarni@oracle.com>
---
tools/testing/selftests/connector/Makefile | 23 ++++-
.../testing/selftests/connector/proc_filter.c | 5 +
tools/testing/selftests/connector/thread.c | 90 ++++++++++++++++++
.../selftests/connector/thread_filter.c | 93 +++++++++++++++++++
4 files changed, 210 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/connector/thread.c
create mode 100644 tools/testing/selftests/connector/thread_filter.c
diff --git a/tools/testing/selftests/connector/Makefile b/tools/testing/selftests/connector/Makefile
index 92188b9bac5c..bf335826bc3b 100644
--- a/tools/testing/selftests/connector/Makefile
+++ b/tools/testing/selftests/connector/Makefile
@@ -1,5 +1,26 @@
# SPDX-License-Identifier: GPL-2.0
-CFLAGS += -Wall $(KHDR_INCLUDES)
+KERNEL="../../../.."
+
+CFLAGS += -Wall $(KHDR_INCLUDES) -I $(KERNEL)/include/uapi -I $(KERNEL)/include
+
+proc_filter: proc_filter.o
+ cc proc_filter.o -o proc_filter
+
+proc_filter.o: proc_filter.c
+ cc -c proc_filter.c -o proc_filter.o $(CFLAGS)
+
+thread: thread.o thread_filter.o
+ cc thread.o thread_filter.o -o thread
+
+thread.o: thread.c $(DEPS)
+ cc -c thread.c -o thread.o $(CFLAGS)
+
+thread_filter.o: thread_filter.c
+ cc -c thread_filter.c -o thread_filter.o $(CFLAGS)
+
+define EXTRA_CLEAN
+ rm *.o thread
+endef
TEST_GEN_PROGS = proc_filter
diff --git a/tools/testing/selftests/connector/proc_filter.c b/tools/testing/selftests/connector/proc_filter.c
index 4a825b997666..6fb4842894f8 100644
--- a/tools/testing/selftests/connector/proc_filter.c
+++ b/tools/testing/selftests/connector/proc_filter.c
@@ -1,4 +1,9 @@
// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Author: Anjali Kulkarni <anjali.k.kulkarni@oracle.com>
+ *
+ * Copyright (c) 2024 Oracle and/or its affiliates.
+ */
#include <sys/types.h>
#include <sys/epoll.h>
diff --git a/tools/testing/selftests/connector/thread.c b/tools/testing/selftests/connector/thread.c
new file mode 100644
index 000000000000..77cba2b266dc
--- /dev/null
+++ b/tools/testing/selftests/connector/thread.c
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Author: Anjali Kulkarni <anjali.k.kulkarni@oracle.com>
+ *
+ * Copyright (c) 2024 Oracle and/or its affiliates.
+ */
+
+#include <pthread.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <signal.h>
+
+/*
+ * This code tests a thread exit notification when thread exits abnormally.
+ * Normally, when a thread exits abnormally, the kernel is not aware of the
+ * exit code. This is usually only conveyed from child to parent via the
+ * pthread_exit() and pthread_join() calls. Sometimes, however, a parent
+ * process cannot monitor all child processes via pthread_join(), particularly
+ * when there is a huge amount of child processes. In this case, the parent
+ * has created the child with PTHREAD_CREATE_DETACHED attribute.
+ * To fix this problem, either when child wants to convey non-zero exit via
+ * pthread_exit() or in a signal handler, the child can notify the kernel's
+ * connector module it's exit status via a netlink call with new type
+ * PROC_CN_MCAST_NOTIFY. (Implemented in the thread_filter.c file).
+ * This will send the exit code from the child to the kernel, which the kernel
+ * can later return to proc_filter program when the child actually exits.
+ * To test this usecase:
+ * Compile:
+ * make thread
+ * make proc_filter
+ * To see non-zero exit notifications, run:
+ * ./proc_filter -f
+ * Start the threads code, creating 2 threads, in another window:
+ * ./threads
+ * Note the 2 child thread IDs reported above
+ * Send SIGSEGV signal to the child handling SIGSEGV:
+ * kill -11 <child1-tid>
+ * Watch the event being notified with exit code 11 to proc_filter
+ * Watch child 2 tid being notified with exit code 1 (value defined in code)
+ * to proc_filter
+ */
+
+extern int notify_netlink_thread_exit(unsigned int exit_code);
+
+static void sigsegvh(int sig)
+{
+ unsigned int exit_code = (unsigned int) sig;
+ /*
+ * Send any non-zero value to get a notification. Here we are
+ * sending the signal number for SIGSEGV which is 11
+ */
+ notify_netlink_thread_exit(exit_code);
+}
+
+void *threadc1(void *ptr)
+{
+ signal(SIGSEGV, sigsegvh);
+ printf("Child 1 thread id %d, handling SIGSEGV\n", gettid());
+ sleep(30);
+ pthread_exit(NULL);
+}
+
+void *threadc2(void *ptr)
+{
+ int exit_val = 1;
+
+ printf("Child 2 thread id %d, wants to exit with value %d\n",
+ gettid(), exit_val);
+ sleep(2);
+ notify_netlink_thread_exit(exit_val);
+ pthread_exit(NULL);
+}
+
+int main(int argc, char **argv)
+{
+ pthread_t thread1, thread2;
+ pthread_attr_t attr1, attr2;
+
+ pthread_attr_init(&attr1);
+ pthread_attr_setdetachstate(&attr1, PTHREAD_CREATE_DETACHED);
+ pthread_create(&thread1, &attr1, *threadc1, NULL);
+
+ pthread_attr_init(&attr2);
+ pthread_attr_setdetachstate(&attr2, PTHREAD_CREATE_DETACHED);
+ pthread_create(&thread2, &attr2, *threadc2, NULL);
+
+ sleep(30);
+ exit(0);
+}
diff --git a/tools/testing/selftests/connector/thread_filter.c b/tools/testing/selftests/connector/thread_filter.c
new file mode 100644
index 000000000000..4b666004313b
--- /dev/null
+++ b/tools/testing/selftests/connector/thread_filter.c
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Author: Anjali Kulkarni <anjali.k.kulkarni@oracle.com>
+ *
+ * Copyright (c) 2024 Oracle and/or its affiliates.
+ */
+
+#include <sys/types.h>
+#include <sys/epoll.h>
+#include <sys/socket.h>
+#include <linux/netlink.h>
+#include <linux/connector.h>
+#include <linux/cn_proc.h>
+
+#include <stddef.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <strings.h>
+#include <errno.h>
+#include <signal.h>
+#include <string.h>
+
+#define NL_MESSAGE_SIZE (sizeof(struct nlmsghdr) + sizeof(struct cn_msg) + \
+ sizeof(struct proc_input))
+
+/*
+ * Send PROC_CN_MCAST_NOTIFY type notification to the connector code in kernel.
+ * This will send the exit_code specified by user to the connector layer, so
+ * it can send a notification for that event to any listening process
+ */
+int send_message(int nl_sock, unsigned int exit_code)
+{
+ char buff[NL_MESSAGE_SIZE];
+ struct nlmsghdr *hdr;
+ struct cn_msg *msg;
+
+ hdr = (struct nlmsghdr *)buff;
+ hdr->nlmsg_len = NL_MESSAGE_SIZE;
+ hdr->nlmsg_type = NLMSG_DONE;
+ hdr->nlmsg_flags = 0;
+ hdr->nlmsg_seq = 0;
+ hdr->nlmsg_pid = getpid();
+
+ msg = (struct cn_msg *)NLMSG_DATA(hdr);
+ msg->id.idx = CN_IDX_PROC;
+ msg->id.val = CN_VAL_PROC;
+ msg->seq = 0;
+ msg->ack = 0;
+ msg->flags = 0;
+
+ msg->len = sizeof(struct proc_input);
+ ((struct proc_input *)msg->data)->mcast_op =
+ PROC_CN_MCAST_NOTIFY;
+ ((struct proc_input *)msg->data)->uexit_code = exit_code;
+
+ if (send(nl_sock, hdr, hdr->nlmsg_len, 0) == -1) {
+ perror("send failed");
+ return -errno;
+ }
+ return 0;
+}
+
+int notify_netlink_thread_exit(unsigned int exit_code)
+{
+ struct sockaddr_nl sa_nl;
+ int err = 0;
+ int nl_sock;
+
+ nl_sock = socket(PF_NETLINK, SOCK_DGRAM, NETLINK_CONNECTOR);
+
+ if (nl_sock == -1) {
+ perror("socket failed");
+ return -errno;
+ }
+
+ bzero(&sa_nl, sizeof(sa_nl));
+ sa_nl.nl_family = AF_NETLINK;
+ sa_nl.nl_groups = CN_IDX_PROC;
+ sa_nl.nl_pid = gettid();
+
+ if (bind(nl_sock, (struct sockaddr *)&sa_nl, sizeof(sa_nl)) == -1) {
+ perror("bind failed");
+ return -errno;
+ }
+
+ err = send_message(nl_sock, exit_code);
+
+ if (err < 0)
+ return err;
+
+ return 0;
+}
--
2.46.0
^ permalink raw reply related [flat|nested] 7+ messages in thread