[openib-general] Re: [PATCH (repost)] error values cleanup
Libor Michalek
libor at topspin.com
Tue Jun 28 10:55:25 PDT 2005
On Sun, Jun 19, 2005 at 10:33:36AM +0300, Michael S. Tsirkin wrote:
> Here's an update to the latest bits: Libor, does this make sense?
>
> Make sdp_cm_actv_error and sdp_conn_inet_error accept standard, negative
> error values (esp. for sdp_cm_actv_error it was sufficiently confusing to
> be passing a positive value to warrant a comment).
> There are still a couple of functions which return positive error
> values but these are static and these positive errors never propagate
> outside a single .c file.
>
> As a side effect, "(0 - result)" in a couple of places gets replaced with
> "result" which is also good. The patch does a couple of cosmetic cleanups
> as well.
Michael, I expanded your patch to remove SDP_CONN_{GET,SET}_ERR, conn->error,
and sdp_conn_error() and just use the already available sk->sk_err and
sock_error() instead.
-Libor
Index: sdp_inet.c
===================================================================
--- sdp_inet.c (revision 2731)
+++ sdp_inet.c (working copy)
@@ -214,10 +214,12 @@
*/
static int sdp_inet_disconnect(struct sdp_opt *conn)
{
+ struct sock *sk;
int result = 0;
/*
* close buffered data transmission space
*/
+ sk = sk_sdp(conn);
conn->send_buf = 0;
/*
* Generate a Disconnect message, and mark self as disconnecting.
@@ -230,7 +232,7 @@
* completions needs to complete the closing.
*/
SDP_CONN_ST_SET(conn, SDP_CONN_ST_ERROR);
- SDP_CONN_SET_ERR(conn, ECONNRESET);
+ sk->sk_err = ECONNRESET;
break;
case SDP_CONN_ST_REQ_RECV:
case SDP_CONN_ST_REP_RECV:
@@ -279,7 +281,7 @@
/*
* abortive close.
*/
- sdp_conn_inet_error(conn, ECONNRESET);
+ sdp_conn_inet_error(conn, -ECONNRESET);
(void)ib_send_cm_dreq(conn->cm_id, NULL, 0);
return result;
@@ -563,7 +565,7 @@
inet_sk(sk)->sport = htons(conn->src_port);
}
- SDP_CONN_SET_ERR(conn, 0);
+ sk->sk_err = 0;
sock->state = SS_CONNECTING;
@@ -658,7 +660,7 @@
break;
case SDP_CONN_ST_CLOSED:
case SDP_CONN_ST_ERROR:
- result = sdp_conn_error(conn) ? : -ECONNABORTED;
+ result = sock_error(sk) ? : -ECONNABORTED;
sock->state = SS_UNCONNECTED;
break;
default:
@@ -1266,7 +1268,7 @@
* completions needs to complete the closing.
*/
SDP_CONN_ST_SET(conn, SDP_CONN_ST_ERROR);
- SDP_CONN_SET_ERR(conn, ECONNRESET);
+ sock->sk->sk_err = ECONNRESET;
break;
case SDP_CONN_ST_LISTEN:
if (flag & RCV_SHUTDOWN) {
Index: sdp_send.c
===================================================================
--- sdp_send.c (revision 2731)
+++ sdp_send.c (working copy)
@@ -2022,8 +2022,8 @@
timeout = sock_sndtimeo(sk, (MSG_DONTWAIT &
msg->msg_flags));
- if (SDP_CONN_GET_ERR(conn)) {
- result = (copied > 0) ? 0 : sdp_conn_error(conn);
+ if (sk->sk_err) {
+ result = (copied > 0) ? 0 : sock_error(sk);
break;
}
Index: sdp_actv.c
===================================================================
--- sdp_actv.c (revision 2731)
+++ sdp_actv.c (working copy)
@@ -43,8 +43,6 @@
int result;
struct sock *sk;
/*
- * error value is positive error.
- *
* Handle errors within active connections stream.
* First generate appropriate response, REJ, DREQ or nothing.
* Second the socket must be notified of the error.
@@ -90,15 +88,16 @@
break;
}
- SDP_CONN_SET_ERR(conn, error);
conn->shutdown = SHUTDOWN_MASK;
conn->send_buf = 0;
sk = sk_sdp(conn);
+ sk->sk_err = -error;
+
if (sk->sk_socket)
sk->sk_socket->state = SS_UNCONNECTED;
- sdp_iocb_q_cancel_all(conn, (0 - error));
+ sdp_iocb_q_cancel_all(conn, error);
sdp_inet_wake_error(sk);
}
@@ -322,7 +321,7 @@
return 0;
error:
- sdp_cm_actv_error(conn, (0 - result));
+ sdp_cm_actv_error(conn, result);
if (conn->state == SDP_CONN_ST_CLOSED) {
conn->cm_id = NULL;
@@ -504,7 +503,7 @@
goto done;
failed:
- sdp_cm_actv_error(conn, (0 - status));
+ sdp_cm_actv_error(conn, status);
done:
sdp_conn_unlock(conn);
sdp_conn_put(conn); /* address resolution reference */
Index: sdp_conn.c
===================================================================
--- sdp_conn.c (revision 2731)
+++ sdp_conn.c (working copy)
@@ -87,22 +87,23 @@
(void)sdp_inet_accept_q_remove(conn);
SDP_CONN_ST_SET(conn, SDP_CONN_ST_ERROR);
- SDP_CONN_SET_ERR(conn, error);
conn->shutdown = SHUTDOWN_MASK;
conn->send_buf = 0;
sk = sk_sdp(conn);
+ sk->sk_err = -error;
+
if (sk->sk_socket)
sk->sk_socket->state = SS_UNCONNECTED;
- sdp_iocb_q_cancel_all(conn, -error);
+ sdp_iocb_q_cancel_all(conn, error);
sdp_inet_wake_error(sk);
}
void sdp_conn_abort(struct sdp_opt *conn)
{
int result;
- int error = ECONNRESET;
+ int error = -ECONNRESET;
sdp_dbg_ctrl(conn, "Abort send. src <%08x:%04x> dst <%08x:%04x>",
conn->src_addr, conn->src_port,
@@ -137,7 +138,7 @@
conn->flags &= ~SDP_CONN_F_DIS_PEND;
case SDP_CONN_ST_DIS_RECV_1:
- error = EPIPE;
+ error = -EPIPE;
case SDP_CONN_ST_ESTABLISHED:
/*
* abortive close.
@@ -154,7 +155,7 @@
* outstanding CM request. Mark it in error, and CM
* completion needs to complete the closing.
*/
- error = ECONNREFUSED;
+ error = -ECONNREFUSED;
break;
case SDP_CONN_ST_ERROR:
case SDP_CONN_ST_CLOSED:
Index: sdp_recv.c
===================================================================
--- sdp_recv.c (revision 2731)
+++ sdp_recv.c (working copy)
@@ -1278,8 +1278,8 @@
* check connection errors, and then wait for more data.
* check status. POSIX 1003.1g order.
*/
- if (SDP_CONN_GET_ERR(conn)) {
- result = (copied > 0) ? 0 : sdp_conn_error(conn);
+ if (sk->sk_err) {
+ result = (copied > 0) ? 0 : sock_error(sk);
break;
}
Index: sdp_conn.h
===================================================================
--- sdp_conn.h (revision 2731)
+++ sdp_conn.h (working copy)
@@ -119,11 +119,6 @@
return (struct sock *)conn;
}
-#define SDP_CONN_SET_ERR(conn, val) \
- ((conn)->error = sk_sdp(conn)->sk_err = (val))
-#define SDP_CONN_GET_ERR(conn) \
- ((conn)->error)
-
/*
* state transition information recording
*/
@@ -264,8 +259,6 @@
*/
u32 nond_recv; /* non discarded buffers received. */
u32 nond_send; /* non discarded buffers sent */
-
- s32 error; /* error value on connection. */
/*
* OOB/URG data transfer.
*/
@@ -491,22 +484,6 @@
sdp_conn_destruct(conn);
}
-/*
- * sdp_conn_error - get the connections error value destructively
- */
-static inline int sdp_conn_error(struct sdp_opt *conn)
-{
- /*
- * The connection error parameter is set and read under the connection
- * lock, however the linux socket error, needs to be xchg'd since the
- * SO_ERROR getsockopt happens outside of the connection lock.
- */
- int error = xchg(&sk_sdp(conn)->sk_err, 0);
- SDP_CONN_SET_ERR(conn, 0);
-
- return -error;
-}
-
static inline void *hashent_arg(s32 hashent)
{
return (void *)(unsigned long)hashent;
Index: sdp_pass.c
===================================================================
--- sdp_pass.c (revision 2731)
+++ sdp_pass.c (working copy)
@@ -42,7 +42,7 @@
{
struct ib_qp_attr *qp_attr;
int attr_mask = 0;
- int result = 0;
+ int result;
sdp_dbg_ctrl(conn, "Passive Establish src <%08x:%04x> dst <%08x:%04x>",
conn->src_addr, conn->src_port,
@@ -63,27 +63,26 @@
result = ib_cm_init_qp_attr(conn->cm_id, qp_attr, &attr_mask);
if (result) {
- sdp_dbg_warn(conn, "Error <%d> QP attributes for RTS",
- result);
+ sdp_dbg_warn(conn, "Error <%d> QP attributes for RTS", result);
goto error;
}
result = ib_modify_qp(conn->qp, qp_attr, attr_mask);
if (result) {
- sdp_dbg_warn(conn, "Error <%d> modifiying QP to RTS", result);
+ sdp_dbg_warn(conn, "Error <%d> modifying QP to RTS", result);
goto error;
}
conn->send_buf = SDP_INET_SEND_SIZE;
result = sdp_send_flush(conn);
- if (0 > result) {
+ if (result < 0) {
sdp_dbg_warn(conn, "Error <%d> flushing sends.", result);
goto error;
}
result = sdp_recv_flush(conn);
- if (0 > result) {
+ if (result < 0) {
sdp_dbg_warn(conn, "Error <%d> flushing receives.", result);
goto error;
}
@@ -95,7 +94,7 @@
error:
kfree(qp_attr);
done:
- sdp_conn_inet_error(conn, -result);
+ sdp_conn_inet_error(conn, result);
return result;
}
/*
Index: sdp_event.c
===================================================================
--- sdp_event.c (revision 2731)
+++ sdp_event.c (working copy)
@@ -185,7 +185,7 @@
static void sdp_cm_to_error(struct sdp_opt *conn)
{
- sdp_conn_inet_error(conn, ECONNRESET);
+ sdp_conn_inet_error(conn, -ECONNRESET);
conn->cm_id = NULL;
sdp_conn_put(conn); /* CM reference */
}
@@ -203,11 +203,11 @@
*/
switch (conn->state) {
case SDP_CONN_ST_REQ_SENT:
- sdp_cm_actv_error(conn, ECONNREFUSED);
+ sdp_cm_actv_error(conn, -ECONNREFUSED);
break;
case SDP_CONN_ST_REQ_RECV:
case SDP_CONN_ST_ESTABLISHED:
- sdp_conn_inet_error(conn, ECONNREFUSED);
+ sdp_conn_inet_error(conn, -ECONNREFUSED);
break;
case SDP_CONN_ST_TIME_WAIT_1:
sdp_dbg_warn(conn, "Unexpected connection state");
More information about the general
mailing list