[tabled patch 1/1] Fix spinning if EOF from client

[tabled patch 1/1] Fix spinning if EOF from client

[Pete Zaitcev]

I observed that Tabled sometimes starts spinning on CPU. It happens when
it manages to receive a EOF from client (reads zero bytes from the socket).

The fix is to treat EOF as an error and simply dispose of the cli.

However, we need to know when there are no bytes to be read from
a socket that is still open. To that end we modify the API of cli_read:
now it only returns zero in case of EOF, and passes -EAGAIN up.

In order to prevent busy spinning for slow clients, callers of cli_read
return false to the dispatch loop in case of -EAGAIN.

Note that this patch assumes that zero bytes is always EOF, and that
we receive -EAGAIN if there is nothing to read in the first place.

Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>

---
 server/server.c |   18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/server/server.c b/server/server.c
index a28965c..8d100f7 100644
--- a/server/server.c
+++ b/server/server.c
@@ -671,6 +671,12 @@ size_t cli_wqueued(struct client *cli)
 	return cli->write_cnt;
 }
 
+/*
+ * Return:
+ *   0: progress was NOT made (EOF)
+ *  >0: some data was gotten
+ *  <0: an error happened (equals to system error * -1; includes -EAGAIN)
+ */
 static int cli_read(struct client *cli)
 {
 	ssize_t rc;
@@ -682,8 +688,6 @@ do_read:
 	if (rc < 0) {
 		if (errno == EINTR)
 			goto do_read;
-		if (errno == EAGAIN)
-			return 0;
 		return -errno;
 	}
 
@@ -699,7 +703,7 @@ do_read:
 	if (cli->req_used == CLI_REQ_BUF_SZ)
 		return -ENOSPC;
 
-	return 0;
+	return rc != 0;
 }
 
 bool cli_cb_free(struct client *cli, void *cb_data, bool done)
@@ -1186,9 +1190,11 @@ static bool cli_evt_parse_hdr(struct client *cli, unsigned int events)
 static bool cli_evt_read_hdr(struct client *cli, unsigned int events)
 {
 	int rc = cli_read(cli);
-	if (rc < 0) {
+	if (rc <= 0) {
 		if (rc == -ENOSPC)
 			return cli_err(cli, InvalidArgument);
+		if (rc == -EAGAIN)
+			return false;
 
 		cli->state = evt_dispose;
 	} else
@@ -1268,9 +1274,11 @@ err_out:
 static bool cli_evt_read_req(struct client *cli, unsigned int events)
 {
 	int rc = cli_read(cli);
-	if (rc < 0) {
+	if (rc <= 0) {
 		if (rc == -ENOSPC)
 			return cli_err(cli, InvalidArgument);
+		if (rc == -EAGAIN)
+			return false;
 
 		cli->state = evt_dispose;
 	} else

Re: [tabled patch 1/1] Fix spinning if EOF from client

[Jeff Garzik]

On 05/12/2010 12:09 AM, Pete Zaitcev wrote:
> I observed that Tabled sometimes starts spinning on CPU. It happens when
> it manages to receive a EOF from client (reads zero bytes from the socket).
>
> The fix is to treat EOF as an error and simply dispose of the cli.
>
> However, we need to know when there are no bytes to be read from
> a socket that is still open. To that end we modify the API of cli_read:
> now it only returns zero in case of EOF, and passes -EAGAIN up.
>
> In order to prevent busy spinning for slow clients, callers of cli_read
> return false to the dispatch loop in case of -EAGAIN.
>
> Note that this patch assumes that zero bytes is always EOF, and that
> we receive -EAGAIN if there is nothing to read in the first place.
>
> Signed-off-by: Pete Zaitcev<zaitcev@redhat.com>

applied