From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Elder Subject: Re: [PATCH 08/13] libceph: start separating connection flags from state Date: Fri, 01 Jun 2012 07:13:41 -0500 Message-ID: <4FC8B1F5.5040306@inktank.com> References: <4FC673FC.3060004@inktank.com> <4FC67666.4040607@inktank.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mail-gh0-f180.google.com ([209.85.160.180]:44553 "EHLO mail-gh0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755437Ab2FAMNi (ORCPT ); Fri, 1 Jun 2012 08:13:38 -0400 Received: by ghbz12 with SMTP id z12so1931673ghb.11 for ; Fri, 01 Jun 2012 05:13:37 -0700 (PDT) In-Reply-To: Sender: ceph-devel-owner@vger.kernel.org List-ID: To: Sage Weil Cc: ceph-devel@vger.kernel.org On 05/31/2012 11:25 PM, Sage Weil wrote: > On Wed, 30 May 2012, Alex Elder wrote: >> A ceph_connection holds a mixture of connection state (as in "state >> machine" state) and connection flags in a single "state" field. To >> make the distinction more clear, define a new "flags" field and use >> it rather than the "state" field to hold Boolean flag values. >> >> Signed-off-by: Alex Elder >> --- >> include/linux/ceph/messenger.h | 18 +++++++++---- >> net/ceph/messenger.c | 50 >> ++++++++++++++++++++-------------------- >> 2 files changed, 37 insertions(+), 31 deletions(-) >> >> diff --git a/include/linux/ceph/messenger.h b/include/linux/ceph/messenger.h >> index 3fbd4be..920235e 100644 >> --- a/include/linux/ceph/messenger.h >> +++ b/include/linux/ceph/messenger.h >> @@ -103,20 +103,25 @@ struct ceph_msg_pos { >> #define MAX_DELAY_INTERVAL (5 * 60 * HZ) >> >> /* >> - * ceph_connection state bit flags >> + * ceph_connection flag bits >> */ >> + >> #define LOSSYTX 0 /* we can close channel or drop messages on errors >> */ >> -#define CONNECTING 1 >> -#define NEGOTIATING 2 >> #define KEEPALIVE_PENDING 3 >> #define WRITE_PENDING 4 /* we have data ready to send */ >> +#define SOCK_CLOSED 11 /* socket state changed to closed */ >> +#define BACKOFF 15 >> + >> +/* >> + * ceph_connection states >> + */ >> +#define CONNECTING 1 >> +#define NEGOTIATING 2 >> #define STANDBY 8 /* no outgoing messages, socket closed. we >> keep >> * the ceph_connection around to maintain shared >> * state with the peer. */ >> #define CLOSED 10 /* we've closed the connection */ >> -#define SOCK_CLOSED 11 /* socket state changed to closed */ >> #define OPENING 13 /* open connection w/ (possibly new) peer */ >> -#define BACKOFF 15 > > Later it might be work prefixing these with FLAG_ and/or STATE_. Absolutely, I'm saving that easy stuff for the end. I'll move the definitions into messenger.c as well if I can. > Reviewed-by: Sage Weil Thanks. -Alex >> >> /* >> * A single connection with another host. >> @@ -133,7 +138,8 @@ struct ceph_connection { >> >> struct ceph_messenger *msgr; >> struct socket *sock; >> - unsigned long state; /* connection state (see flags above) */ >> + unsigned long flags; >> + unsigned long state; >> const char *error_msg; /* error message, if any */ >> >> struct ceph_entity_addr peer_addr; /* peer address */ >> diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c >> index 19f1948..29055df 100644 >> --- a/net/ceph/messenger.c >> +++ b/net/ceph/messenger.c >> @@ -176,7 +176,7 @@ static void ceph_sock_write_space(struct sock *sk) >> * buffer. See net/ipv4/tcp_input.c:tcp_check_space() >> * and net/core/stream.c:sk_stream_write_space(). >> */ >> - if (test_bit(WRITE_PENDING,&con->state)) { >> + if (test_bit(WRITE_PENDING,&con->flags)) { >> if (sk_stream_wspace(sk)>= sk_stream_min_wspace(sk)) { >> dout("%s %p queueing write work\n", __func__, con); >> clear_bit(SOCK_NOSPACE,&sk->sk_socket->flags); >> @@ -203,7 +203,7 @@ static void ceph_sock_state_change(struct sock *sk) >> dout("%s TCP_CLOSE\n", __func__); >> case TCP_CLOSE_WAIT: >> dout("%s TCP_CLOSE_WAIT\n", __func__); >> - if (test_and_set_bit(SOCK_CLOSED,&con->state) == 0) { >> + if (test_and_set_bit(SOCK_CLOSED,&con->flags) == 0) { >> if (test_bit(CONNECTING,&con->state)) >> con->error_msg = "connection failed"; >> else >> @@ -393,9 +393,9 @@ void ceph_con_close(struct ceph_connection *con) >> ceph_pr_addr(&con->peer_addr.in_addr)); >> set_bit(CLOSED,&con->state); /* in case there's queued work */ >> clear_bit(STANDBY,&con->state); /* avoid connect_seq bump */ >> - clear_bit(LOSSYTX,&con->state); /* so we retry next connect */ >> - clear_bit(KEEPALIVE_PENDING,&con->state); >> - clear_bit(WRITE_PENDING,&con->state); >> + clear_bit(LOSSYTX,&con->flags); /* so we retry next connect */ >> + clear_bit(KEEPALIVE_PENDING,&con->flags); >> + clear_bit(WRITE_PENDING,&con->flags); >> mutex_lock(&con->mutex); >> reset_connection(con); >> con->peer_global_seq = 0; >> @@ -612,7 +612,7 @@ static void prepare_write_message(struct ceph_connection >> *con) >> prepare_write_message_footer(con); >> } >> >> - set_bit(WRITE_PENDING,&con->state); >> + set_bit(WRITE_PENDING,&con->flags); >> } >> >> /* >> @@ -633,7 +633,7 @@ static void prepare_write_ack(struct ceph_connection *con) >> &con->out_temp_ack); >> >> con->out_more = 1; /* more will follow.. eventually.. */ >> - set_bit(WRITE_PENDING,&con->state); >> + set_bit(WRITE_PENDING,&con->flags); >> } >> >> /* >> @@ -644,7 +644,7 @@ static void prepare_write_keepalive(struct ceph_connection >> *con) >> dout("prepare_write_keepalive %p\n", con); >> con_out_kvec_reset(con); >> con_out_kvec_add(con, sizeof (tag_keepalive),&tag_keepalive); >> - set_bit(WRITE_PENDING,&con->state); >> + set_bit(WRITE_PENDING,&con->flags); >> } >> >> /* >> @@ -673,7 +673,7 @@ static struct ceph_auth_handshake >> *get_connect_authorizer(struct ceph_connection >> >> if (IS_ERR(auth)) >> return auth; >> - if (test_bit(CLOSED,&con->state) || test_bit(OPENING,&con->state)) >> + if (test_bit(CLOSED,&con->state) || test_bit(OPENING,&con->flags)) >> return ERR_PTR(-EAGAIN); >> >> con->auth_reply_buf = auth->authorizer_reply_buf; >> @@ -693,7 +693,7 @@ static void prepare_write_banner(struct ceph_connection >> *con) >> &con->msgr->my_enc_addr); >> >> con->out_more = 0; >> - set_bit(WRITE_PENDING,&con->state); >> + set_bit(WRITE_PENDING,&con->flags); >> } >> >> static int prepare_write_connect(struct ceph_connection *con) >> @@ -743,7 +743,7 @@ static int prepare_write_connect(struct ceph_connection >> *con) >> auth->authorizer_buf); >> >> con->out_more = 0; >> - set_bit(WRITE_PENDING,&con->state); >> + set_bit(WRITE_PENDING,&con->flags); >> >> return 0; >> } >> @@ -1490,7 +1490,7 @@ static int process_connect(struct ceph_connection *con) >> le32_to_cpu(con->in_reply.connect_seq)); >> >> if (con->in_reply.flags& CEPH_MSG_CONNECT_LOSSY) >> - set_bit(LOSSYTX,&con->state); >> + set_bit(LOSSYTX,&con->flags); >> >> prepare_read_tag(con); >> break; >> @@ -1931,14 +1931,14 @@ do_next: >> prepare_write_ack(con); >> goto more; >> } >> - if (test_and_clear_bit(KEEPALIVE_PENDING,&con->state)) { >> + if (test_and_clear_bit(KEEPALIVE_PENDING,&con->flags)) { >> prepare_write_keepalive(con); >> goto more; >> } >> } >> >> /* Nothing to do! */ >> - clear_bit(WRITE_PENDING,&con->state); >> + clear_bit(WRITE_PENDING,&con->flags); >> dout("try_write nothing else to write.\n"); >> ret = 0; >> out: >> @@ -2104,7 +2104,7 @@ static void con_work(struct work_struct *work) >> >> mutex_lock(&con->mutex); >> restart: >> - if (test_and_clear_bit(BACKOFF,&con->state)) { >> + if (test_and_clear_bit(BACKOFF,&con->flags)) { >> dout("con_work %p backing off\n", con); >> if (queue_delayed_work(ceph_msgr_wq,&con->work, >> round_jiffies_relative(con->delay))) { >> @@ -2133,7 +2133,7 @@ restart: >> con_close_socket(con); >> } >> >> - if (test_and_clear_bit(SOCK_CLOSED,&con->state)) >> + if (test_and_clear_bit(SOCK_CLOSED,&con->flags)) >> goto fault; >> >> ret = try_read(con); >> @@ -2172,7 +2172,7 @@ static void ceph_fault(struct ceph_connection *con) >> dout("fault %p state %lu to peer %s\n", >> con, con->state, ceph_pr_addr(&con->peer_addr.in_addr)); >> >> - if (test_bit(LOSSYTX,&con->state)) { >> + if (test_bit(LOSSYTX,&con->flags)) { >> dout("fault on LOSSYTX channel\n"); >> goto out; >> } >> @@ -2194,9 +2194,9 @@ static void ceph_fault(struct ceph_connection *con) >> /* If there are no messages queued or keepalive pending, place >> * the connection in a STANDBY state */ >> if (list_empty(&con->out_queue)&& >> - !test_bit(KEEPALIVE_PENDING,&con->state)) { >> + !test_bit(KEEPALIVE_PENDING,&con->flags)) { >> dout("fault %p setting STANDBY clearing WRITE_PENDING\n", >> con); >> - clear_bit(WRITE_PENDING,&con->state); >> + clear_bit(WRITE_PENDING,&con->flags); >> set_bit(STANDBY,&con->state); >> } else { >> /* retry after a delay. */ >> @@ -2220,7 +2220,7 @@ static void ceph_fault(struct ceph_connection *con) >> * that when con_work restarts we schedule the >> * delay then. >> */ >> - set_bit(BACKOFF,&con->state); >> + set_bit(BACKOFF,&con->flags); >> } >> } >> >> @@ -2276,8 +2276,8 @@ static void clear_standby(struct ceph_connection *con) >> mutex_lock(&con->mutex); >> dout("clear_standby %p and ++connect_seq\n", con); >> con->connect_seq++; >> - WARN_ON(test_bit(WRITE_PENDING,&con->state)); >> - WARN_ON(test_bit(KEEPALIVE_PENDING,&con->state)); >> + WARN_ON(test_bit(WRITE_PENDING,&con->flags)); >> + WARN_ON(test_bit(KEEPALIVE_PENDING,&con->flags)); >> mutex_unlock(&con->mutex); >> } >> } >> @@ -2315,7 +2315,7 @@ void ceph_con_send(struct ceph_connection *con, struct >> ceph_msg *msg) >> /* if there wasn't anything waiting to send before, queue >> * new work */ >> clear_standby(con); >> - if (test_and_set_bit(WRITE_PENDING,&con->state) == 0) >> + if (test_and_set_bit(WRITE_PENDING,&con->flags) == 0) >> queue_con(con); >> } >> EXPORT_SYMBOL(ceph_con_send); >> @@ -2382,8 +2382,8 @@ void ceph_con_keepalive(struct ceph_connection *con) >> { >> dout("con_keepalive %p\n", con); >> clear_standby(con); >> - if (test_and_set_bit(KEEPALIVE_PENDING,&con->state) == 0&& >> - test_and_set_bit(WRITE_PENDING,&con->state) == 0) >> + if (test_and_set_bit(KEEPALIVE_PENDING,&con->flags) == 0&& >> + test_and_set_bit(WRITE_PENDING,&con->flags) == 0) >> queue_con(con); >> } >> EXPORT_SYMBOL(ceph_con_keepalive); >> -- >> 1.7.5.4 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> >