Fix/homa destroy ordering

.gitignore

@@ -2,6 +2,7 @@
 \#*#
 *.pyc
 *.o
+*.d
 *.hi
 *.dump
 *.log
@@ -22,4 +23,4 @@
 reports/
 traces/
 users/
-saved_traces/
+saved_traces/

homa_plumbing.c

@@ -526,7 +526,10 @@ int __init homa_load(void)
 #endif /* See strip.py */
 
 #ifndef __UPSTREAM__ /* See strip.py */
-	tt_init("timetrace");
+	status = tt_init("timetrace");
+	/* tt_init cleans up its own partial initialization on failure. */
+	if (status)
+		return status;
 #endif /* See strip.py */
 
 	status = homa_init(homa);
@@ -644,8 +647,6 @@ int __init homa_load(void)
 	if (init_metrics)
 		homa_metrics_end();
 #endif /* See strip.py */
-	if (init_homa)
-		homa_destroy(homa);
 	if (init_protocol)
 		inet_del_protocol(&homa_protocol, IPPROTO_HOMA);
 	if (init_protocol6)
@@ -660,6 +661,13 @@ int __init homa_load(void)
 		proto_unregister(&homav6_prot);
 	if (init_net_ops)
 		unregister_pernet_subsys(&homa_net_ops);
+	/* Namespace cleanup still needs homa's socket and peer tables. */
+	if (init_homa)
+		homa_destroy(homa);
+#ifndef __UPSTREAM__ /* See strip.py */
+	/* Remove proc callbacks before a failed load releases module memory. */
+	tt_destroy();
+#endif /* See strip.py */
 	return status;
 }
 

homa_utils.c

@@ -122,6 +122,8 @@ int homa_init(struct homa *homa)
  */
 void homa_destroy(struct homa *homa)
 {
+	UNIT_LOG("; ", "homa_destroy");
+
 	/* The order of the following cleanups matters! */
 	if (homa->socktab) {
 		homa_socktab_destroy(homa->socktab, NULL);

test/mock.c

@@ -675,6 +675,7 @@ int inet6_del_offload(const struct net_offload *prot, unsigned char protocol)
 
 int inet6_del_protocol(const struct inet6_protocol *prot, unsigned char num)
 {
+	UNIT_LOG("; ", "inet6_del_protocol %u", num);
 	return 0;
 }
 
@@ -700,7 +701,10 @@ int inet6_release(struct socket *sock)
 	return 0;
 }
 
-void inet6_unregister_protosw(struct inet_protosw *p) {}
+void inet6_unregister_protosw(struct inet_protosw *p)
+{
+	UNIT_LOG("; ", "inet6_unregister_protosw %u", p->protocol);
+}
 
 int inet_add_offload(const struct net_offload *prot, unsigned char protocol)
 {
@@ -719,6 +723,7 @@ int inet_del_offload(const struct net_offload *prot, unsigned char protocol)
 
 int inet_del_protocol(const struct net_protocol *prot, unsigned char num)
 {
+	UNIT_LOG("; ", "inet_del_protocol %u", num);
 	return 0;
 }
 
@@ -758,7 +763,9 @@ int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 }
 
 void inet_unregister_protosw(struct inet_protosw *p)
-{}
+{
+	UNIT_LOG("; ", "inet_unregister_protosw %u", p->protocol);
+}
 
 void __init_swait_queue_head(struct swait_queue_head *q, const char *name,
 		struct lock_class_key *key)
@@ -1328,7 +1335,10 @@ int proto_register(struct proto *prot, int alloc_slab)
 	return 0;
 }
 
-void proto_unregister(struct proto *prot) {}
+void proto_unregister(struct proto *prot)
+{
+	UNIT_LOG("; ", "proto_unregister %s", prot->name);
+}
 
 void *__pskb_pull_tail(struct sk_buff *skb, int delta)
 {
@@ -1767,7 +1777,9 @@ void unregister_net_sysctl_table(struct ctl_table_header *header)
 }
 
 void unregister_pernet_subsys(struct pernet_operations *)
-{}
+{
+	UNIT_LOG("; ", "unregister_pernet_subsys");
+}
 
 void unregister_qdisc(struct Qdisc_ops *qops)
 {

test/unit_homa_plumbing.c

@@ -134,6 +134,30 @@ static void create_rpcs_hook(char *id)
 	saved_self = NULL;
 }
 
+#ifndef __UPSTREAM__ /* See strip.py */
+TEST_F(homa_plumbing, homa_load__error_in_tt_init)
+{
+	int result;
+
+	homa_destroy(&self->homa);
+
+	/* Fail the first timetrace-buffer allocation. homa_load must
+	 * propagate tt_init's error instead of continuing initialization.
+	 */
+	mock_kmalloc_errors = 1;
+	result = homa_load();
+	EXPECT_EQ(-1, result);
+
+	/* tt_init cleans up its own partial initialization on failure. */
+	for (int i = 0; i < nr_cpu_ids; i++)
+		EXPECT_EQ(NULL, tt_buffers[i]);
+
+	/* Clean up if a regression allowed loading to succeed. */
+	if (result == 0)
+		homa_unload();
+}
+#endif /* See strip.py */
+
 TEST_F(homa_plumbing, homa_load__error_in_inet6_register_protosw)
 {
 	homa_destroy(&self->homa);
@@ -142,12 +166,71 @@ TEST_F(homa_plumbing, homa_load__error_in_inet6_register_protosw)
 	mock_register_protosw_errors = 1;
 	EXPECT_EQ(EINVAL, -homa_load());
 
+	/* Failed loading must release every timetrace buffer before
+	 * another load is attempted. A successful retry followed by
+	 * unload could otherwise hide missing failure-path cleanup.
+	 */
+	for (int i = 0; i < nr_cpu_ids; i++)
+		EXPECT_EQ(NULL, tt_buffers[i]);
+
 	/* Second attempt succeeds. */
-	EXPECT_EQ(0, -homa_load());
+	ASSERT_EQ(0, homa_load());
 
 	homa_unload();
 }
 
+TEST_F(homa_plumbing, homa_load__destroy_after_unregister)
+{
+	const char *events[] = {
+		"inet_del_protocol ",
+		"inet6_del_protocol ",
+		"inet_unregister_protosw ",
+		"inet6_unregister_protosw ",
+		"proto_unregister HOMA;",
+		"proto_unregister HOMAv6;",
+		"unregister_pernet_subsys;"
+	};
+	const char *log;
+	const char *destroy;
+	int result;
+
+	homa_destroy(&self->homa);
+
+	/* Exclude destruction of the fixture's Homa instance. */
+	unit_log_clear();
+
+	/* Fail after all registrations have succeeded, so loading must
+	 * unwind them before destroying the shared Homa state.
+	 */
+	mock_kthread_create_errors = 1;
+	result = homa_load();
+	EXPECT_EQ(-EACCES, result);
+
+	log = unit_log_get();
+	destroy = strstr(log, "homa_destroy");
+	EXPECT_NE(NULL, destroy);
+
+	/* Require each unregistration to precede destruction, without
+	 * constraining the order of the unregistrations themselves.
+	 */
+	for (int i = 0; i < ARRAY_SIZE(events); i++) {
+		const char *event = strstr(log, events[i]);
+
+		EXPECT_NE(NULL, event);
+		if (event && destroy)
+			EXPECT_TRUE(event < destroy);
+	}
+
+	/* The global Homa instance must be destroyed exactly once. */
+	if (destroy)
+		EXPECT_EQ(NULL, strstr(destroy + strlen("homa_destroy"),
+				       "homa_destroy"));
+
+	/* Clean up if a regression allowed loading to succeed. */
+	if (result == 0)
+		homa_unload();
+}
+
 TEST_F(homa_plumbing, homa_net_exit__free_peers)
 {
 	struct in6_addr addr1 = unit_get_in_addr("1.2.3.4");

test/unit_homa_rpc.c

@@ -328,7 +328,6 @@ TEST_F(homa_rpc, homa_rpc_ack__multiple_acks)
 TEST_F(homa_rpc, homa_rpc_ack__release_rpc_lock)
 {
 	struct homa_rpc *srpc;
-	struct homa_sock hsk;
 
 	srpc = unit_server_rpc(&self->hsk, UNIT_OUTGOING, self->client_ip,
 			self->server_ip, self->client_port, self->server_id,
@@ -345,8 +344,6 @@ TEST_F(homa_rpc, homa_rpc_ack__release_rpc_lock)
 	homa_rpc_ack(&self->hsk, NULL, self->client_ip, 0, NULL);
 	EXPECT_EQ(1, mock_total_spin_locks);
 	homa_rpc_unlock(srpc);
-
-	unit_sock_destroy(&hsk);
 }
 TEST_F(homa_rpc, homa_rpc_ack__lookup_socket)
 {