-
Notifications
You must be signed in to change notification settings - Fork 56
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 12 of 12 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @yangx-jy)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 12 files at r1.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @yangx-jy)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 8 of 12 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @yangx-jy)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 12 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @yangx-jy)
examples/06-multiple-connections/server.c, line 153 at r1 (raw file):
/* get the connection's completion fd and add it to epoll */
This comment should be moved before rpma_cq_get_fd(cq, &fd);
and a new one should be added here.
examples/06-multiple-connections/server.c, line 216 at r1 (raw file):
/* prepare detected completions for processing */
wrong comment
examples/06-multiple-connections/server.c, line 220 at r1 (raw file):
/* another error occurred - disconnect */
.
d042f72
to
d569a2d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 11 of 12 files reviewed, 3 unresolved discussions (waiting on @grom72 and @ldorau)
examples/06-multiple-connections/server.c, line 153 at r1 (raw file):
Previously, ldorau (Lukasz Dorau) wrote…
/* get the connection's completion fd and add it to epoll */
This comment should be moved before
rpma_cq_get_fd(cq, &fd);
and a new one should be added here.
Done.
examples/06-multiple-connections/server.c, line 216 at r1 (raw file):
Previously, ldorau (Lukasz Dorau) wrote…
/* prepare detected completions for processing */
wrong comment
Done.
examples/06-multiple-connections/server.c, line 220 at r1 (raw file):
Previously, ldorau (Lukasz Dorau) wrote…
/* another error occurred - disconnect */
.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @yangx-jy)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @yangx-jy)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 11 of 12 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @yangx-jy)
examples/06-multiple-connections/server.c, line 401 at r2 (raw file):
*/ return; }
I think you can add this part here:
/* get the connection's main CQ */
struct rpma_cq *cq = NULL;
int ret = rpma_conn_get_cq(clnt->conn, &clnt->cq);
if (ret) {
/* an error occurred - disconnect */
(void) rpma_conn_disconnect(clnt->conn);
return;
}
so, you don't need to call _get_cq()
over and over again.
NOTE: You have to extend the struct client_res
structure to accommodate also the cq
.
examples/08-messages-ping-pong/client.c, line 131 at r2 (raw file):
break; } else if ((ret = rpma_cq_get_completion(cq, &cmpl))) {
Won't it fit into a single line?
Signed-off-by: Xiao Yang <[email protected]>
Signed-off-by: Xiao Yang <[email protected]>
Signed-off-by: Xiao Yang <[email protected]>
Signed-off-by: Xiao Yang <[email protected]>
Signed-off-by: Xiao Yang <[email protected]>
Signed-off-by: Xiao Yang <[email protected]>
Signed-off-by: Xiao Yang <[email protected]>
Signed-off-by: Xiao Yang <[email protected]>
Signed-off-by: Xiao Yang <[email protected]>
d569a2d
to
87dfa45
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 11 of 12 files reviewed, 2 unresolved discussions (waiting on @grom72, @janekmi, and @ldorau)
examples/06-multiple-connections/server.c, line 401 at r2 (raw file):
Previously, janekmi (Jan Michalski) wrote…
I think you can add this part here:
/* get the connection's main CQ */ struct rpma_cq *cq = NULL; int ret = rpma_conn_get_cq(clnt->conn, &clnt->cq); if (ret) { /* an error occurred - disconnect */ (void) rpma_conn_disconnect(clnt->conn); return; }so, you don't need to call
_get_cq()
over and over again.NOTE: You have to extend the
struct client_res
structure to accommodate also thecq
.
Done. Good suggestion. ^_^
examples/08-messages-ping-pong/client.c, line 131 at r2 (raw file):
Previously, janekmi (Jan Michalski) wrote…
Won't it fit into a single line?
Yes, make cstyle
will show line > 80 characters
when it is changed to a single line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @janekmi)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @janekmi)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @yangx-jy)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status:
complete! all files reviewed, all discussions resolved (waiting on @yangx-jy)
Ref: #1212
This change is