[Devel] [PATCH VZ10 v2 1/4] connector: annotate ve->cn with __rcu

[Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>]

The following patches will clear ve->cn on container stop and free the
per-VE connector state only after an RCU grace period, so that the
proc event delivery path can use it under rcu_read_lock() even when
the reported task no longer pins the VE.

Prepare for that: annotate ve->cn with __rcu and switch all accesses
to the RCU accessors so sparse can verify the protocol. All current
users run either in the context of a task alive in the VE or from
the VE start/stop hooks under ve->op_sem (or on ve0), so plain readers
use rcu_dereference_check(ve->cn, 1) with a comment and the start/stop
hooks use rcu_dereference_protected() with the proper lockdep
condition.

No functional change intended

https://virtuozzo.atlassian.net/browse/VSTOR-140421
Signed-off-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>

Feature: ve: ve generic structures
---
 drivers/connector/cn_proc.c   | 48 +++++++++++++++++++++++++++++++------------
 drivers/connector/connector.c | 43 +++++++++++++++++++++++++-------------
 include/linux/ve.h            |  2 +-
 3 files changed, 65 insertions(+), 28 deletions(-)

diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index d41c8aca1866..d4ce1697dd0b 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -84,6 +84,8 @@ static int cn_filter(struct sock *dsk, struct sk_buff *skb, void *data)
 
 static inline void send_msg_ve(struct ve_struct *ve, struct cn_msg *msg)
 {
+	/* See the comment in proc_event_num_listeners() */
+	struct cn_private *cn = rcu_dereference_check(ve->cn, 1);
 	struct local_event *le_ptr;
 	__u32 filter_data[2];
 
@@ -93,9 +95,9 @@ static inline void send_msg_ve(struct ve_struct *ve, struct cn_msg *msg)
 	 * so be of the safe side.
 	 */
 	BUILD_BUG_ON(offsetof(struct local_event, lock) != 0);
-	local_lock(&ve->cn->local_event->lock);
+	local_lock(&cn->local_event->lock);
 
-	le_ptr = this_cpu_ptr(ve->cn->local_event);
+	le_ptr = this_cpu_ptr(cn->local_event);
 	msg->seq = le_ptr->count++;
 	((struct proc_event *)msg->data)->cpu = smp_processor_id();
 
@@ -116,7 +118,7 @@ static inline void send_msg_ve(struct ve_struct *ve, struct cn_msg *msg)
 	cn_netlink_send_mult_ve(ve, msg, msg->len, 0, CN_IDX_PROC, GFP_NOWAIT,
 				cn_filter, (void *)filter_data);
 
-	local_unlock(&ve->cn->local_event->lock);
+	local_unlock(&cn->local_event->lock);
 }
 
 static struct cn_msg *cn_msg_fill(__u8 *buffer, struct ve_struct *ve,
@@ -147,8 +149,15 @@ static struct cn_msg *cn_msg_fill(__u8 *buffer, struct ve_struct *ve,
 
 static int proc_event_num_listeners(struct ve_struct *ve)
 {
-	if (ve->cn)
-		return atomic_read(&ve->cn->proc_event_num_listeners);
+	/*
+	 * Callers not under rcu_read_lock() are pinned by a live task
+	 * of this VE (or run on ve0 whose connector state lives as long
+	 * as the connector itself).
+	 */
+	struct cn_private *cn = rcu_dereference_check(ve->cn, 1);
+
+	if (cn)
+		return atomic_read(&cn->proc_event_num_listeners);
 	return 0;
 }
 
@@ -414,6 +423,8 @@ static void cn_proc_mcast_ctl(struct cn_msg *msg,
 	enum proc_cn_event ev_type = 0;
 	int err = 0, initial = 0;
 	struct sock *sk = NULL;
+	/* current is a live task of this VE, it cannot be stopped under us */
+	struct cn_private *cn = rcu_dereference_check(ve->cn, 1);
 
 	/* 
 	 * Events are reported with respect to the initial pid
@@ -467,11 +478,11 @@ static void cn_proc_mcast_ctl(struct cn_msg *msg,
 	switch (mc_op) {
 	case PROC_CN_MCAST_LISTEN:
 		if (initial || (prev_mc_op != PROC_CN_MCAST_LISTEN))
-			atomic_inc(&ve->cn->proc_event_num_listeners);
+			atomic_inc(&cn->proc_event_num_listeners);
 		break;
 	case PROC_CN_MCAST_IGNORE:
 		if (!initial && (prev_mc_op != PROC_CN_MCAST_IGNORE))
-			atomic_dec(&ve->cn->proc_event_num_listeners);
+			atomic_dec(&cn->proc_event_num_listeners);
 		((struct proc_input *)(sk->sk_user_data))->event_type =
 			PROC_EVENT_NONE;
 		break;
@@ -488,13 +499,18 @@ int cn_proc_init_ve(struct ve_struct *ve)
 {
 	int err, cpu;
 	struct local_event *le_ptr;
+	struct cn_private *cn;
 
-	ve->cn->local_event = alloc_percpu(struct local_event);
-	if (!ve->cn->local_event)
+	cn = rcu_dereference_protected(ve->cn,
+				       lockdep_is_held(&ve->op_sem) ||
+				       ve_is_super(ve));
+
+	cn->local_event = alloc_percpu(struct local_event);
+	if (!cn->local_event)
 		return -ENOMEM;
 
 	for_each_possible_cpu(cpu) {
-		le_ptr = per_cpu_ptr(ve->cn->local_event, cpu);
+		le_ptr = per_cpu_ptr(cn->local_event, cpu);
 		local_lock_init(&le_ptr->lock);
 	}
 
@@ -503,15 +519,21 @@ int cn_proc_init_ve(struct ve_struct *ve)
 				  &cn_proc_mcast_ctl);
 	if (err) {
 		pr_warn("VE#%d: cn_proc failed to register\n", ve->veid);
-		free_percpu(ve->cn->local_event);
+		free_percpu(cn->local_event);
 		return err;
 	}
-	atomic_set(&ve->cn->proc_event_num_listeners, 0);
+	atomic_set(&cn->proc_event_num_listeners, 0);
 	return 0;
 }
 
 void cn_proc_fini_ve(struct ve_struct *ve)
 {
+	struct cn_private *cn;
+
+	cn = rcu_dereference_protected(ve->cn,
+				       lockdep_is_held(&ve->op_sem) ||
+				       ve_is_super(ve));
+
 	cn_del_callback_ve(ve, &cn_proc_event_id);
-	free_percpu(ve->cn->local_event);
+	free_percpu(cn->local_event);
 }
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index 39a697803d0f..bf4a83f3f870 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -29,7 +29,13 @@ MODULE_ALIAS_NET_PF_PROTO(PF_NETLINK, NETLINK_CONNECTOR);
 
 static struct cn_dev *get_cdev(struct ve_struct *ve)
 {
-	return &ve->cn->cdev;
+	/*
+	 * Callers not under rcu_read_lock() are pinned by a live task
+	 * of this VE or run from the VE start/stop hooks.
+	 */
+	struct cn_private *cn = rcu_dereference_check(ve->cn, 1);
+
+	return cn ? &cn->cdev : NULL;
 }
 
 /*
@@ -230,12 +236,13 @@ int cn_add_callback_ve(struct ve_struct *ve,
 		       void (*callback)(struct cn_msg *,
 					struct netlink_skb_parms *))
 {
-	struct cn_dev *dev = get_cdev(ve);
+	/* See the comment in get_cdev() */
+	struct cn_private *cn = rcu_dereference_check(ve->cn, 1);
 
-	if (!ve->cn->cn_already_initialized)
+	if (!cn || !cn->cn_already_initialized)
 		return -EAGAIN;
 
-	return cn_queue_add_callback(dev->cbdev, name, id, callback);
+	return cn_queue_add_callback(cn->cdev.cbdev, name, id, callback);
 }
 
 /*
@@ -297,6 +304,7 @@ static int __maybe_unused cn_proc_show(struct seq_file *m, void *v)
 static int cn_init_ve(void *data)
 {
 	struct ve_struct *ve = data;
+	struct cn_private *cn;
 	struct cn_dev *dev;
 	struct netlink_kernel_cfg cfg = {
 		.groups	= CN_NETLINK_USERS + 0xf,
@@ -308,11 +316,12 @@ static int cn_init_ve(void *data)
 	struct net *net;
 	int err;
 
-	ve->cn = kzalloc(sizeof(*ve->cn), GFP_KERNEL);
-	if (!ve->cn)
+	cn = kzalloc(sizeof(*cn), GFP_KERNEL);
+	if (!cn)
 		return -ENOMEM;
+	rcu_assign_pointer(ve->cn, cn);
 
-	dev = &ve->cn->cdev;
+	dev = &cn->cdev;
 
 	/*
 	 * This is a hook, hooks are called under a single lock, so ve_nsproxy will
@@ -331,7 +340,7 @@ static int cn_init_ve(void *data)
 		goto netlink_release;
 	}
 
-	ve->cn->cn_already_initialized = 1;
+	cn->cn_already_initialized = 1;
 
 	if (!proc_ve_create_single("connector", S_IRUGO, net->proc_net,
 				cn_proc_show)) {
@@ -353,18 +362,24 @@ static int cn_init_ve(void *data)
 netlink_release:
 	netlink_kernel_release(dev->nls);
 free_cn:
-	kfree(ve->cn);
-	ve->cn = NULL;
+	RCU_INIT_POINTER(ve->cn, NULL);
+	kfree(cn);
 	goto net_unlock;
 }
 
 static void cn_fini_ve(void *data)
 {
 	struct ve_struct *ve = data;
-	struct cn_dev *dev = get_cdev(ve);
+	struct cn_private *cn;
+	struct cn_dev *dev;
 	struct net *net;
 
-	ve->cn->cn_already_initialized = 0;
+	cn = rcu_dereference_protected(ve->cn,
+				       lockdep_is_held(&ve->op_sem) ||
+				       ve_is_super(ve));
+	dev = &cn->cdev;
+
+	cn->cn_already_initialized = 0;
 
 	cn_proc_fini_ve(ve);
 
@@ -379,8 +394,8 @@ static void cn_fini_ve(void *data)
 	cn_queue_free_dev(dev->cbdev);
 	netlink_kernel_release(dev->nls);
 
-	kfree(ve->cn);
-	ve->cn = NULL;
+	RCU_INIT_POINTER(ve->cn, NULL);
+	kfree(cn);
 }
 
 #ifdef CONFIG_VE
diff --git a/include/linux/ve.h b/include/linux/ve.h
index cba827260d07..3334bd1e9517 100644
--- a/include/linux/ve.h
+++ b/include/linux/ve.h
@@ -94,7 +94,7 @@ struct ve_struct {
 	char			core_pattern[CORENAME_MAX_SIZE];
 #endif
 #ifdef CONFIG_CONNECTOR
-	struct cn_private	*cn;
+	struct cn_private __rcu	*cn;
 #endif
 
 	struct kthread_worker	*kthreadd_worker;

-- 
2.43.0