[sheepdog] [PATCH V3 1/2] sheep: simplify client_decref() and move it into free_request() and add a helper function

[Yunkai Zhang]

From: Yunkai Zhang <qiushu.zyk at taobao.com>

1) In previous code, sheep calls client_incref() in alloc_request(), but
free_request() does not call client_desref() in it, as a result it's difficult
to keep ci->refcnt with correct value. Now I drop client_incref/client_decref
and call ci->refcnt++/ci->refcnt-- in alloc_request/free_request directly.

2) A bug in put_request(): before calling client_decref(), we should do some
clear actions like client_handler().

3) ci->refcnt should only be increased by alloc_request(), let's initialize
   it with 0 in create_client().

4) remove error message in unregister_event() when lookup_event() failed,
   as this function may be called several times in new helper function which
   named clear_client() before ci->refcnt reach zero.

Signed-off-by: Yunkai Zhang <qiushu.zyk at taobao.com>
---
 lib/event.c   |    4 +---
 sheep/sdnet.c |   39 +++++++++++++++++++--------------------
 2 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/lib/event.c b/lib/event.c
index 8a69dc4..1557493 100644
--- a/lib/event.c
+++ b/lib/event.c
@@ -125,10 +125,8 @@ void unregister_event(int fd)
 	struct event_info *ei;
 
 	ei = lookup_event(fd);
-	if (!ei) {
-		eprintf("event info for fd %d not found\n", fd);
+	if (!ei)
 		return;
-	}
 
 	ret = epoll_ctl(efd, EPOLL_CTL_DEL, fd, NULL);
 	if (ret)
diff --git a/sheep/sdnet.c b/sheep/sdnet.c
index c13cdb0..bde2526 100644
--- a/sheep/sdnet.c
+++ b/sheep/sdnet.c
@@ -368,8 +368,7 @@ static void requeue_request(struct request *req)
 	queue_request(req);
 }
 
-static void client_incref(struct client_info *ci);
-static void client_decref(struct client_info *ci);
+static void clear_client(struct client_info *ci);
 
 static struct request *alloc_local_request(void *data, int data_length)
 {
@@ -429,7 +428,7 @@ static struct request *alloc_request(struct client_info *ci, int data_length)
 		return NULL;
 
 	req->ci = ci;
-	client_incref(ci);
+	ci->refcnt++;
 	if (data_length) {
 		req->data_length = data_length;
 		req->data = valloc(data_length);
@@ -453,6 +452,7 @@ static void free_request(struct request *req)
 	sys->nr_outstanding_reqs--;
 	sys->outstanding_data_size -= req->data_length;
 
+	req->ci->refcnt--;
 	put_vnode_info(req->vnodes);
 	free(req->data);
 	free(req);
@@ -473,11 +473,10 @@ void put_request(struct request *req)
 		if (conn_tx_on(&ci->conn)) {
 			dprintf("connection seems to be dead\n");
 			free_request(req);
+			clear_client(ci);
 		} else {
 			list_add(&req->request_list, &ci->done_reqs);
 		}
-
-		client_decref(ci);
 	}
 }
 
@@ -653,16 +652,21 @@ static void destroy_client(struct client_info *ci)
 	free(ci);
 }
 
-static void client_incref(struct client_info *ci)
+static void clear_client(struct client_info *ci)
 {
-	if (ci)
-		ci->refcnt++;
-}
+	if (!list_empty(&ci->conn.blocking_siblings))
+		list_del_init(&ci->conn.blocking_siblings);
 
-static void client_decref(struct client_info *ci)
-{
-	if (ci && --ci->refcnt == 0)
-		destroy_client(ci);
+	unregister_event(ci->conn.fd);
+
+	dprintf("refcnt:%d, fd:%d, %s:%d\n",
+		ci->refcnt, ci->conn.fd,
+		ci->conn.ipstr, ci->conn.port);
+
+	if (ci->refcnt)
+		return;
+
+	destroy_client(ci);
 }
 
 static struct client_info *create_client(int fd, struct cluster_info *cluster)
@@ -693,7 +697,7 @@ static struct client_info *create_client(int fd, struct cluster_info *cluster)
 
 	ci->conn.fd = fd;
 	ci->conn.events = EPOLLIN;
-	ci->refcnt = 1;
+	ci->refcnt = 0;
 
 	INIT_LIST_HEAD(&ci->done_reqs);
 	INIT_LIST_HEAD(&ci->conn.blocking_siblings);
@@ -720,12 +724,7 @@ static void client_handler(int fd, int events, void *data)
 err:
 		dprintf("closed connection %d, %s:%d\n", fd,
 			ci->conn.ipstr, ci->conn.port);
-
-		if (!list_empty(&ci->conn.blocking_siblings))
-			list_del_init(&ci->conn.blocking_siblings);
-
-		unregister_event(fd);
-		client_decref(ci);
+		clear_client(ci);
 	}
 }
 
-- 
1.7.10.4