Question: blktrace server mode & orderly shut down

PATCH Alan D. Brunelle 2 participants 3 messages Feb 3, 2009 – Feb 6, 2009

lore

Share & tools

lore

Alan D. Brunelle

author

2009-02-03 22:30 UTC #0 lore raw

According to the recv(2) man page:

The return value will be 0 when the peer has performed an orderly shutdown.

Yet the code in read_data_net and net_get_header both attempt to re-read from a socket that will _never_ have data. (I hit this problem today on blktrace2 - I've got orderly shut downs in place.)

Does this pseudo-patch break something?

blktrace.c +0 -0

index c55b491..9ef7313 100644
@@ -550,7 +550,7 @@ static int read_data_net(struct thread_information

*tip, voi

                ret = recv(nc->in_fd, buf, bytes_left, MSG_WAITALL);

                if (!ret)
-                       continue;
+                       return 0;
                else if (ret < 0) {
                        if (errno != EAGAIN) {
                                perror(tip->fn);
@@ -1466,8 +1466,7 @@ static int net_get_header(struct net_connection *nc,
                        usleep(1000);
                        continue;
                } else if (!ret) {
-                       usleep(1000);
-                       continue;
+                       return 0;
                } else {
                        p += ret;
                        bytes_left -= ret;

Jens Axboe

2009-02-06 08:03 UTC #1 lore raw

Quoted 26 lines · Alan D. Brunelle

According to the recv(2) man page:
The return value will be 0 when the peer has performed an orderly shutdown.
Yet the code in read_data_net and net_get_header both attempt to re-read from a socket that will _never_ have data. (I hit this problem today on blktrace2 - I've got orderly shut downs in place.)
Does this pseudo-patch break something?
blktrace.c +1 -1
index c55b491..9ef7313 100644
@@ -550,7 +550,7 @@ static int read_data_net(struct thread_information
 *tip, voi
                ret = recv(nc->in_fd, buf, bytes_left, MSG_WAITALL);
 
                if (!ret)
-                       continue;
+                       return 0;
                else if (ret < 0) {
                        if (errno != EAGAIN) {
                                perror(tip->fn);

blktrace.c static int net_get_header(struct net_connection *nc, · line 1472

Quoted 9 lines

blktrace.c +1 -2

@@ -1466,8 +1466,7 @@ static int net_get_header(struct net_connection *nc,
                        usleep(1000);
                        continue;
                } else if (!ret) {
-                       usleep(1000);
-                       continue;
+                       return 0;
                } else {
                        p += ret;
                        bytes_left -= ret;

My bad, I think the patch looks correct. Feel free to commit it, you can add my acked-by to it.

Signature · 1 line

Jens Axboe

Alan D. Brunelle

author

2009-02-06 11:36 UTC #2 lore raw

In reply to Alan D. Brunelle

Quoted 31 lines · Jens Axboe

Quoted 30 lines · Alan D. Brunelle

According to the recv(2) man page:

The return value will be 0 when the peer has performed an orderly shutdown.

Yet the code in read_data_net and net_get_header both attempt to re-read from a socket that will _never_ have data. (I hit this problem today on blktrace2 - I've got orderly shut downs in place.)

Does this pseudo-patch break something?

blktrace.c +1 -1

index c55b491..9ef7313 100644
@@ -550,7 +550,7 @@ static int read_data_net(struct thread_information
 *tip, voi
                ret = recv(nc->in_fd, buf, bytes_left, MSG_WAITALL);
 
                if (!ret)
-                       continue;
+                       return 0;
                else if (ret < 0) {
                        if (errno != EAGAIN) {

                                perror(tip->fn);

blktrace.c +0 -1

@@ -1466,8 +1466,7 @@ static int net_get_header(struct net_connection *nc,
                        usleep(1000);
                        continue;
                } else if (!ret) {
-                       usleep(1000);

Quoted 9 lines

blktrace.c static int net_get_header(struct net_connection *nc, · line 1472
Quoted 5 lines
blktrace.c +1 -1
-                       continue;
+                       return 0;
                } else {
                        p += ret;
                        bytes_left -= ret;
My bad, I think the patch looks correct. Feel free to commit it, you can add my acked-by to it.

OK, but this patch may not be sufficient: need to look at it some more (in particular, just doing this causes other errors to occur because the connections still try to be read - typically an infinite hang (reading 0)). In blktrace2 I'm adding in orderly shutdowns on both ends of the socket.