CEPH filesystem development
 help / color / mirror / Atom feed
From: Alex Elder <elder@inktank.com>
To: Sage Weil <sage@inktank.com>
Cc: ceph-devel@vger.kernel.org
Subject: Re: [PATCH 08/13] libceph: start separating connection flags from state
Date: Fri, 01 Jun 2012 07:13:41 -0500	[thread overview]
Message-ID: <4FC8B1F5.5040306@inktank.com> (raw)
In-Reply-To: <Pine.LNX.4.64.1205312125060.2166@cobra.newdream.net>

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<elder@inktank.com>
>> ---
>>   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<sage@inktank.com>

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
>>
>>
>


  reply	other threads:[~2012-06-01 12:13 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-30 19:24 [PATCH 00/13] libceph: cleanups preparing for state cleanup Alex Elder
2012-05-30 19:34 ` [PATCH 01/13] libceph: eliminate connection state "DEAD" Alex Elder
2012-05-31 16:20   ` Yehuda Sadeh
2012-05-30 19:34 ` [PATCH 02/13] libceph: kill bad_proto ceph connection op Alex Elder
2012-05-31 16:30   ` Yehuda Sadeh
2012-05-30 19:34 ` [PATCH 03/13] libceph: delete useless SOCK_CLOSED manipulations Alex Elder
2012-06-01 18:47   ` Alex Elder
2012-05-30 19:34 ` [PATCH 04/13] libceph: rename socket callbacks Alex Elder
2012-05-31 16:33   ` Yehuda Sadeh Weinraub
2012-06-01  4:02   ` Sage Weil
2012-05-30 19:34 ` [PATCH 05/13] libceph: rename kvec_reset and kvec_add functions Alex Elder
2012-05-31 16:34   ` Yehuda Sadeh
2012-06-01  4:02   ` Sage Weil
2012-05-30 19:34 ` [PATCH 06/13] libceph: embed ceph messenger structure in ceph_client Alex Elder
2012-05-31 16:44   ` Yehuda Sadeh
2012-06-01  4:04   ` Sage Weil
2012-05-30 19:34 ` [PATCH 07/13] libceph: embed ceph connection structure in mon_client Alex Elder
2012-06-01  4:24   ` Sage Weil
2012-06-01 12:12     ` Alex Elder
2012-06-01 13:30       ` Alex Elder
2012-06-01 16:20         ` Sage Weil
2012-06-01 16:32           ` Alex Elder
2012-06-01 16:39             ` Sage Weil
2012-06-01 17:09     ` Alex Elder
2012-06-01 17:10       ` Sage Weil
2012-05-30 19:35 ` [PATCH 08/13] libceph: start separating connection flags from state Alex Elder
2012-06-01  4:25   ` Sage Weil
2012-06-01 12:13     ` Alex Elder [this message]
2012-05-30 19:35 ` [PATCH 09/13] libceph: start tracking connection socket state Alex Elder
2012-06-01  4:28   ` Sage Weil
2012-06-01 12:15     ` Alex Elder
2012-06-12  4:52   ` Yan, Zheng
2012-06-12  5:00     ` Sage Weil
2012-06-12  5:02       ` Yan, Zheng
2012-06-12 16:58         ` Alex Elder
2012-06-13  1:50           ` Yan, Zheng
2012-05-30 19:35 ` [PATCH 10/13] libceph: provide osd number when creating osd Alex Elder
2012-06-01  4:29   ` Sage Weil
2012-05-30 19:35 ` [PATCH 11/13] libceph: init monitor connection when opening Alex Elder
2012-06-01  4:30   ` Sage Weil
2012-05-30 19:35 ` [PATCH 12/13] libceph: fully initialize connection in con_init() Alex Elder
2012-06-01  4:31   ` Sage Weil
2012-05-30 19:35 ` [PATCH 13/13] libceph: set CLOSED state bit in con_init Alex Elder
2012-06-01  4:32   ` Sage Weil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FC8B1F5.5040306@inktank.com \
    --to=elder@inktank.com \
    --cc=ceph-devel@vger.kernel.org \
    --cc=sage@inktank.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox