Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/sage/ceph-client

Pull Ceph fixes from Sage Weil: "Two of Alex's patches deal with a race when reseting server connections for open RBD images, one demotes some non-fatal BUGs to WARNs, and my patch fixes a protocol feature bit failure path." * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/sage/ceph-client: libceph: fix protocol feature mismatch failure path libceph: WARN, don't BUG on unexpected connection states libceph: always reset osds when kicking libceph: move linger requests sooner in kick_requests()

Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/sage/ceph-client
Pull Ceph fixes from Sage Weil: "Two of Alex's patches deal with a race when reseting server connections for open RBD images, one demotes some non-fatal BUGs to WARNs, and my patch fixes a protocol feature bit failure path." * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/sage/ceph-client: libceph: fix protocol feature mismatch failure path libceph: WARN, don't BUG on unexpected connection states libceph: always reset osds when kicking libceph: move linger requests sooner in kick_requests()
Linus Torvalds
2 parents 42288fe366 0fa6ebc600
Showing 2 changed files Side-by-side Diff
net/ceph/messenger.c
net/ceph/osd_client.c
@@ -506,6 +506,7 @@
 {
 	/* reset connection, out_queue, msg_ and connect_seq */
 	/* discard existing out_queue and msg_seq */
+	dout("reset_connection %p\n", con);
 	ceph_msg_remove_list(&con->out_queue);
 	ceph_msg_remove_list(&con->out_sent);
  
@@ -561,7 +562,7 @@
 	mutex_lock(&con->mutex);
 	dout("con_open %p %s\n", con, ceph_pr_addr(&addr->in_addr));
  
-	BUG_ON(con->state != CON_STATE_CLOSED);
+	WARN_ON(con->state != CON_STATE_CLOSED);
 	con->state = CON_STATE_PREOPEN;
  
 	con->peer_name.type = (__u8) entity_type;
@@ -1506,13 +1507,6 @@
 	return 0;
 }
  
-static void fail_protocol(struct ceph_connection *con)
-{
-	reset_connection(con);
-	BUG_ON(con->state != CON_STATE_NEGOTIATING);
-	con->state = CON_STATE_CLOSED;
-}
-
 static int process_connect(struct ceph_connection *con)
 {
 	u64 sup_feat = con->msgr->supported_features;
@@ -1530,7 +1524,7 @@
 		       ceph_pr_addr(&con->peer_addr.in_addr),
 		       sup_feat, server_feat, server_feat & ~sup_feat);
 		con->error_msg = "missing required protocol features";
-		fail_protocol(con);
+		reset_connection(con);
 		return -1;
  
 	case CEPH_MSGR_TAG_BADPROTOVER:
@@ -1541,7 +1535,7 @@
 		       le32_to_cpu(con->out_connect.protocol_version),
 		       le32_to_cpu(con->in_reply.protocol_version));
 		con->error_msg = "protocol version mismatch";
-		fail_protocol(con);
+		reset_connection(con);
 		return -1;
  
 	case CEPH_MSGR_TAG_BADAUTHORIZER:
  
@@ -1631,11 +1625,11 @@
 			       ceph_pr_addr(&con->peer_addr.in_addr),
 			       req_feat, server_feat, req_feat & ~server_feat);
 			con->error_msg = "missing required protocol features";
-			fail_protocol(con);
+			reset_connection(con);
 			return -1;
 		}
  
-		BUG_ON(con->state != CON_STATE_NEGOTIATING);
+		WARN_ON(con->state != CON_STATE_NEGOTIATING);
 		con->state = CON_STATE_OPEN;
  
 		con->peer_global_seq = le32_to_cpu(con->in_reply.global_seq);
@@ -2132,7 +2126,6 @@
 		if (ret < 0)
 			goto out;
  
-		BUG_ON(con->state != CON_STATE_CONNECTING);
 		con->state = CON_STATE_NEGOTIATING;
  
 		/*
@@ -2160,7 +2153,7 @@
 		goto more;
 	}
  
-	BUG_ON(con->state != CON_STATE_OPEN);
+	WARN_ON(con->state != CON_STATE_OPEN);
  
 	if (con->in_base_pos < 0) {
 		/*
@@ -2382,7 +2375,7 @@
 	dout("fault %p state %lu to peer %s\n",
 	     con, con->state, ceph_pr_addr(&con->peer_addr.in_addr));
  
-	BUG_ON(con->state != CON_STATE_CONNECTING &&
+	WARN_ON(con->state != CON_STATE_CONNECTING &&
 	       con->state != CON_STATE_NEGOTIATING &&
 	       con->state != CON_STATE_OPEN);
  
@@ -1270,7 +1270,7 @@
  * Requeue requests whose mapping to an OSD has changed.  If requests map to
  * no osd, request a new map.
  *
- * Caller should hold map_sem for read and request_mutex.
+ * Caller should hold map_sem for read.
  */
 static void kick_requests(struct ceph_osd_client *osdc, int force_resend)
 {
@@ -1284,6 +1284,24 @@
 	for (p = rb_first(&osdc->requests); p; ) {
 		req = rb_entry(p, struct ceph_osd_request, r_node);
 		p = rb_next(p);
+
+		/*
+		 * For linger requests that have not yet been
+		 * registered, move them to the linger list; they'll
+		 * be sent to the osd in the loop below.  Unregister
+		 * the request before re-registering it as a linger
+		 * request to ensure the __map_request() below
+		 * will decide it needs to be sent.
+		 */
+		if (req->r_linger && list_empty(&req->r_linger_item)) {
+			dout("%p tid %llu restart on osd%d\n",
+			     req, req->r_tid,
+			     req->r_osd ? req->r_osd->o_osd : -1);
+			__unregister_request(osdc, req);
+			__register_linger_request(osdc, req);
+			continue;
+		}
+
 		err = __map_request(osdc, req, force_resend);
 		if (err < 0)
 			continue;  /* error */
@@ -1298,17 +1316,6 @@
 				req->r_flags |= CEPH_OSD_FLAG_RETRY;
 			}
 		}
-		if (req->r_linger && list_empty(&req->r_linger_item)) {
-			/*
-			 * register as a linger so that we will
-			 * re-submit below and get a new tid
-			 */
-			dout("%p tid %llu restart on osd%d\n",
-			     req, req->r_tid,
-			     req->r_osd ? req->r_osd->o_osd : -1);
-			__register_linger_request(osdc, req);
-			__unregister_request(osdc, req);
-		}
 	}
  
 	list_for_each_entry_safe(req, nreq, &osdc->req_linger,
@@ -1316,6 +1323,7 @@
 		dout("linger req=%p req->r_osd=%p\n", req, req->r_osd);
  
 		err = __map_request(osdc, req, force_resend);
+		dout("__map_request returned %d\n", err);
 		if (err == 0)
 			continue;  /* no change and no osd was specified */
 		if (err < 0)
@@ -1337,6 +1345,7 @@
 		dout("%d requests for down osds, need new map\n", needmap);
 		ceph_monc_request_next_osdmap(&osdc->client->monc);
 	}
+	reset_changed_osds(osdc);
 }
  
  
@@ -1393,7 +1402,6 @@
 				osdc->osdmap = newmap;
 			}
 			kick_requests(osdc, 0);
-			reset_changed_osds(osdc);
 		} else {
 			dout("ignoring incremental map %u len %d\n",
 			     epoch, maplen);
...	...	@@ -506,6 +506,7 @@
506	506	{
507	507	/* reset connection, out_queue, msg_ and connect_seq */
508	508	/* discard existing out_queue and msg_seq */
	509	+ dout("reset_connection %p\n", con);
509	510	ceph_msg_remove_list(&con->out_queue);
510	511	ceph_msg_remove_list(&con->out_sent);
511	512
...	...	@@ -561,7 +562,7 @@
561	562	mutex_lock(&con->mutex);
562	563	dout("con_open %p %s\n", con, ceph_pr_addr(&addr->in_addr));
563	564
564		- BUG_ON(con->state != CON_STATE_CLOSED);
	565	+ WARN_ON(con->state != CON_STATE_CLOSED);
565	566	con->state = CON_STATE_PREOPEN;
566	567
567	568	con->peer_name.type = (__u8) entity_type;
...	...	@@ -1506,13 +1507,6 @@
1506	1507	return 0;
1507	1508	}
1508	1509
1509		-static void fail_protocol(struct ceph_connection *con)
1510		-{
1511		- reset_connection(con);
1512		- BUG_ON(con->state != CON_STATE_NEGOTIATING);
1513		- con->state = CON_STATE_CLOSED;
1514		-}
1515		-
1516	1510	static int process_connect(struct ceph_connection *con)
1517	1511	{
1518	1512	u64 sup_feat = con->msgr->supported_features;
...	...	@@ -1530,7 +1524,7 @@
1530	1524	ceph_pr_addr(&con->peer_addr.in_addr),
1531	1525	sup_feat, server_feat, server_feat & ~sup_feat);
1532	1526	con->error_msg = "missing required protocol features";
1533		- fail_protocol(con);
	1527	+ reset_connection(con);
1534	1528	return -1;
1535	1529
1536	1530	case CEPH_MSGR_TAG_BADPROTOVER:
...	...	@@ -1541,7 +1535,7 @@
1541	1535	le32_to_cpu(con->out_connect.protocol_version),
1542	1536	le32_to_cpu(con->in_reply.protocol_version));
1543	1537	con->error_msg = "protocol version mismatch";
1544		- fail_protocol(con);
	1538	+ reset_connection(con);
1545	1539	return -1;
1546	1540
1547	1541	case CEPH_MSGR_TAG_BADAUTHORIZER:
1548	1542
...	...	@@ -1631,11 +1625,11 @@
1631	1625	ceph_pr_addr(&con->peer_addr.in_addr),
1632	1626	req_feat, server_feat, req_feat & ~server_feat);
1633	1627	con->error_msg = "missing required protocol features";
1634		- fail_protocol(con);
	1628	+ reset_connection(con);
1635	1629	return -1;
1636	1630	}
1637	1631
1638		- BUG_ON(con->state != CON_STATE_NEGOTIATING);
	1632	+ WARN_ON(con->state != CON_STATE_NEGOTIATING);
1639	1633	con->state = CON_STATE_OPEN;
1640	1634
1641	1635	con->peer_global_seq = le32_to_cpu(con->in_reply.global_seq);
...	...	@@ -2132,7 +2126,6 @@
2132	2126	if (ret < 0)
2133	2127	goto out;
2134	2128
2135		- BUG_ON(con->state != CON_STATE_CONNECTING);
2136	2129	con->state = CON_STATE_NEGOTIATING;
2137	2130
2138	2131	/*
...	...	@@ -2160,7 +2153,7 @@
2160	2153	goto more;
2161	2154	}
2162	2155
2163		- BUG_ON(con->state != CON_STATE_OPEN);
	2156	+ WARN_ON(con->state != CON_STATE_OPEN);
2164	2157
2165	2158	if (con->in_base_pos < 0) {
2166	2159	/*
...	...	@@ -2382,7 +2375,7 @@
2382	2375	dout("fault %p state %lu to peer %s\n",
2383	2376	con, con->state, ceph_pr_addr(&con->peer_addr.in_addr));
2384	2377
2385		- BUG_ON(con->state != CON_STATE_CONNECTING &&
	2378	+ WARN_ON(con->state != CON_STATE_CONNECTING &&
2386	2379	con->state != CON_STATE_NEGOTIATING &&
2387	2380	con->state != CON_STATE_OPEN);
2388	2381
...	...	@@ -1270,7 +1270,7 @@
1270	1270	* Requeue requests whose mapping to an OSD has changed. If requests map to
1271	1271	* no osd, request a new map.
1272	1272	*
1273		- * Caller should hold map_sem for read and request_mutex.
	1273	+ * Caller should hold map_sem for read.
1274	1274	*/
1275	1275	static void kick_requests(struct ceph_osd_client *osdc, int force_resend)
1276	1276	{
...	...	@@ -1284,6 +1284,24 @@
1284	1284	for (p = rb_first(&osdc->requests); p; ) {
1285	1285	req = rb_entry(p, struct ceph_osd_request, r_node);
1286	1286	p = rb_next(p);
	1287	+
	1288	+ /*
	1289	+ * For linger requests that have not yet been
	1290	+ * registered, move them to the linger list; they'll
	1291	+ * be sent to the osd in the loop below. Unregister
	1292	+ * the request before re-registering it as a linger
	1293	+ * request to ensure the __map_request() below
	1294	+ * will decide it needs to be sent.
	1295	+ */
	1296	+ if (req->r_linger && list_empty(&req->r_linger_item)) {
	1297	+ dout("%p tid %llu restart on osd%d\n",
	1298	+ req, req->r_tid,
	1299	+ req->r_osd ? req->r_osd->o_osd : -1);
	1300	+ __unregister_request(osdc, req);
	1301	+ __register_linger_request(osdc, req);
	1302	+ continue;
	1303	+ }
	1304	+
1287	1305	err = __map_request(osdc, req, force_resend);
1288	1306	if (err < 0)
1289	1307	continue; /* error */
...	...	@@ -1298,17 +1316,6 @@
1298	1316	req->r_flags \|= CEPH_OSD_FLAG_RETRY;
1299	1317	}
1300	1318	}
1301		- if (req->r_linger && list_empty(&req->r_linger_item)) {
1302		- /*
1303		- * register as a linger so that we will
1304		- * re-submit below and get a new tid
1305		- */
1306		- dout("%p tid %llu restart on osd%d\n",
1307		- req, req->r_tid,
1308		- req->r_osd ? req->r_osd->o_osd : -1);
1309		- __register_linger_request(osdc, req);
1310		- __unregister_request(osdc, req);
1311		- }
1312	1319	}
1313	1320
1314	1321	list_for_each_entry_safe(req, nreq, &osdc->req_linger,
...	...	@@ -1316,6 +1323,7 @@
1316	1323	dout("linger req=%p req->r_osd=%p\n", req, req->r_osd);
1317	1324
1318	1325	err = __map_request(osdc, req, force_resend);
	1326	+ dout("__map_request returned %d\n", err);
1319	1327	if (err == 0)
1320	1328	continue; /* no change and no osd was specified */
1321	1329	if (err < 0)
...	...	@@ -1337,6 +1345,7 @@
1337	1345	dout("%d requests for down osds, need new map\n", needmap);
1338	1346	ceph_monc_request_next_osdmap(&osdc->client->monc);
1339	1347	}
	1348	+ reset_changed_osds(osdc);
1340	1349	}
1341	1350
1342	1351
...	...	@@ -1393,7 +1402,6 @@
1393	1402	osdc->osdmap = newmap;
1394	1403	}
1395	1404	kick_requests(osdc, 0);
1396		- reset_changed_osds(osdc);
1397	1405	} else {
1398	1406	dout("ignoring incremental map %u len %d\n",
1399	1407	epoch, maplen);