libceph: cleanups preparing for state cleanup
Subsystems (2)
cover libceph: cleanups preparing for state cleanup
I am working on some fairly big changes to the ceph/RADOS client messenger code. The ultimate goal is to simplify the code by having it more obviously follow a state machine, but getting to that point isn't necessarily easy.
My approach is to make evolutionary changes, making a long series of small changes, each of which either produces code that works identical to before, or which very explicitly fixes a bug or adds or modifies a feature while continuing to provide functionality that is equivalent and/or meets what is required.
This series contains a few small batches of changes to begin this process. In general they're dependent on each other, so they are being provided in one series, but I group them below into some smaller logical subsets.
-Alex
[PATCH 01/13] libceph: eliminate connection state "DEAD" [PATCH 02/13] libceph: kill bad_proto ceph connection op [PATCH 03/13] libceph: delete useless SOCK_CLOSED manipulations
These three delete dead/unused code
[PATCH 04/13] libceph: rename socket callbacks
[PATCH 05/13] libceph: rename kvec_reset and kvec_add functions
These two simply rename some symbols.
[PATCH 06/13] libceph: embed ceph messenger structure in ceph_client [PATCH 07/13] libceph: embed ceph connection structure in mon_client
These two each change a structure definition so that what was once a pointer to a structure becomes instead an embedded structure of the pointed-to type. Doing this makes it obvious that the relationship between the containing structure and the embedded one is purely one-to-one.
[PATCH 08/13] libceph: start separating connection flags from state
This identifies a set of values kept in the "state" field and records them instead in a new "flags" field, so it is obvious the role each plays (whether it's a state diagram state, or whether it's a Boolean flag).
[PATCH 09/13] libceph: start tracking connection socket state
This adds code to explicitly track the state of the socket used by a ceph connection. It begins the process of trying to clean up some fuzziness in how the overall state of a ceph connection is tracked.
[PATCH 10/13] libceph: provide osd number when creating osd [PATCH 11/13] libceph: init monitor connection when opening [PATCH 12/13] libceph: fully initialize connection in con_init() [PATCH 13/13] libceph: set CLOSED state bit in con_init
This series moves things around a bit so that all ceph connection initialization is done at one time, by code defined with the "net/ceph/messenger.c" source file. It also makes explicit that a newly initialized ceph connection is in CLOSED state.
PATCH 1/13 libceph: eliminate connection state "DEAD"
The ceph connection state "DEAD" is never set and is therefore not needed. Eliminate it.
--- include/linux/ceph/messenger.h | 1 - net/ceph/messenger.c | 6 ------ 2 files changed, 0 insertions(+), 7 deletions(-)
include/linux/ceph/messenger.h +0 -1
@@ -119,7 +119,6 @@ struct ceph_msg_pos {#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 DEAD 14/* dead, about to kfree */ #define BACKOFF 15/*
net/ceph/messenger.c +0 -6
@@ -2087,12 +2087,6 @@ bad_tag:*/ static void queue_con (struct ceph_connection *con) { -if (test_bit (DEAD , &con->state)) { -dout ("queue_con %p ignoring: DEAD\n" , - con); -return ; - } -if (!con->ops->get (con)) {dout ("queue_con %p ref count 0\n" , con);return ;
Signature · 1 line
1.7.5.4
PATCH 2/13 libceph: kill bad_proto ceph connection op
In reply to Alex ElderNo code sets a bad_proto method in its ceph connection operations vector, so just get rid of it.
--- include/linux/ceph/messenger.h | 3 --- net/ceph/messenger.c | 5 ----- 2 files changed, 0 insertions(+), 8 deletions(-)
include/linux/ceph/messenger.h +0 -3
@@ -31,9 +31,6 @@ struct ceph_connection_operations {int (*verify_authorizer_reply) (struct ceph_connection *con,int len);int (*invalidate_authorizer)(struct ceph_connection *con); -/* protocol version mismatch */ -void (*bad_proto) (struct ceph_connection *con); -/* there was some error on the socket (disconnect, whatever) */ void (*fault) (struct ceph_connection *con);
net/ceph/messenger.c +0 -5
@@ -1356,11 +1356,6 @@ static void fail_protocol(struct ceph_connection *con) {reset_connection (con);set_bit (CLOSED , &con->state);/* in case there's queued work */ - -mutex_unlock (&con->mutex); -if (con->ops->bad_proto) - con->ops->bad_proto (con); -mutex_lock (&con->mutex); }static int process_connect (struct ceph_connection *con)
Signature · 1 line
1.7.5.4
PATCH 3/13 libceph: delete useless SOCK_CLOSED manipulations
In reply to Alex ElderIn con_close_socket(), SOCK_CLOSED is set in the connection state, then cleared again after shutting down the socket. Nothing between the setting and clearing of that bit will ever be affected by it, so there's no point in setting/clearing it at all. So don't.
--- net/ceph/messenger.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
net/ceph/messenger.c +0 -2
@@ -338,11 +338,9 @@ static int con_close_socket(struct ceph_connection *con)dout ("con_close_socket on %p sock %p\n" , con, con->sock);if (!con->sock)return 0 ; -set_bit (SOCK_CLOSED , &con->state); rc = con->sock->ops->shutdown (con->sock,SHUT_RDWR );sock_release (con->sock); con->sock =NULL ; -clear_bit (SOCK_CLOSED , &con->state);return rc; }
Signature · 1 line
1.7.5.4
PATCH 4/13 libceph: rename socket callbacks
In reply to Alex ElderChange the names of the three socket callback functions to make it more obvious they're specifically associated with a connection's socket (not the ceph connection that uses it).
--- net/ceph/messenger.c | 28 ++++++++++++++-------------- 1 files changed, 14 insertions(+), 14 deletions(-)
net/ceph/messenger.c +14 -14
@@ -153,46 +153,46 @@ EXPORT_SYMBOL(ceph_msgr_flush);*/ /* data available on socket, or listen socket received a connect */ -static void ceph_data_ready (struct sock *sk,int count_unused) +static void ceph_sock_data_ready (struct sock *sk,int count_unused) {struct ceph_connection *con = sk->sk_user_data;if (sk->sk_state !=TCP_CLOSE_WAIT ) { -dout ("ceph_data_ready on %p state = %lu, queueing work\n" , +dout ("%s on %p state = %lu, queueing work\n" , __func__, con, con->state);queue_con (con); } }/* socket has buffer space for writing */ -static void ceph_write_space (struct sock *sk) +static void ceph_sock_write_space (struct sock *sk) {struct ceph_connection *con = sk->sk_user_data;/* only queue to workqueue if there is data we want to write, * and there is sufficient space in the socket buffer to accept -* more data. clear SOCK_NOSPACE so that ceph_write_space() +* more data. clear SOCK_NOSPACE so that ceph_sock_write_space() * doesn't get called again until try_write() fills the socket * 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 (sk_stream_wspace (sk) >=sk_stream_min_wspace (sk)) { -dout ("ceph_write_space %p queueing write work\n" , con); +dout ("%s %p queueing write work\n" , __func__, con);clear_bit (SOCK_NOSPACE , &sk->sk_socket->flags);queue_con (con); } }else { -dout ("ceph_write_space %p nothing to write\n" , con); +dout ("%s %p nothing to write\n" , __func__, con); } }/* socket's state has changed */ -static void ceph_state_change (struct sock *sk) +static void ceph_sock_state_change (struct sock *sk) {struct ceph_connection *con = sk->sk_user_data; -dout ("ceph_state_change %p state = %lu sk_state = %u\n" , +dout ("%s %p state = %lu sk_state = %u\n" , __func__, con, con->state, sk->sk_state);if (test_bit (CLOSED , &con->state))
@@ -200,9 +200,9 @@ static void ceph_state_change(struct sock *sk)switch (sk->sk_state) {case TCP_CLOSE : -dout ("ceph_state_change TCP_CLOSE\n" ); +dout ("%s TCP_CLOSE\n" , __func__);case TCP_CLOSE_WAIT : -dout ("ceph_state_change TCP_CLOSE_WAIT\n" ); +dout ("%s TCP_CLOSE_WAIT\n" , __func__);if (test_and_set_bit (SOCK_CLOSED , &con->state) ==0 ) {if (test_bit (CONNECTING , &con->state)) con->error_msg ="connection failed" ;
@@ -212,7 +212,7 @@ static void ceph_state_change(struct sock *sk) }break ;case TCP_ESTABLISHED : -dout ("ceph_state_change TCP_ESTABLISHED\n" ); +dout ("%s TCP_ESTABLISHED\n" , __func__);queue_con (con);break ;default :/* Everything else is uninteresting */
@@ -228,9 +228,9 @@ static void set_sock_callbacks(struct socket *sock, {struct sock *sk = sock->sk; sk->sk_user_data = con; - sk->sk_data_ready = ceph_data_ready; - sk->sk_write_space = ceph_write_space; - sk->sk_state_change = ceph_state_change; + sk->sk_data_ready = ceph_sock_data_ready; + sk->sk_write_space = ceph_sock_write_space; + sk->sk_state_change = ceph_sock_state_change; }
Signature · 1 line
1.7.5.4
PATCH 5/13 libceph: rename kvec_reset and kvec_add functions
In reply to Alex ElderThe functions ceph_con_out_kvec_reset() and ceph_con_out_kvec_add() are entirely private functions, so drop the "ceph_" prefix in their name to make them slightly more wieldy.
--- net/ceph/messenger.c | 48 ++++++++++++++++++++++++------------------------ 1 files changed, 24 insertions(+), 24 deletions(-)
net/ceph/messenger.c +24 -24
@@ -484,14 +484,14 @@ static u32 get_global_seq(struct ceph_messenger *msgr, u32 gt)return ret; } -static void ceph_con_out_kvec_reset (struct ceph_connection *con) +static void con_out_kvec_reset (struct ceph_connection *con) { con->out_kvec_left =0 ; con->out_kvec_bytes =0 ; con->out_kvec_cur = &con->out_kvec[0 ]; } -static void ceph_con_out_kvec_add (struct ceph_connection *con, +static void con_out_kvec_add (struct ceph_connection *con,size_t size,void *data) {int index;
@@ -532,7 +532,7 @@ static void prepare_write_message(struct ceph_connection *con)struct ceph_msg *m;u32 crc; -ceph_con_out_kvec_reset (con); +con_out_kvec_reset (con); con->out_kvec_is_msg = true; con->out_msg_done = false;
@@ -540,9 +540,9 @@ static void prepare_write_message(struct ceph_connection *con)* TCP packet that's a good thing. */ if (con->in_seq > con->in_seq_acked) { con->in_seq_acked = con->in_seq; -ceph_con_out_kvec_add (con,sizeof (tag_ack), &tag_ack); +con_out_kvec_add (con,sizeof (tag_ack), &tag_ack); con->out_temp_ack =cpu_to_le64 (con->in_seq_acked); -ceph_con_out_kvec_add (con,sizeof (con->out_temp_ack), +con_out_kvec_add (con,sizeof (con->out_temp_ack), &con->out_temp_ack); }
@@ -570,12 +570,12 @@ static void prepare_write_message(struct ceph_connection *con)BUG_ON (le32_to_cpu (m->hdr.front_len) != m->front.iov_len);/* tag + hdr + front + middle */ -ceph_con_out_kvec_add (con,sizeof (tag_msg), &tag_msg); -ceph_con_out_kvec_add (con,sizeof (m->hdr), &m->hdr); -ceph_con_out_kvec_add (con, m->front.iov_len, m->front.iov_base); +con_out_kvec_add (con,sizeof (tag_msg), &tag_msg); +con_out_kvec_add (con,sizeof (m->hdr), &m->hdr); +con_out_kvec_add (con, m->front.iov_len, m->front.iov_base);if (m->middle) -ceph_con_out_kvec_add (con, m->middle->vec.iov_len, +con_out_kvec_add (con, m->middle->vec.iov_len, m->middle->vec.iov_base);/* fill in crc (except data pages), footer */
@@ -624,12 +624,12 @@ static void prepare_write_ack(struct ceph_connection *con) con->in_seq_acked, con->in_seq); con->in_seq_acked = con->in_seq; -ceph_con_out_kvec_reset (con); +con_out_kvec_reset (con); -ceph_con_out_kvec_add (con,sizeof (tag_ack), &tag_ack); +con_out_kvec_add (con,sizeof (tag_ack), &tag_ack); con->out_temp_ack =cpu_to_le64 (con->in_seq_acked); -ceph_con_out_kvec_add (con,sizeof (con->out_temp_ack), +con_out_kvec_add (con,sizeof (con->out_temp_ack), &con->out_temp_ack); con->out_more =1 ;/* more will follow.. eventually.. */
@@ -642,8 +642,8 @@ static void prepare_write_ack(struct ceph_connection *con)static void prepare_write_keepalive (struct ceph_connection *con) {dout ("prepare_write_keepalive %p\n" , con); -ceph_con_out_kvec_reset (con); -ceph_con_out_kvec_add (con,sizeof (tag_keepalive), &tag_keepalive); +con_out_kvec_reset (con); +con_out_kvec_add (con,sizeof (tag_keepalive), &tag_keepalive);set_bit (WRITE_PENDING , &con->state); }
@@ -688,8 +688,8 @@ static struct ceph_auth_handshake *get_connect_authorizer(struct ceph_connection*/ static void prepare_write_banner (struct ceph_connection *con) { -ceph_con_out_kvec_add (con,strlen (CEPH_BANNER ),CEPH_BANNER ); -ceph_con_out_kvec_add (con,sizeof (con->msgr->my_enc_addr), +con_out_kvec_add (con,strlen (CEPH_BANNER ),CEPH_BANNER ); +con_out_kvec_add (con,sizeof (con->msgr->my_enc_addr), &con->msgr->my_enc_addr); con->out_more =0 ;
@@ -736,10 +736,10 @@ static int prepare_write_connect(struct ceph_connection *con) con->out_connect.authorizer_len = auth ?cpu_to_le32 (auth->authorizer_buf_len) :0 ; -ceph_con_out_kvec_add (con,sizeof (con->out_connect), +con_out_kvec_add (con,sizeof (con->out_connect), &con->out_connect);if (auth && auth->authorizer_buf_len) -ceph_con_out_kvec_add (con, auth->authorizer_buf_len, +con_out_kvec_add (con, auth->authorizer_buf_len, auth->authorizer_buf); con->out_more =0 ;
@@ -933,7 +933,7 @@ static int write_partial_msg_pages(struct ceph_connection *con)/* prepare and queue up footer, too */ if (!do_datacrc) con->out_msg->footer.flags |=CEPH_MSG_FOOTER_NOCRC ; -ceph_con_out_kvec_reset (con); +con_out_kvec_reset (con);prepare_write_message_footer (con); ret =1 ; out:
@@ -1396,7 +1396,7 @@ static int process_connect(struct ceph_connection *con)return -1 ; } con->auth_retry =1 ; -ceph_con_out_kvec_reset (con); +con_out_kvec_reset (con); ret =prepare_write_connect (con);if (ret <0 )return ret;
@@ -1417,7 +1417,7 @@ static int process_connect(struct ceph_connection *con)ENTITY_NAME (con->peer_name), ceph_pr_addr(&con->peer_addr.in_addr));reset_connection (con); -ceph_con_out_kvec_reset (con); +con_out_kvec_reset (con); ret =prepare_write_connect (con);if (ret <0 )return ret;
@@ -1443,7 +1443,7 @@ static int process_connect(struct ceph_connection *con)le32_to_cpu (con->out_connect.connect_seq), le32_to_cpu(con->in_connect.connect_seq)); con->connect_seq =le32_to_cpu (con->in_connect.connect_seq); -ceph_con_out_kvec_reset (con); +con_out_kvec_reset (con); ret =prepare_write_connect (con);if (ret <0 )return ret;
@@ -1460,7 +1460,7 @@ static int process_connect(struct ceph_connection *con) le32_to_cpu(con->in_connect.global_seq));get_global_seq (con->msgr,le32_to_cpu (con->in_connect.global_seq)); -ceph_con_out_kvec_reset (con); +con_out_kvec_reset (con); ret =prepare_write_connect (con);if (ret <0 )return ret;
@@ -1867,7 +1867,7 @@ more:/* open the socket first? */ if (con->sock ==NULL ) { -ceph_con_out_kvec_reset (con); +con_out_kvec_reset (con);prepare_write_banner (con); ret =prepare_write_connect (con);if (ret <0 )
Signature · 1 line
1.7.5.4
PATCH 6/13 libceph: embed ceph messenger structure in ceph_client
In reply to Alex ElderA ceph client has a pointer to a ceph messenger structure in it. There is always exactly one ceph messenger for a ceph client, so there is no need to allocate it separate from the ceph client structure.
Switch the ceph_client structure to embed its ceph_messenger structure.
--- fs/ceph/mds_client.c | 2 +- include/linux/ceph/libceph.h | 2 +- include/linux/ceph/messenger.h | 9 +++++---- net/ceph/ceph_common.c | 18 +++++------------- net/ceph/messenger.c | 30 +++++++++--------------------- net/ceph/mon_client.c | 6 +++--- net/ceph/osd_client.c | 4 ++-- 7 files changed, 26 insertions(+), 45 deletions(-)
fs/ceph/mds_client.c +1 -1
@@ -394,7 +394,7 @@ static struct ceph_mds_session *register_session(struct ceph_mds_client *mdsc, s->s_seq =0 ;mutex_init (&s->s_mutex); -ceph_con_init (mdsc->fsc->client->msgr, &s->s_con); +ceph_con_init (&mdsc->fsc->client->msgr, &s->s_con); s->s_con.private = s; s->s_con.ops = &mds_con_ops; s->s_con.peer_name.type =CEPH_ENTITY_TYPE_MDS ;
include/linux/ceph/libceph.h +1 -1
@@ -131,7 +131,7 @@ struct ceph_client {u32 supported_features;u32 required_features; -struct ceph_messenger *msgr;/* messenger instance */ +struct ceph_messenger msgr;/* messenger instance */ struct ceph_mon_client monc;struct ceph_osd_client osdc;
include/linux/ceph/messenger.h +5 -4
@@ -211,10 +211,11 @@ extern int ceph_msgr_init(void);extern void ceph_msgr_exit (void );extern void ceph_msgr_flush (void ); -extern struct ceph_messenger *ceph_messenger_create ( -struct ceph_entity_addr *myaddr, -u32 features,u32 required); -extern void ceph_messenger_destroy (struct ceph_messenger *); +extern void ceph_messenger_init (struct ceph_messenger *msgr, +struct ceph_entity_addr *myaddr, +u32 supported_features, +u32 required_features, +bool nocrc);extern void ceph_con_init (struct ceph_messenger *msgr,struct ceph_connection *con);
net/ceph/ceph_common.c +5 -13
@@ -468,19 +468,15 @@ struct ceph_client *ceph_create_client(struct ceph_options *opt, void *private,/* msgr */ if (ceph_test_opt (client,MYIP )) myaddr = &client->options->my_addr; - client->msgr =ceph_messenger_create (myaddr, - client->supported_features, - client->required_features); -if (IS_ERR (client->msgr)) { - err =PTR_ERR (client->msgr); - goto fail; - } - client->msgr->nocrc =ceph_test_opt (client,NOCRC ); +ceph_messenger_init (&client->msgr, myaddr, + client->supported_features, + client->required_features, +ceph_test_opt (client,NOCRC ));/* subsystems */ err =ceph_monc_init (&client->monc, client);if (err <0 ) - goto fail_msgr; + goto fail; err =ceph_osdc_init (&client->osdc, client);if (err <0 ) goto fail_monc;
@@ -489,8 +485,6 @@ struct ceph_client *ceph_create_client(struct ceph_options *opt, void *private, fail_monc:ceph_monc_stop (&client->monc); -fail_msgr: -ceph_messenger_destroy (client->msgr); fail:kfree (client);return ERR_PTR (err);
@@ -515,8 +509,6 @@ void ceph_destroy_client(struct ceph_client *client)ceph_debugfs_client_cleanup (client); -ceph_messenger_destroy (client->msgr); -ceph_destroy_options (client->options);kfree (client);
net/ceph/messenger.c +9 -21
@@ -2243,18 +2243,14 @@ out:/* -* create a new messenger instance +* initialize a new messenger instance */ -struct ceph_messenger *ceph_messenger_create (struct ceph_entity_addr *myaddr, -u32 supported_features, -u32 required_features) +void ceph_messenger_init (struct ceph_messenger *msgr, +struct ceph_entity_addr *myaddr, +u32 supported_features, +u32 required_features, +bool nocrc) { -struct ceph_messenger *msgr; - - msgr =kzalloc (sizeof (*msgr),GFP_KERNEL ); -if (msgr ==NULL ) -return ERR_PTR (-ENOMEM ); - msgr->supported_features = supported_features; msgr->required_features = required_features;
@@ -2267,19 +2263,11 @@ struct ceph_messenger *ceph_messenger_create(struct ceph_entity_addr *myaddr, msgr->inst.addr.type =0 ;get_random_bytes (&msgr->inst.addr.nonce,sizeof (msgr->inst.addr.nonce));encode_my_addr (msgr); + msgr->nocrc = nocrc; -dout ("messenger_create %p\n" , msgr); -return msgr; -} -EXPORT_SYMBOL (ceph_messenger_create); - -void ceph_messenger_destroy (struct ceph_messenger *msgr) -{ -dout ("destroy %p\n" , msgr); -kfree (msgr); -dout ("destroyed messenger %p\n" , msgr); +dout ("%s %p\n" , __func__, msgr); } -EXPORT_SYMBOL (ceph_messenger_destroy); +EXPORT_SYMBOL (ceph_messenger_init);static void clear_standby (struct ceph_connection *con) {
net/ceph/mon_client.c +3 -3
@@ -763,7 +763,7 @@ int ceph_monc_init(struct ceph_mon_client *monc, struct ceph_client *cl) monc->con =kmalloc (sizeof (*monc->con),GFP_KERNEL );if (!monc->con) goto out_monmap; -ceph_con_init (monc->client->msgr, monc->con); +ceph_con_init (&monc->client->msgr, monc->con); monc->con->private = monc; monc->con->ops = &mon_con_ops;
@@ -880,8 +880,8 @@ static void handle_auth_reply(struct ceph_mon_client *monc, }else if (!was_auth && monc->auth->ops->is_authenticated (monc->auth)) {dout ("authenticated, starting session\n" ); - monc->client->msgr->inst.name.type =CEPH_ENTITY_TYPE_CLIENT ; - monc->client->msgr->inst.name.num = + monc->client->msgr.inst.name.type =CEPH_ENTITY_TYPE_CLIENT ; + monc->client->msgr.inst.name.num =cpu_to_le64 (monc->auth->global_id);__send_subscribe (monc);
net/ceph/osd_client.c +2 -2
@@ -639,7 +639,7 @@ static struct ceph_osd *create_osd(struct ceph_osd_client *osdc)INIT_LIST_HEAD (&osd->o_osd_lru); osd->o_incarnation =1 ; -ceph_con_init (osdc->client->msgr, &osd->o_con); +ceph_con_init (&osdc->client->msgr, &osd->o_con); osd->o_con.private = osd; osd->o_con.ops = &osd_con_ops; osd->o_con.peer_name.type =CEPH_ENTITY_TYPE_OSD ;
@@ -1391,7 +1391,7 @@ void ceph_osdc_handle_map(struct ceph_osd_client *osdc, struct ceph_msg *msg) epoch, maplen); newmap =osdmap_apply_incremental (&p, next, osdc->osdmap, - osdc->client->msgr); + &osdc->client->msgr);if (IS_ERR (newmap)) { err =PTR_ERR (newmap); goto bad;
Signature · 1 line
1.7.5.4
PATCH 7/13 libceph: embed ceph connection structure in mon_client
In reply to Alex ElderA monitor client has a pointer to a ceph connection structure in it. This is the only one of the three ceph client types that do it this way; the OSD and MDS clients embed the connection into their main structures. There is always exactly one ceph connection for a monitor client, so there is no need to allocate it separate from the monitor client structure.
So switch the ceph_mon_client structure to embed its ceph_connection structure.
--- include/linux/ceph/mon_client.h | 2 +- net/ceph/mon_client.c | 47 ++++++++++++++++---------------------- 2 files changed, 21 insertions(+), 28 deletions(-)
include/linux/ceph/mon_client.h +1 -1
@@ -70,7 +70,7 @@ struct ceph_mon_client {bool hunting;int cur_mon;/* last monitor i contacted */ unsigned long sub_sent, sub_renew_after; -struct ceph_connection *con; +struct ceph_connection con;bool have_fsid;/* pending generic requests */
net/ceph/mon_client.c +20 -27
126 lines not on this page · raw message
Signature · 1 line
1.7.5.4
PATCH 8/13 libceph: start separating connection flags from state
In reply to Alex ElderA 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.
--- include/linux/ceph/messenger.h | 18 +++++++++---- net/ceph/messenger.c | 50 ++++++++++++++++++++-------------------- 2 files changed, 37 insertions(+), 31 deletions(-)
include/linux/ceph/messenger.h +2 -1
@@ -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
/*
* 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 */
net/ceph/messenger.c +25 -25
171 lines not on this page · raw message
Signature · 1 line
1.7.5.4
PATCH 9/13 libceph: start tracking connection socket state
In reply to Alex ElderStart explicitly keeping track of the state of a ceph connection's socket, separate from the state of the connection itself. Create placeholder functions to encapsulate the state transitions.
--------
| NEW* | transient initial state
--------
| con_sock_state_init()
v
----------
| CLOSED | initialized, but no socket (and no
---------- TCP connection)
^ \
| \ con_sock_state_connecting()
| ----------------------
| \
+ con_sock_state_closed() \
|\ \
| \ \
| ----------- \
| | CLOSING | socket event; \
| ----------- await close \
| ^ |
| | |
| + con_sock_state_closing() |
| / \ |
| / --------------- |
| / \ v
| / --------------
| / -----------------| CONNECTING | socket created, TCP
| | / -------------- connect initiated
| | | con_sock_state_connected()
| | v
-------------
| CONNECTED | TCP connection established
-------------Make the socket state an atomic variable, reinforcing that it's a distinct transtion with no possible "intermediate/both" states. This is almost certainly overkill at this point, though the transitions into CONNECTED and CLOSING state do get called via socket callback (the rest of the transitions occur with the connection mutex held). We can back out the atomicity later.
--- include/linux/ceph/messenger.h | 8 ++++- net/ceph/messenger.c | 63 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-)
include/linux/ceph/messenger.h +6 -2
@@ -137,14 +137,18 @@ struct ceph_connection {const struct ceph_connection_operations *ops;struct ceph_messenger *msgr; + +atomic_t sock_state;struct socket *sock; +struct ceph_entity_addr peer_addr;/* peer address */ +struct ceph_entity_addr peer_addr_for_me; +unsigned long flags;unsigned long state;const char *error_msg;/* error message, if any */ -struct ceph_entity_addr peer_addr;/* peer address */ struct ceph_entity_name peer_name;/* peer name */ -struct ceph_entity_addr peer_addr_for_me; +unsigned peer_features;u32 connect_seq;/* identify the most recent connection attempt for this connection, client */
net/ceph/messenger.c +63 -0
105 lines not on this page · raw message
Signature · 1 line
1.7.5.4
PATCH 10/13 libceph: provide osd number when creating osd
In reply to Alex ElderPass the osd number to the create_osd() routine, and move the initialization of fields that depend on it therein.
--- net/ceph/osd_client.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
net/ceph/osd_client.c +4 -4
@@ -624,7 +624,7 @@ static void osd_reset(struct ceph_connection *con)/* * Track open sessions with osds. */ -static struct ceph_osd *create_osd (struct ceph_osd_client *osdc) +static struct ceph_osd *create_osd (struct ceph_osd_client *osdc,int onum) {struct ceph_osd *osd;
@@ -634,6 +634,7 @@ static struct ceph_osd *create_osd(struct ceph_osd_client *osdc)atomic_set (&osd->o_ref,1 ); osd->o_osdc = osdc; + osd->o_osd = onum;INIT_LIST_HEAD (&osd->o_requests);INIT_LIST_HEAD (&osd->o_linger_requests);INIT_LIST_HEAD (&osd->o_osd_lru);
@@ -643,6 +644,7 @@ static struct ceph_osd *create_osd(struct ceph_osd_client *osdc) osd->o_con.private = osd; osd->o_con.ops = &osd_con_ops; osd->o_con.peer_name.type =CEPH_ENTITY_TYPE_OSD ; + osd->o_con.peer_name.num =cpu_to_le64 (onum);INIT_LIST_HEAD (&osd->o_keepalive_item);return osd;
@@ -998,15 +1000,13 @@ static int __map_request(struct ceph_osd_client *osdc, req->r_osd =__lookup_osd (osdc, o);if (!req->r_osd && o >=0 ) { err = -ENOMEM ; - req->r_osd =create_osd (osdc); + req->r_osd =create_osd (osdc, o);if (!req->r_osd) {list_move (&req->r_req_lru_item, &osdc->req_notarget); goto out; }dout ("map_request osd %p is osd%d\n" , req->r_osd, o); - req->r_osd->o_osd = o; - req->r_osd->o_con.peer_name.num =cpu_to_le64 (o);__insert_osd (osdc, req->r_osd);ceph_con_open (&req->r_osd->o_con, &osdc->osdmap->osd_addr[o]);
Signature · 1 line
1.7.5.4
PATCH 11/13 libceph: init monitor connection when opening
In reply to Alex ElderHold off initializing a monitor client's connection until just before it gets opened for use.
--- net/ceph/mon_client.c | 13 ++++++------- 1 files changed, 6 insertions(+), 7 deletions(-)
net/ceph/mon_client.c +6 -7
@@ -119,6 +119,7 @@ static void __close_session(struct ceph_mon_client *monc)dout ("__close_session closing mon%d\n" , monc->cur_mon);ceph_con_revoke (&monc->con, monc->m_auth);ceph_con_close (&monc->con); + monc->con.private =NULL ; monc->cur_mon =-1 ; monc->pending_auth =0 ;ceph_auth_reset (monc->auth);
@@ -141,9 +142,13 @@ static int __open_session(struct ceph_mon_client *monc) monc->sub_renew_after = jiffies;/* i.e., expired */ monc->want_next_osdmap = !!monc->want_next_osdmap; -dout ("open_session mon%d opening\n" , monc->cur_mon); +ceph_con_init (&monc->client->msgr, &monc->con); + monc->con.private = monc; + monc->con.ops = &mon_con_ops; monc->con.peer_name.type =CEPH_ENTITY_TYPE_MON ; monc->con.peer_name.num =cpu_to_le64 (monc->cur_mon); + +dout ("open_session mon%d opening\n" , monc->cur_mon);ceph_con_open (&monc->con, &monc->monmap->mon_inst[monc->cur_mon].addr);
@@ -760,10 +765,6 @@ int ceph_monc_init(struct ceph_mon_client *monc, struct ceph_client *cl) goto out;/* connection */ -ceph_con_init (&monc->client->msgr, &monc->con); - monc->con.private = monc; - monc->con.ops = &mon_con_ops; -/* authentication */ monc->auth =ceph_auth_init (cl->options->name, cl->options->key);
@@ -836,8 +837,6 @@ void ceph_monc_stop(struct ceph_mon_client *monc)mutex_lock (&monc->mutex);__close_session (monc); - monc->con.private =NULL ; -mutex_unlock (&monc->mutex);ceph_auth_destroy (monc->auth);
Signature · 1 line
1.7.5.4
PATCH 12/13 libceph: fully initialize connection in con_init()
In reply to Alex ElderMove the initialization of a ceph connection's private pointer, operations vector pointer, and peer name information into ceph_con_init(). Rearrange the arguments so the connection pointer is first. Hide the byte-swapping of the peer entity number inside ceph_con_init()
--- fs/ceph/mds_client.c | 7 ++----- include/linux/ceph/messenger.h | 6 ++++-- net/ceph/messenger.c | 9 ++++++++- net/ceph/mon_client.c | 8 +++----- net/ceph/osd_client.c | 7 ++----- 5 files changed, 19 insertions(+), 18 deletions(-)
fs/ceph/mds_client.c +2 -5
@@ -394,11 +394,8 @@ static struct ceph_mds_session *register_session(struct ceph_mds_client *mdsc, s->s_seq =0 ;mutex_init (&s->s_mutex); -ceph_con_init (&mdsc->fsc->client->msgr, &s->s_con); - s->s_con.private = s; - s->s_con.ops = &mds_con_ops; - s->s_con.peer_name.type =CEPH_ENTITY_TYPE_MDS ; - s->s_con.peer_name.num =cpu_to_le64 (mds); +ceph_con_init (&s->s_con, s, &mds_con_ops, &mdsc->fsc->client->msgr, +CEPH_ENTITY_TYPE_MDS , mds);spin_lock_init (&s->s_gen_ttl_lock); s->s_cap_gen =0 ;
include/linux/ceph/messenger.h +4 -2
@@ -227,8 +227,10 @@ extern void ceph_messenger_init(struct ceph_messenger *msgr,u32 required_features, bool nocrc); -extern void ceph_con_init (struct ceph_messenger *msgr, -struct ceph_connection *con); +extern void ceph_con_init (struct ceph_connection *con,void *private, +const struct ceph_connection_operations *ops, +struct ceph_messenger *msgr,__u8 entity_type, +__u64 entity_num);extern void ceph_con_open (struct ceph_connection *con,struct ceph_entity_addr *addr);extern bool ceph_con_opened (struct ceph_connection *con);
net/ceph/messenger.c +8 -1
@@ -514,15 +514,22 @@ void ceph_con_put(struct ceph_connection *con)/* * initialize a new connection. */ -void ceph_con_init (struct ceph_messenger *msgr,struct ceph_connection *con) +void ceph_con_init (struct ceph_connection *con,void *private, +const struct ceph_connection_operations *ops, +struct ceph_messenger *msgr,__u8 entity_type,__u64 entity_num) {dout ("con_init %p\n" , con);memset (con,0 ,sizeof (*con)); + con->private = private;atomic_set (&con->nref,1 ); + con->ops = ops; con->msgr = msgr;con_sock_state_init (con); + con->peer_name.type = (__u8 ) entity_type; + con->peer_name.num =cpu_to_le64 (entity_num); +mutex_init (&con->mutex);INIT_LIST_HEAD (&con->out_queue);INIT_LIST_HEAD (&con->out_sent);
net/ceph/mon_client.c +3 -5
@@ -142,11 +142,9 @@ static int __open_session(struct ceph_mon_client *monc) monc->sub_renew_after = jiffies;/* i.e., expired */ monc->want_next_osdmap = !!monc->want_next_osdmap; -ceph_con_init (&monc->client->msgr, &monc->con); - monc->con.private = monc; - monc->con.ops = &mon_con_ops; - monc->con.peer_name.type =CEPH_ENTITY_TYPE_MON ; - monc->con.peer_name.num =cpu_to_le64 (monc->cur_mon); +ceph_con_init (&monc->con, monc, &mon_con_ops, + &monc->client->msgr, +CEPH_ENTITY_TYPE_MON , monc->cur_mon);dout ("open_session mon%d opening\n" , monc->cur_mon); ceph_con_open(&monc->con,
net/ceph/osd_client.c +2 -5
@@ -640,11 +640,8 @@ static struct ceph_osd *create_osd(struct ceph_osd_client *osdc, int onum)INIT_LIST_HEAD (&osd->o_osd_lru); osd->o_incarnation =1 ; -ceph_con_init (&osdc->client->msgr, &osd->o_con); - osd->o_con.private = osd; - osd->o_con.ops = &osd_con_ops; - osd->o_con.peer_name.type =CEPH_ENTITY_TYPE_OSD ; - osd->o_con.peer_name.num =cpu_to_le64 (onum); +ceph_con_init (&osd->o_con, osd, &osd_con_ops, &osdc->client->msgr, +CEPH_ENTITY_TYPE_OSD , onum);INIT_LIST_HEAD (&osd->o_keepalive_item);return osd;
Signature · 1 line
1.7.5.4
PATCH 13/13 libceph: set CLOSED state bit in con_init
In reply to Alex ElderOnce a connection is fully initialized, it is really in a CLOSED state, so make that explicit by setting the bit in its state field.
It is possible for a connection in NEGOTIATING state to get a failure, leading to ceph_fault() and ultimately ceph_con_close(). Clear that bits if it is set in that case, to reflect that the connection truly is closed and is no longer participating in a connect sequence.
Issue a warning if ceph_con_open() is called on a connection that is not in CLOSED state.
--- net/ceph/messenger.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)
net/ceph/messenger.c +7 -1
@@ -452,10 +452,13 @@ void ceph_con_close(struct ceph_connection *con)dout ("con_close %p peer %s\n" , con,ceph_pr_addr (&con->peer_addr.in_addr));set_bit (CLOSED , &con->state);/* in case there's queued work */ +clear_bit (NEGOTIATING , &con->state);clear_bit (STANDBY , &con->state);/* avoid connect_seq bump */ +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 ;
@@ -472,7 +475,8 @@ void ceph_con_open(struct ceph_connection *con, struct ceph_entity_addr *addr) {dout ("con_open %p %s\n" , con,ceph_pr_addr (&addr->in_addr));set_bit (OPENING , &con->state); -clear_bit (CLOSED , &con->state); +WARN_ON (!test_and_clear_bit (CLOSED , &con->state)); +memcpy (&con->peer_addr, addr,sizeof (*addr)); con->delay =0 ;/* reset backoff memory */ queue_con (con);
@@ -534,6 +538,8 @@ void ceph_con_init(struct ceph_connection *con, void *private,INIT_LIST_HEAD (&con->out_queue);INIT_LIST_HEAD (&con->out_sent);INIT_DELAYED_WORK (&con->work, con_work); + +set_bit (CLOSED , &con->state); }EXPORT_SYMBOL (ceph_con_init);
Signature · 1 line
1.7.5.4
Re: [PATCH 01/13] libceph: eliminate connection state "DEAD"
In reply to Alex ElderQuoted 42 lines · Alex Elder
The ceph connection state "DEAD" is never set and is therefore not needed. Eliminate it.
Signed-off-by Alex Elder <[email protected]>---
include/linux/ceph/messenger.h | 1 -
net/ceph/messenger.c | 6 ------2 files changed, 0 insertions(+), 7 deletions(-)include/linux/ceph/messenger.h +0
-1@@ -119,7 +119,6 @@ struct ceph_msg_pos { #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 DEAD 14/* dead, about to kfree */ #define BACKOFF 15 /*net/ceph/messenger.c +0
-7@@ -2087,12 +2087,6 @@ bad_tag:*/ static void queue_con(struct ceph_connection *con) { - if (test_bit(DEAD, &con->state)) { - dout("queue_con %p ignoring: DEAD\n", - con); - return; - } - if (!con->ops->get(con)) { dout("queue_con %p ref count 0\n", con); return; --1.7.5.4
Quoted 4 lines
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/13] libceph: kill bad_proto ceph connection op
In reply to Alex ElderQuoted 40 lines · Alex Elder
No code sets a
bad_protomethod in its ceph connection operations vector, so just get rid of it.Signed-off-by Alex Elder <[email protected]>---
include/linux/ceph/messenger.h | 3 ---
net/ceph/messenger.c | 5 -----2 files changed, 0 insertions(+), 8 deletions(-)include/linux/ceph/messenger.h +0
-0@@ -31,9 +31,6 @@ struct ceph_connection_operations {int (
*verify_authorizer_reply) (structceph_connection*con, int len);
int (*invalidate_authorizer)(structceph_connection*con);- /* protocol version mismatch */
- void (*bad_proto) (structceph_connection*con);
-
/* there was some error on the socket (disconnect, whatever) */ void (*fault) (structceph_connection*con);net/ceph/messenger.c +0
-0@@ -1356,11 +1356,6 @@ static void fail_protocol(struct ceph_connection *con) { reset_connection(con);set_bit(CLOSED,
&con->state); /* in case there's queued work */ -
-mutex_unlock(&con->mutex);
- if (con->ops->bad_proto)
-con->ops->bad_proto(con);
-mutex_lock(&con->mutex);
}
Quoted 8 lines
static int
process_connect(struct ceph_connection *con)--
1.7.5.4--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/13] libceph: rename socket callbacks
In reply to Alex ElderQuoted 106 lines · Alex Elder
Change the names of the three socket callback functions to make it more obvious they're specifically associated with a connection's socket (not the ceph connection that uses it).
Signed-off-by Alex Elder <[email protected]>---
net/ceph/messenger.c | 28 ++++++++++++++--------------1 files changed, 14 insertions(+), 14 deletions(-)net/ceph/messenger.c +8
-8@@ -153,46 +153,46 @@ EXPORT_SYMBOL(ceph_msgr_flush);*/ /* data available on socket, or listen socket received a connect */ -static void ceph_data_ready (struct sock *sk,int count_unused) +static void ceph_sock_data_ready (struct sock *sk,int count_unused) { struct ceph_connection *con = sk->sk_user_data; if (sk->sk_state != TCP_CLOSE_WAIT) { - dout("ceph_data_ready on %p state = %lu, queueing work\n", + dout("%s on %p state = %lu, queueing work\n", __func__, con, con->state); queue_con(con); } } /* socket has buffer space for writing */ -static void ceph_write_space (struct sock *sk) +static void ceph_sock_write_space (struct sock *sk) { struct ceph_connection *con = sk->sk_user_data; /* only queue to workqueue if there is data we want to write, * and there is sufficient space in the socket buffer to accept - * more data. clear SOCK_NOSPACE so that ceph_write_space() + * more data. clear SOCK_NOSPACE so that ceph_sock_write_space() * doesn't get called again until try_write() fills the socket * 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 (sk_stream_wspace(sk) >= sk_stream_min_wspace(sk)) { - dout("ceph_write_space %p queueing write work\n", con); + dout("%s %p queueing write work\n", __func__, con); clear_bit(SOCK_NOSPACE, &sk->sk_socket->flags); queue_con(con); } } else { - dout("ceph_write_space %p nothing to write\n", con); + dout("%s %p nothing to write\n", __func__, con); } } /* socket's state has changed */ -static void ceph_state_change (struct sock *sk) +static void ceph_sock_state_change (struct sock *sk) { struct ceph_connection *con = sk->sk_user_data; - dout("ceph_state_change %p state = %lu sk_state = %u\n", + dout("%s %p state = %lu sk_state = %u\n", __func__, con, con->state, sk->sk_state);if (
test_bit(CLOSED, &con->state))net/ceph/messenger.c +0
-0@@ -200,9 +200,9 @@ static void ceph_state_change(struct sock *sk) switch (sk->sk_state) {case
TCP_CLOSE:
-dout("ceph_state_change TCP_CLOSE\n");
+dout("%s TCP_CLOSE\n", __func__);
caseTCP_CLOSE_WAIT:
-dout("ceph_state_change TCP_CLOSE_WAIT\n");
+dout("%s TCP_CLOSE_WAIT\n", __func__);
if (test_and_set_bit(SOCK_CLOSED, &con->state)== 0) { if (test_bit(CONNECTING, &con->state))con->error_msg= "connection failed";net/ceph/messenger.c +0
-0@@ -212,7 +212,7 @@ static void ceph_state_change(struct sock *sk) } break;case
TCP_ESTABLISHED:
-dout("ceph_state_change TCP_ESTABLISHED\n");
+dout("%s TCP_ESTABLISHED\n", __func__);queue_con(con);
break;
default: /* Everything else is uninteresting */net/ceph/messenger.c +3
-4@@ -228,9 +228,9 @@ static void set_sock_callbacks(struct socket *sock, { struct sock *sk = sock->sk; sk->sk_user_data = con; - sk->sk_data_ready = ceph_data_ready; - sk->sk_write_space = ceph_write_space; - sk->sk_state_change = ceph_state_change; + sk->sk_data_ready = ceph_sock_data_ready; + sk->sk_write_space = ceph_sock_write_space; + sk->sk_state_change = ceph_sock_state_change; } --1.7.5.4
Quoted 4 lines
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/13] libceph: rename kvec_reset and kvec_add functions
In reply to Alex ElderQuoted 190 lines · Alex Elder
The functions
ceph_con_out_kvec_reset()andceph_con_out_kvec_add()are entirely private functions, so drop the "ceph_" prefix in their name to make them slightly more wieldy.Signed-off-by Alex Elder <[email protected]>---
net/ceph/messenger.c | 48
++++++++++++++++++++++++------------------------1 files changed, 24 insertions(+), 24 deletions(-)net/ceph/messenger.c +2
-2@@ -484,14 +484,14 @@ static u32 get_global_seq(struct ceph_messenger *msgr, u32 gt) return ret; } -static void ceph_con_out_kvec_reset (struct ceph_connection *con) +static void con_out_kvec_reset (struct ceph_connection *con) { con->out_kvec_left = 0; con->out_kvec_bytes = 0; con->out_kvec_cur = &con->out_kvec[0]; } -static void ceph_con_out_kvec_add (struct ceph_connection *con, +static void con_out_kvec_add (struct ceph_connection *con, size_t size, void *data) {int index;
net/ceph/messenger.c +7
-7@@ -532,7 +532,7 @@ static void prepare_write_message(struct ceph_connection *con) struct ceph_msg *m; u32 crc; - ceph_con_out_kvec_reset(con); + con_out_kvec_reset(con); con->out_kvec_is_msg = true; con->out_msg_done = false;@@ -540,9 +540,9 @@ static void prepare_write_message(struct ceph_connection*con) * TCP packet that's a good thing. */ if (con->in_seq > con->in_seq_acked) { con->in_seq_acked = con->in_seq; - ceph_con_out_kvec_add(con, sizeof (tag_ack), &tag_ack); + con_out_kvec_add(con, sizeof (tag_ack), &tag_ack); con->out_temp_ack = cpu_to_le64(con->in_seq_acked); - ceph_con_out_kvec_add(con, sizeof (con->out_temp_ack), + con_out_kvec_add(con, sizeof (con->out_temp_ack), &con->out_temp_ack); }@@ -570,12 +570,12 @@ static void prepare_write_message(struct ceph_connection *con) BUG_ON(le32_to_cpu(m->hdr.front_len) != m->front.iov_len); /* tag + hdr + front + middle */ - ceph_con_out_kvec_add(con, sizeof (tag_msg), &tag_msg); - ceph_con_out_kvec_add(con, sizeof (m->hdr), &m->hdr); - ceph_con_out_kvec_add(con, m->front.iov_len, m->front.iov_base); + con_out_kvec_add(con, sizeof (tag_msg), &tag_msg); + con_out_kvec_add(con, sizeof (m->hdr), &m->hdr); + con_out_kvec_add(con, m->front.iov_len, m->front.iov_base); if (m->middle) - ceph_con_out_kvec_add(con, m->middle->vec.iov_len, + con_out_kvec_add(con, m->middle->vec.iov_len, m->middle->vec.iov_base);/* fill in crc (except data pages), footer */
net/ceph/messenger.c +3
-3@@ -624,12 +624,12 @@ static void prepare_write_ack(struct ceph_connection *con) con->in_seq_acked, con->in_seq); con->in_seq_acked = con->in_seq; - ceph_con_out_kvec_reset(con); + con_out_kvec_reset(con); - ceph_con_out_kvec_add(con, sizeof (tag_ack), &tag_ack); + con_out_kvec_add(con, sizeof (tag_ack), &tag_ack); con->out_temp_ack = cpu_to_le64(con->in_seq_acked); - ceph_con_out_kvec_add(con, sizeof (con->out_temp_ack), + con_out_kvec_add(con, sizeof (con->out_temp_ack), &con->out_temp_ack);con->out_more = 1; /* more will follow.. eventually.. */
net/ceph/messenger.c +4
-4@@ -642,8 +642,8 @@ static void prepare_write_ack(struct ceph_connection *con) static void prepare_write_keepalive(struct ceph_connection *con) { dout("prepare_write_keepalive %p\n", con); - ceph_con_out_kvec_reset(con); - ceph_con_out_kvec_add(con, sizeof (tag_keepalive), &tag_keepalive); + con_out_kvec_reset(con); + con_out_kvec_add(con, sizeof (tag_keepalive), &tag_keepalive); set_bit(WRITE_PENDING, &con->state); }@@ -688,8 +688,8 @@ static struct ceph_auth_handshake*get_connect_authorizer(struct ceph_connection */ static void prepare_write_banner(struct ceph_connection *con) { - ceph_con_out_kvec_add(con, strlen(CEPH_BANNER), CEPH_BANNER); - ceph_con_out_kvec_add(con, sizeof (con->msgr->my_enc_addr), + con_out_kvec_add(con, strlen(CEPH_BANNER), CEPH_BANNER); + con_out_kvec_add(con, sizeof (con->msgr->my_enc_addr), &con->msgr->my_enc_addr);con->out_more = 0;
net/ceph/messenger.c +0
-0@@ -736,10 +736,10 @@ static int prepare_write_connect(struct ceph_connection *con)con->out_connect.authorizer_len = auth ?
cpu_to_le32(auth->authorizer_buf_len): 0;- ceph_con_out_kvec_add(con, sizeof (
con->out_connect),
+con_out_kvec_add(con, sizeof (con->out_connect), &con->out_connect); if (auth &&auth->authorizer_buf_len)
- ceph_con_out_kvec_add(con,auth->authorizer_buf_len,
+con_out_kvec_add(con, auth->authorizer_buf_len, auth->authorizer_buf);con->out_more = 0;
net/ceph/messenger.c +1
-1@@ -933,7 +933,7 @@ static int write_partial_msg_pages(struct ceph_connection *con) /* prepare and queue up footer, too */ if (!do_datacrc) con->out_msg->footer.flags |= CEPH_MSG_FOOTER_NOCRC; - ceph_con_out_kvec_reset(con); + con_out_kvec_reset(con); prepare_write_message_footer(con); ret = 1;out:
net/ceph/messenger.c +1
-1@@ -1396,7 +1396,7 @@ static int process_connect(struct ceph_connection *con) return -1; } con->auth_retry = 1; - ceph_con_out_kvec_reset(con); + con_out_kvec_reset(con); ret = prepare_write_connect(con); if (ret < 0)return ret;
net/ceph/messenger.c +1
-1@@ -1417,7 +1417,7 @@ static int process_connect(struct ceph_connection *con) ENTITY_NAME(con->peer_name), ceph_pr_addr(&con->peer_addr.in_addr)); reset_connection(con); - ceph_con_out_kvec_reset(con); + con_out_kvec_reset(con); ret = prepare_write_connect(con); if (ret < 0)return ret;
net/ceph/messenger.c +1
-1@@ -1443,7 +1443,7 @@ static int process_connect(struct ceph_connection *con) le32_to_cpu(con->out_connect.connect_seq), le32_to_cpu(con->in_connect.connect_seq)); con->connect_seq = le32_to_cpu(con->in_connect.connect_seq); - ceph_con_out_kvec_reset(con); + con_out_kvec_reset(con); ret = prepare_write_connect(con); if (ret < 0)return ret;
net/ceph/messenger.c +1
-1@@ -1460,7 +1460,7 @@ static int process_connect(struct ceph_connection *con) le32_to_cpu(con->in_connect.global_seq)); get_global_seq(con->msgr, le32_to_cpu(con->in_connect.global_seq)); - ceph_con_out_kvec_reset(con); + con_out_kvec_reset(con); ret = prepare_write_connect(con); if (ret < 0)return ret;
net/ceph/messenger.c +1
-2@@ -1867,7 +1867,7 @@ more: /* open the socket first? */ if (con->sock == NULL) { - ceph_con_out_kvec_reset(con); + con_out_kvec_reset(con); prepare_write_banner(con); ret = prepare_write_connect(con); if (ret < 0) --1.7.5.4
Quoted 4 lines
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/13] libceph: embed ceph messenger structure in ceph_client
In reply to Alex ElderQuoted 227 lines · Alex Elder
A ceph client has a pointer to a ceph messenger structure in it. There is always exactly one ceph messenger for a ceph client, so there is no need to allocate it separate from the ceph client structure.
Switch the
ceph_clientstructure to embed itsceph_messengerstructure.Signed-off-by Alex Elder <[email protected]>---
fs/ceph/mds_client.c | 2 +-include/linux/ceph/libceph.h| 2 +-include/linux/ceph/messenger.h| 9 +++++----net/ceph/ceph_common.c| 18 +++++-------------net/ceph/messenger.c | 30 +++++++++--------------------- net/ceph/mon_client.c | 6 +++--- net/ceph/osd_client.c | 4 ++-- 7 files changed, 26 insertions(+), 45 deletions(-)fs/ceph/mds_client.c +1
-1@@ -394,7 +394,7 @@ static struct ceph_mds_session *register_session(structceph_mds_client *mdsc, s->s_seq = 0; mutex_init(&s->s_mutex); - ceph_con_init(mdsc->fsc->client->msgr, &s->s_con); + ceph_con_init(&mdsc->fsc->client->msgr, &s->s_con); s->s_con.private = s; s->s_con.ops = &mds_con_ops;s->s_con.peer_name.type =
CEPH_ENTITY_TYPE_MDS;include/linux/ceph/libceph.h +1
-1@@ -131,7 +131,7 @@ struct ceph_client { u32 supported_features; u32 required_features; - struct ceph_messenger *msgr; /* messenger instance */ + struct ceph_messenger msgr; /* messenger instance */ struct ceph_mon_client monc; struct ceph_osd_client osdc;include/linux/ceph/messenger.h +5
-4@@ -211,10 +211,11 @@ extern int ceph_msgr_init(void); extern void ceph_msgr_exit(void); extern void ceph_msgr_flush(void); -extern struct ceph_messenger *ceph_messenger_create ( - struct ceph_entity_addr *myaddr, - u32 features, u32 required); -extern void ceph_messenger_destroy (struct ceph_messenger *); +extern void ceph_messenger_init (struct ceph_messenger *msgr, + struct ceph_entity_addr *myaddr, + u32 supported_features, + u32 required_features, + bool nocrc); extern void ceph_con_init(struct ceph_messenger *msgr, struct ceph_connection *con);net/ceph/ceph_common.c +5
-9@@ -468,19 +468,15 @@ struct ceph_client *ceph_create_client(struct ceph_options *opt, void *private, /* msgr */ if (ceph_test_opt(client, MYIP)) myaddr = &client->options->my_addr; - client->msgr = ceph_messenger_create(myaddr, - client->supported_features, - client->required_features); - if (IS_ERR(client->msgr)) { - err = PTR_ERR(client->msgr); - goto fail; - } - client->msgr->nocrc = ceph_test_opt(client, NOCRC); + ceph_messenger_init(&client->msgr, myaddr, + client->supported_features, + client->required_features, + ceph_test_opt(client, NOCRC)); /* subsystems */ err = ceph_monc_init(&client->monc, client); if (err < 0) - goto fail_msgr; + goto fail; err = ceph_osdc_init(&client->osdc, client); if (err < 0)goto
fail_monc;net/ceph/ceph_common.c +0
-0@@ -489,8 +485,6 @@ struct ceph_client *ceph_create_client(struct ceph_options *opt, void *private,fail_monc:
ceph_monc_stop(&client->monc);
-fail_msgr:
-ceph_messenger_destroy(client->msgr);
fail:
kfree(client);
returnERR_PTR(err);net/ceph/ceph_common.c +0
-2@@ -515,8 +509,6 @@ void ceph_destroy_client(struct ceph_client *client) ceph_debugfs_client_cleanup(client); - ceph_messenger_destroy(client->msgr); - ceph_destroy_options(client->options); kfree(client);net/ceph/messenger.c +9
-21@@ -2243,18 +2243,14 @@ out: /* -* create a new messenger instance +* initialize a new messenger instance */ -struct ceph_messenger *ceph_messenger_create (struct ceph_entity_addr *myaddr, - u32 supported_features, - u32 required_features) +void ceph_messenger_init(struct ceph_messenger *msgr, + struct ceph_entity_addr *myaddr, + u32 supported_features, + u32 required_features, + bool nocrc) { - struct ceph_messenger *msgr; - - msgr = kzalloc(sizeof(*msgr), GFP_KERNEL); - if (msgr == NULL) - return ERR_PTR(-ENOMEM); - msgr->supported_features = supported_features; msgr->required_features = required_features;@@ -2267,19 +2263,11 @@ struct ceph_messenger *ceph_messenger_create(structceph_entity_addr *myaddr, msgr->inst.addr.type = 0; get_random_bytes(&msgr->inst.addr.nonce,sizeof (msgr->inst.addr.nonce)); encode_my_addr(msgr); + msgr->nocrc = nocrc; - dout("messenger_create %p\n", msgr); - return msgr; -} -EXPORT_SYMBOL (ceph_messenger_create); - -void ceph_messenger_destroy (struct ceph_messenger *msgr) -{ - dout("destroy %p\n", msgr); - kfree(msgr); - dout("destroyed messenger %p\n", msgr); + dout("%s %p\n", __func__, msgr); } -EXPORT_SYMBOL (ceph_messenger_destroy); +EXPORT_SYMBOL (ceph_messenger_init);static void
clear_standby(struct ceph_connection *con){net/ceph/mon_client.c +3
-3@@ -763,7 +763,7 @@ int ceph_monc_init(struct ceph_mon_client *monc, struct ceph_client *cl) monc->con = kmalloc(sizeof(*monc->con), GFP_KERNEL); if (!monc->con) goto out_monmap; - ceph_con_init(monc->client->msgr, monc->con); + ceph_con_init(&monc->client->msgr, monc->con); monc->con->private = monc; monc->con->ops = &mon_con_ops;@@ -880,8 +880,8 @@ static void handle_auth_reply(struct ceph_mon_client *monc, } else if (!was_auth && monc->auth->ops->is_authenticated (monc->auth)) { dout("authenticated, starting session\n"); - monc->client->msgr->inst.name.type =CEPH_ENTITY_TYPE_CLIENT ; - monc->client->msgr->inst.name.num = + monc->client->msgr.inst.name.type = CEPH_ENTITY_TYPE_CLIENT; + monc->client->msgr.inst.name.num =cpu_to_le64(monc->auth->global_id);
__send_subscribe(monc);
net/ceph/osd_client.c +1
-1@@ -639,7 +639,7 @@ static struct ceph_osd *create_osd(struct ceph_osd_client *osdc) INIT_LIST_HEAD(&osd->o_osd_lru); osd->o_incarnation = 1; - ceph_con_init(osdc->client->msgr, &osd->o_con); + ceph_con_init(&osdc->client->msgr, &osd->o_con); osd->o_con.private = osd; osd->o_con.ops = &osd_con_ops;osd->o_con.peer_name.type =
CEPH_ENTITY_TYPE_OSD;net/ceph/osd_client.c +1
-1@@ -1391,7 +1391,7 @@ void ceph_osdc_handle_map(struct ceph_osd_client *osdc, struct ceph_msg *msg) epoch, maplen); newmap = osdmap_apply_incremental(&p, next, osdc->osdmap, - osdc->client->msgr); + &osdc->client->msgr);if (
IS_ERR(newmap)) { err =PTR_ERR(newmap); goto bad;
--
1.7.5.4
Quoted 4 lines
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/13] libceph: rename socket callbacks
In reply to Alex ElderQuoted 15 lines · Alex Elder
Change the names of the three socket callback functions to make it more obvious they're specifically associated with a connection's socket (not the ceph connection that uses it).
Signed-off-by Alex Elder <[email protected]>--- net/ceph/messenger.c | 28 ++++++++++++++-------------- 1 files changed, 14 insertions(+), 14 deletions(-)net/ceph/messenger.c +0
-0@@ -153,46 +153,46 @@ EXPORT_SYMBOL(ceph_msgr_flush);*/
net/ceph/messenger.c EXPORT_SYMBOL(ceph_msgr_flush); · line 161
Quoted 10 lines
net/ceph/messenger.c +2
-2/* data available on socket, or listen socket received a connect */ -static void ceph_data_ready (struct sock *sk,int count_unused) +static void ceph_sock_data_ready (struct sock *sk,int count_unused) {struct ceph_connection *con = sk->sk_user_data;if (sk->sk_state !=TCP_CLOSE_WAIT ) { - dout("ceph_data_ready on %p state = %lu, queueing work\n", + dout("%s on %p state = %lu, queueing work\n", __func__,
I think it's marginally better to do
dout(__func__ " on %p state = %lu, queueing work\n",
so that the concatenation happens at compile-time instead of runtime.
Otherwise, looks good!
Quoted 87 lines
net/ceph/messenger.c +6
-6con, con->state);queue_con (con); } }/* socket has buffer space for writing */ -static void ceph_write_space (struct sock *sk) +static void ceph_sock_write_space (struct sock *sk) {struct ceph_connection *con = sk->sk_user_data;/* only queue to workqueue if there is data we want to write, * and there is sufficient space in the socket buffer to accept -* more data. clear SOCK_NOSPACE so that ceph_write_space() +* more data. clear SOCK_NOSPACE so that ceph_sock_write_space() * doesn't get called again until try_write() fills the socket * 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 (sk_stream_wspace (sk) >=sk_stream_min_wspace (sk)) { -dout ("ceph_write_space %p queueing write work\n" , con); +dout ("%s %p queueing write work\n" , __func__, con);clear_bit (SOCK_NOSPACE , &sk->sk_socket->flags);queue_con (con); } }else { -dout ("ceph_write_space %p nothing to write\n" , con); +dout ("%s %p nothing to write\n" , __func__, con); } }/* socket's state has changed */ -static void ceph_state_change (struct sock *sk) +static void ceph_sock_state_change (struct sock *sk) {struct ceph_connection *con = sk->sk_user_data; -dout ("ceph_state_change %p state = %lu sk_state = %u\n" , +dout ("%s %p state = %lu sk_state = %u\n" , __func__, con, con->state, sk->sk_state);net/ceph/messenger.c +6
-6if (test_bit (CLOSED , &con->state))@@ -200,9 +200,9 @@ static void ceph_state_change(struct sock *sk)switch (sk->sk_state) {case TCP_CLOSE : -dout ("ceph_state_change TCP_CLOSE\n" ); +dout ("%s TCP_CLOSE\n" , __func__);case TCP_CLOSE_WAIT : -dout ("ceph_state_change TCP_CLOSE_WAIT\n" ); +dout ("%s TCP_CLOSE_WAIT\n" , __func__);if (test_and_set_bit (SOCK_CLOSED , &con->state) ==0 ) {if (test_bit (CONNECTING , &con->state)) con->error_msg ="connection failed" ;@@ -212,7 +212,7 @@ static void ceph_state_change(struct sock *sk) }break ;case TCP_ESTABLISHED : -dout ("ceph_state_change TCP_ESTABLISHED\n" ); +dout ("%s TCP_ESTABLISHED\n" , __func__);queue_con (con);break ;default :/* Everything else is uninteresting */ @@ -228,9 +228,9 @@ static void set_sock_callbacks(struct socket *sock, {struct sock *sk = sock->sk; sk->sk_user_data = con; - sk->sk_data_ready = ceph_data_ready; - sk->sk_write_space = ceph_write_space; - sk->sk_state_change = ceph_state_change; + sk->sk_data_ready = ceph_sock_data_ready; + sk->sk_write_space = ceph_sock_write_space; + sk->sk_state_change = ceph_sock_state_change; }Signature · 6 lines
1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/13] libceph: rename kvec_reset and kvec_add functions
In reply to Alex ElderYep
Quoted 190 lines · Alex Elder
The functions
ceph_con_out_kvec_reset()andceph_con_out_kvec_add()are entirely private functions, so drop the "ceph_" prefix in their name to make them slightly more wieldy.Signed-off-by Alex Elder <[email protected]>--- net/ceph/messenger.c | 48 ++++++++++++++++++++++++------------------------ 1 files changed, 24 insertions(+), 24 deletions(-)net/ceph/messenger.c +2
-2@@ -484,14 +484,14 @@ static u32 get_global_seq(struct ceph_messenger *msgr, u32 gt)return ret; } -static void ceph_con_out_kvec_reset (struct ceph_connection *con) +static void con_out_kvec_reset (struct ceph_connection *con) { con->out_kvec_left =0 ; con->out_kvec_bytes =0 ; con->out_kvec_cur = &con->out_kvec[0 ]; } -static void ceph_con_out_kvec_add (struct ceph_connection *con, +static void con_out_kvec_add (struct ceph_connection *con,size_t size,void *data) {net/ceph/messenger.c +7
-7int index;@@ -532,7 +532,7 @@ static void prepare_write_message(struct ceph_connection *con)struct ceph_msg *m;u32 crc; -ceph_con_out_kvec_reset (con); +con_out_kvec_reset (con); con->out_kvec_is_msg = true; con->out_msg_done = false;@@ -540,9 +540,9 @@ static void prepare_write_message(struct ceph_connection*con) * TCP packet that's a good thing. */ if (con->in_seq > con->in_seq_acked) { con->in_seq_acked = con->in_seq; -ceph_con_out_kvec_add (con,sizeof (tag_ack), &tag_ack); +con_out_kvec_add (con,sizeof (tag_ack), &tag_ack); con->out_temp_ack =cpu_to_le64 (con->in_seq_acked); -ceph_con_out_kvec_add (con,sizeof (con->out_temp_ack), +con_out_kvec_add (con,sizeof (con->out_temp_ack), &con->out_temp_ack); }@@ -570,12 +570,12 @@ static void prepare_write_message(struct ceph_connection *con)BUG_ON (le32_to_cpu (m->hdr.front_len) != m->front.iov_len);/* tag + hdr + front + middle */ -ceph_con_out_kvec_add (con,sizeof (tag_msg), &tag_msg); -ceph_con_out_kvec_add (con,sizeof (m->hdr), &m->hdr); -ceph_con_out_kvec_add (con, m->front.iov_len, m->front.iov_base); +con_out_kvec_add (con,sizeof (tag_msg), &tag_msg); +con_out_kvec_add (con,sizeof (m->hdr), &m->hdr); +con_out_kvec_add (con, m->front.iov_len, m->front.iov_base);if (m->middle) -ceph_con_out_kvec_add (con, m->middle->vec.iov_len, +con_out_kvec_add (con, m->middle->vec.iov_len, m->middle->vec.iov_base);net/ceph/messenger.c +3
-3/* fill in crc (except data pages), footer */ @@ -624,12 +624,12 @@ static void prepare_write_ack(struct ceph_connection *con) con->in_seq_acked, con->in_seq); con->in_seq_acked = con->in_seq; -ceph_con_out_kvec_reset (con); +con_out_kvec_reset (con); -ceph_con_out_kvec_add (con,sizeof (tag_ack), &tag_ack); +con_out_kvec_add (con,sizeof (tag_ack), &tag_ack); con->out_temp_ack =cpu_to_le64 (con->in_seq_acked); -ceph_con_out_kvec_add (con,sizeof (con->out_temp_ack), +con_out_kvec_add (con,sizeof (con->out_temp_ack), &con->out_temp_ack);net/ceph/messenger.c +4
-4con->out_more =1 ;/* more will follow.. eventually.. */ @@ -642,8 +642,8 @@ static void prepare_write_ack(struct ceph_connection *con)static void prepare_write_keepalive (struct ceph_connection *con) {dout ("prepare_write_keepalive %p\n" , con); -ceph_con_out_kvec_reset (con); -ceph_con_out_kvec_add (con,sizeof (tag_keepalive), &tag_keepalive); +con_out_kvec_reset (con); +con_out_kvec_add (con,sizeof (tag_keepalive), &tag_keepalive);set_bit (WRITE_PENDING , &con->state); }@@ -688,8 +688,8 @@ static struct ceph_auth_handshake*get_connect_authorizer(struct ceph_connection */ static void prepare_write_banner (struct ceph_connection *con) { -ceph_con_out_kvec_add (con,strlen (CEPH_BANNER ),CEPH_BANNER ); -ceph_con_out_kvec_add (con,sizeof (con->msgr->my_enc_addr), +con_out_kvec_add (con,strlen (CEPH_BANNER ),CEPH_BANNER ); +con_out_kvec_add (con,sizeof (con->msgr->my_enc_addr), &con->msgr->my_enc_addr);net/ceph/messenger.c +2
-2con->out_more =0 ;@@ -736,10 +736,10 @@ static int prepare_write_connect(struct ceph_connection *con) con->out_connect.authorizer_len = auth ?cpu_to_le32 (auth->authorizer_buf_len) :0 ; -ceph_con_out_kvec_add (con,sizeof (con->out_connect), +con_out_kvec_add (con,sizeof (con->out_connect), &con->out_connect);if (auth && auth->authorizer_buf_len) -ceph_con_out_kvec_add (con, auth->authorizer_buf_len, +con_out_kvec_add (con, auth->authorizer_buf_len, auth->authorizer_buf);net/ceph/messenger.c +1
-1con->out_more =0 ;@@ -933,7 +933,7 @@ static int write_partial_msg_pages(struct ceph_connection *con)/* prepare and queue up footer, too */ if (!do_datacrc) con->out_msg->footer.flags |=CEPH_MSG_FOOTER_NOCRC ; -ceph_con_out_kvec_reset (con); +con_out_kvec_reset (con);prepare_write_message_footer (con); ret =1 ;out:
net/ceph/messenger.c +5
-5@@ -1396,7 +1396,7 @@ static int process_connect(struct ceph_connection *con)return -1 ; } con->auth_retry =1 ; -ceph_con_out_kvec_reset (con); +con_out_kvec_reset (con); ret =prepare_write_connect (con);if (ret <0 )return ret;@@ -1417,7 +1417,7 @@ static int process_connect(struct ceph_connection *con)ENTITY_NAME (con->peer_name), ceph_pr_addr(&con->peer_addr.in_addr));reset_connection (con); -ceph_con_out_kvec_reset (con); +con_out_kvec_reset (con); ret =prepare_write_connect (con);if (ret <0 )return ret;@@ -1443,7 +1443,7 @@ static int process_connect(struct ceph_connection *con)le32_to_cpu (con->out_connect.connect_seq), le32_to_cpu(con->in_connect.connect_seq)); con->connect_seq =le32_to_cpu (con->in_connect.connect_seq); -ceph_con_out_kvec_reset (con); +con_out_kvec_reset (con); ret =prepare_write_connect (con);if (ret <0 )return ret;@@ -1460,7 +1460,7 @@ static int process_connect(struct ceph_connection *con) le32_to_cpu(con->in_connect.global_seq));get_global_seq (con->msgr,le32_to_cpu (con->in_connect.global_seq)); -ceph_con_out_kvec_reset (con); +con_out_kvec_reset (con); ret =prepare_write_connect (con);if (ret <0 )return ret;@@ -1867,7 +1867,7 @@ more:/* open the socket first? */ if (con->sock ==NULL ) { -ceph_con_out_kvec_reset (con); +con_out_kvec_reset (con);prepare_write_banner (con); ret =prepare_write_connect (con);if (ret <0 )Signature · 6 lines
1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/13] libceph: embed ceph messenger structure in ceph_client
In reply to Alex ElderQuoted 230 lines · Alex Elder
A ceph client has a pointer to a ceph messenger structure in it. There is always exactly one ceph messenger for a ceph client, so there is no need to allocate it separate from the ceph client structure.
Switch the
ceph_clientstructure to embed itsceph_messengerstructure.Signed-off-by Alex Elder <[email protected]>--- fs/ceph/mds_client.c | 2 +- include/linux/ceph/libceph.h | 2 +- include/linux/ceph/messenger.h | 9 +++++---- net/ceph/ceph_common.c | 18 +++++------------- net/ceph/messenger.c | 30 +++++++++--------------------- net/ceph/mon_client.c | 6 +++--- net/ceph/osd_client.c | 4 ++-- 7 files changed, 26 insertions(+), 45 deletions(-)fs/ceph/mds_client.c +1
-1@@ -394,7 +394,7 @@ static struct ceph_mds_session *register_session(struct ceph_mds_client *mdsc, s->s_seq =0 ;mutex_init (&s->s_mutex); -ceph_con_init (mdsc->fsc->client->msgr, &s->s_con); +ceph_con_init (&mdsc->fsc->client->msgr, &s->s_con); s->s_con.private = s; s->s_con.ops = &mds_con_ops;fs/ceph/mds_client.c +0
-0s->s_con.peer_name.type =CEPH_ENTITY_TYPE_MDS ;include/linux/ceph/libceph.h +1
-1@@ -131,7 +131,7 @@ struct ceph_client {u32 supported_features;u32 required_features; -struct ceph_messenger *msgr;/* messenger instance */ +struct ceph_messenger msgr;/* messenger instance */ struct ceph_mon_client monc;struct ceph_osd_client osdc;include/linux/ceph/messenger.h +5
-4@@ -211,10 +211,11 @@ extern int ceph_msgr_init(void);extern void ceph_msgr_exit (void );extern void ceph_msgr_flush (void ); -extern struct ceph_messenger *ceph_messenger_create ( -struct ceph_entity_addr *myaddr, -u32 features,u32 required); -extern void ceph_messenger_destroy (struct ceph_messenger *); +extern void ceph_messenger_init (struct ceph_messenger *msgr, +struct ceph_entity_addr *myaddr, +u32 supported_features, +u32 required_features, +bool nocrc);extern void ceph_con_init (struct ceph_messenger *msgr,struct ceph_connection *con);net/ceph/ceph_common.c +5
-9@@ -468,19 +468,15 @@ struct ceph_client *ceph_create_client(struct ceph_options *opt, void *private,/* msgr */ if (ceph_test_opt(client, MYIP)) myaddr = &client->options->my_addr; - client->msgr =ceph_messenger_create (myaddr, - client->supported_features, - client->required_features); -if (IS_ERR (client->msgr)) { - err =PTR_ERR (client->msgr); - goto fail; - } - client->msgr->nocrc =ceph_test_opt (client,NOCRC ); +ceph_messenger_init (&client->msgr, myaddr, + client->supported_features, + client->required_features, +ceph_test_opt (client,NOCRC ));/* subsystems */ err =ceph_monc_init (&client->monc, client);if (err <0 ) - goto fail_msgr; + goto fail; err =ceph_osdc_init (&client->osdc, client);if (err <0 )net/ceph/ceph_common.c +0
-2goto fail_monc;@@ -489,8 +485,6 @@ struct ceph_client *ceph_create_client(struct ceph_options *opt, void *private, fail_monc:ceph_monc_stop (&client->monc); -fail_msgr: -ceph_messenger_destroy (client->msgr); fail:kfree (client);net/ceph/ceph_common.c +0
-2return ERR_PTR (err);@@ -515,8 +509,6 @@ void ceph_destroy_client(struct ceph_client *client)ceph_debugfs_client_cleanup (client); -ceph_messenger_destroy (client->msgr); -ceph_destroy_options (client->options);kfree (client);net/ceph/messenger.c +9
-21@@ -2243,18 +2243,14 @@ out:/* -* create a new messenger instance +* initialize a new messenger instance */ -struct ceph_messenger *ceph_messenger_create (struct ceph_entity_addr *myaddr, -u32 supported_features, -u32 required_features) +void ceph_messenger_init (struct ceph_messenger *msgr, +struct ceph_entity_addr *myaddr, +u32 supported_features, +u32 required_features, +bool nocrc) { -struct ceph_messenger *msgr; - - msgr =kzalloc (sizeof (*msgr),GFP_KERNEL ); -if (msgr ==NULL ) -return ERR_PTR (-ENOMEM ); - msgr->supported_features = supported_features; msgr->required_features = required_features;@@ -2267,19 +2263,11 @@ struct ceph_messenger *ceph_messenger_create(struct ceph_entity_addr *myaddr, msgr->inst.addr.type =0 ;get_random_bytes (&msgr->inst.addr.nonce,sizeof (msgr->inst.addr.nonce));encode_my_addr (msgr); + msgr->nocrc = nocrc; -dout ("messenger_create %p\n" , msgr); -return msgr; -} -EXPORT_SYMBOL (ceph_messenger_create); - -void ceph_messenger_destroy (struct ceph_messenger *msgr) -{ -dout ("destroy %p\n" , msgr); -kfree (msgr); -dout ("destroyed messenger %p\n" , msgr); +dout ("%s %p\n" , __func__, msgr); } -EXPORT_SYMBOL (ceph_messenger_destroy); +EXPORT_SYMBOL (ceph_messenger_init);static void
clear_standby(struct ceph_connection *con){net/ceph/mon_client.c +3
-3@@ -763,7 +763,7 @@ int ceph_monc_init(struct ceph_mon_client *monc, struct ceph_client *cl) monc->con = kmalloc(sizeof(*monc->con), GFP_KERNEL);if (!monc->con) goto out_monmap; -ceph_con_init (monc->client->msgr, monc->con); +ceph_con_init (&monc->client->msgr, monc->con); monc->con->private = monc; monc->con->ops = &mon_con_ops;@@ -880,8 +880,8 @@ static void handle_auth_reply(struct ceph_mon_client *monc, }else if (!was_auth && monc->auth->ops->is_authenticated (monc->auth)) {dout ("authenticated, starting session\n" ); - monc->client->msgr->inst.name.type =CEPH_ENTITY_TYPE_CLIENT ; - monc->client->msgr->inst.name.num = + monc->client->msgr.inst.name.type =CEPH_ENTITY_TYPE_CLIENT ; + monc->client->msgr.inst.name.num =cpu_to_le64 (monc->auth->global_id);net/ceph/mon_client.c +0
-0__send_subscribe (monc);net/ceph/osd_client.c +1
-1@@ -639,7 +639,7 @@ static struct ceph_osd *create_osd(struct ceph_osd_client *osdc)INIT_LIST_HEAD (&osd->o_osd_lru); osd->o_incarnation =1 ; -ceph_con_init (osdc->client->msgr, &osd->o_con); +ceph_con_init (&osdc->client->msgr, &osd->o_con); osd->o_con.private = osd; osd->o_con.ops = &osd_con_ops;net/ceph/osd_client.c +1
-1osd->o_con.peer_name.type =CEPH_ENTITY_TYPE_OSD ;@@ -1391,7 +1391,7 @@ void ceph_osdc_handle_map(struct ceph_osd_client *osdc, struct ceph_msg *msg) epoch, maplen); newmap =osdmap_apply_incremental (&p, next, osdc->osdmap, - osdc->client->msgr); + &osdc->client->msgr);if (IS_ERR (newmap)) {net/ceph/osd_client.c +0
-0err =PTR_ERR (newmap); goto bad;Signature · 6 lines
1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/13] libceph: embed ceph connection structure in mon_client
In reply to Alex ElderQuoted 149 lines · Alex Elder
A monitor client has a pointer to a ceph connection structure in it. This is the only one of the three ceph client types that do it this way; the OSD and MDS clients embed the connection into their main structures. There is always exactly one ceph connection for a monitor client, so there is no need to allocate it separate from the monitor client structure.
So switch the
ceph_mon_clientstructure to embed itsceph_connectionstructure.Signed-off-by Alex Elder <[email protected]>--- include/linux/ceph/mon_client.h | 2 +- net/ceph/mon_client.c | 47 ++++++++++++++++---------------------- 2 files changed, 21 insertions(+), 28 deletions(-)include/linux/ceph/mon_client.h +1
-1@@ -70,7 +70,7 @@ struct ceph_mon_client {bool hunting;int cur_mon;/* last monitor i contacted */ unsigned long sub_sent, sub_renew_after; -struct ceph_connection *con; +struct ceph_connection con;bool have_fsid;/* pending generic requests */ net/ceph/mon_client.c +2
-2@@ -106,9 +106,9 @@ static void __send_prepared_auth_request(struct ceph_mon_client *monc, int len) monc->pending_auth = 1; monc->m_auth->front.iov_len = len; monc->m_auth->hdr.front_len =cpu_to_le32 (len); -ceph_con_revoke (monc->con, monc->m_auth); +ceph_con_revoke (&monc->con, monc->m_auth);ceph_msg_get (monc->m_auth);/* keep our ref */ -ceph_con_send (monc->con, monc->m_auth); +ceph_con_send (&monc->con, monc->m_auth); }/*
net/ceph/mon_client.c +2
-2@@ -117,8 +117,8 @@ static void __send_prepared_auth_request(struct ceph_mon_client *monc, int len)static void __close_session (struct ceph_mon_client *monc) {dout ("__close_session closing mon%d\n" , monc->cur_mon); -ceph_con_revoke (monc->con, monc->m_auth); -ceph_con_close (monc->con); +ceph_con_revoke (&monc->con, monc->m_auth); +ceph_con_close (&monc->con); monc->cur_mon =-1 ; monc->pending_auth =0 ;net/ceph/mon_client.c +6
-6ceph_auth_reset (monc->auth);@@ -142,9 +142,9 @@ static int __open_session(struct ceph_mon_client *monc) monc->want_next_osdmap = !!monc->want_next_osdmap;dout ("open_session mon%d opening\n" , monc->cur_mon); - monc->con->peer_name.type =CEPH_ENTITY_TYPE_MON ; - monc->con->peer_name.num =cpu_to_le64 (monc->cur_mon); -ceph_con_open (monc->con, + monc->con.peer_name.type =CEPH_ENTITY_TYPE_MON ; + monc->con.peer_name.num =cpu_to_le64 (monc->cur_mon); +ceph_con_open (&monc->con, &monc->monmap->mon_inst[monc->cur_mon].addr);/* initiatiate authentication handshake */ @@ -226,8 +226,8 @@ static void __send_subscribe(struct ceph_mon_client *monc) msg->front.iov_len = p - msg->front.iov_base; msg->hdr.front_len =cpu_to_le32 (msg->front.iov_len); -ceph_con_revoke (monc->con, msg); -ceph_con_send (monc->con,ceph_msg_get (msg)); +ceph_con_revoke (&monc->con, msg); +ceph_con_send (&monc->con,ceph_msg_get (msg)); monc->sub_sent = jiffies |1 ;/* never 0 */ }@@ -247,7 +247,7 @@ static void handle_subscribe_ack(struct ceph_mon_client *monc, if (monc->hunting) {pr_info ("mon%d %s session established\n" , monc->cur_mon, -ceph_pr_addr (&monc->con->peer_addr.in_addr)); +ceph_pr_addr (&monc->con.peer_addr.in_addr)); monc->hunting = false; }net/ceph/mon_client.c +1
-1dout ("handle_subscribe_ack after %d seconds\n" , seconds);@@ -461,7 +461,7 @@ static int do_generic_request(struct ceph_mon_client *monc, req->request->hdr.tid =cpu_to_le64 (req->tid);__insert_generic_request (monc, req); monc->num_generic_requests++; -ceph_con_send (monc->con,ceph_msg_get (req->request)); +ceph_con_send (&monc->con,ceph_msg_get (req->request));mutex_unlock (&monc->mutex);net/ceph/mon_client.c +7
-10err =wait_for_completion_interruptible (&req->completion);@@ -684,8 +684,8 @@ static void __resend_generic_request(struct ceph_mon_client *monc)for (p =rb_first (&monc->generic_request_tree); p; p =rb_next (p)) { req = rb_entry(p, struct ceph_mon_generic_request, node); -ceph_con_revoke (monc->con, req->request); -ceph_con_send (monc->con,ceph_msg_get (req->request)); +ceph_con_revoke (&monc->con, req->request); +ceph_con_send (&monc->con,ceph_msg_get (req->request)); } }@@ -705,7 +705,7 @@ static void delayed_work(struct work_struct *work)__close_session (monc);__open_session (monc);/* continue hunting */ }else { -ceph_con_keepalive (monc->con); +ceph_con_keepalive (&monc->con);__validate_auth (monc);@@ -760,19 +760,16 @@ int ceph_monc_init(struct ceph_mon_client *monc, struct ceph_client *cl) goto out;/* connection */ - monc->con =kmalloc (sizeof (*monc->con),GFP_KERNEL ); -if (!monc->con) - goto out_monmap; -ceph_con_init (&monc->client->msgr, monc->con); - monc->con->private = monc; - monc->con->ops = &mon_con_ops; +ceph_con_init (&monc->client->msgr, &monc->con); + monc->con.private = monc; + monc->con.ops = &mon_con_ops;/* authentication */ monc->auth =ceph_auth_init (cl->options->name, cl->options->key);if (IS_ERR (monc->auth)) { err =PTR_ERR (monc->auth); - goto out_con; + goto out_monmap; } monc->auth->want_keys =net/ceph/mon_client.c +0
-0CEPH_ENTITY_TYPE_AUTH | CEPH_ENTITY_TYPE_MON |
net/ceph/mon_client.c out_subscribe_ack: · line 828
Quoted 5 lines
net/ceph/mon_client.c +0
-2@@ -824,8 +821,6 @@ out_subscribe_ack:ceph_msg_put (monc->m_subscribe_ack); out_auth:ceph_auth_destroy (monc->auth); -out_con: - monc->con->ops->put (monc->con);
AH!
This reminds me, these connections need to be refcounted. There's a ->get() and ->put() op defined so that you can refcount the containing structure. That means that this patch needs to alo change
static const struct ceph_connection_operations mon_con_ops = {
.get = ceph_con_get,
.put = ceph_con_put,
in mon_client.c. Hopefully the mon_client itself is refcounted, *or* we can ensure that it won't go away before the msgr workqueue is drained and the get/put ops can turn to no-ops.
Also: when poking around, I noticed that ceph_con_get() and put() are called directly from osd_client.c... that's a bug! Those connections have a get and put op defined that twiddles the containing ceph_osd struct's ref count.
I pushed several patches to your latest (wip-messenger-2) branch that fix these issues. Compile tested only! The first should probably be folded into this one, the others follow.
Quoted 32 lines
net/ceph/mon_client.c +2
-4out_monmap:kfree (monc->monmap); out:@@ -841,9 +836,7 @@ void ceph_monc_stop(struct ceph_mon_client *monc)mutex_lock (&monc->mutex);__close_session (monc); - monc->con->private =NULL ; - monc->con->ops->put (monc->con); - monc->con =NULL ; + monc->con.private =NULL ;mutex_unlock (&monc->mutex);@@ -1021,7 +1014,7 @@ static void mon_fault(struct ceph_connection *con)if (!monc->hunting)pr_info ("mon%d %s session lost, " "hunting for new mon\n" , monc->cur_mon, -ceph_pr_addr (&monc->con->peer_addr.in_addr)); +ceph_pr_addr (&monc->con.peer_addr.in_addr));__close_session (monc);if (!monc->hunting) {Signature · 6 lines
1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/13] libceph: start separating connection flags from state
In reply to Alex ElderQuoted 40 lines · Alex Elder
A
ceph_connectionholds 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 <[email protected]>--- include/linux/ceph/messenger.h | 18 +++++++++---- net/ceph/messenger.c | 50 ++++++++++++++++++++-------------------- 2 files changed, 37 insertions(+), 31 deletions(-)include/linux/ceph/messenger.h +10
-3@@ -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
include/linux/ceph/messenger.h
Quoted 6 lines
include/linux/ceph/messenger.h +0
-2* 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_.
Quoted 223 lines
/*
* A single connection with another host.include/linux/ceph/messenger.h +2
-1@@ -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 */ net/ceph/messenger.c +6
-6@@ -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); }/*
net/ceph/messenger.c +2
-2@@ -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); }/*
net/ceph/messenger.c +1
-1@@ -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);net/ceph/messenger.c +1
-1con->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)net/ceph/messenger.c +1
-1@@ -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 ;}
net/ceph/messenger.c +8
-8@@ -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 {net/ceph/messenger.c +4
-4/* 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);net/ceph/messenger.c +2
-2@@ -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);Signature · 6 lines
1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/13] libceph: start tracking connection socket state
In reply to Alex ElderQuoted 28 lines · Alex Elder
Start explicitly keeping track of the state of a ceph connection's socket, separate from the state of the connection itself. Create placeholder functions to encapsulate the state transitions.
-------- | NEW* | transient initial state -------- | con_sock_state_init() v ---------- | CLOSED | initialized, but no socket (and no ---------- TCP connection) ^ \ | \ con_sock_state_connecting() | ---------------------- | \ + con_sock_state_closed() \ |\ \ | \ \ | ----------- \ | | CLOSING | socket event; \ | ----------- await close \ | ^ | | | | | + con_sock_state_closing() | | / \ | | / --------------- | | / \ v
Quoted 8 lines
| / --------------
| / -----------------| CONNECTING | socket created, TCP
| | / -------------- connect initiated
| | | con_sock_state_connected()
| | v
-------------
| CONNECTED | TCP connection established
-------------Can we put this beautiful pictures in the header next to the states?
Quoted 167 lines
Make the socket state an atomic variable, reinforcing that it's a distinct transtion with no possible "intermediate/both" states. This is almost certainly overkill at this point, though the transitions into CONNECTED and CLOSING state do get called via socket callback (the rest of the transitions occur with the connection mutex held). We can back out the atomicity later.
Signed-off-by Alex Elder <[email protected]>--- include/linux/ceph/messenger.h | 8 ++++- net/ceph/messenger.c | 63 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-)include/linux/ceph/messenger.h +6
-2@@ -137,14 +137,18 @@ struct ceph_connection {const struct ceph_connection_operations *ops;struct ceph_messenger *msgr; + +atomic_t sock_state;struct socket *sock; +struct ceph_entity_addr peer_addr;/* peer address */ +struct ceph_entity_addr peer_addr_for_me; +unsigned long flags;unsigned long state;const char *error_msg;/* error message, if any */ -struct ceph_entity_addr peer_addr;/* peer address */ struct ceph_entity_name peer_name;/* peer name */ -struct ceph_entity_addr peer_addr_for_me; +unsigned peer_features;u32 connect_seq;/* identify the most recent connection attempt for this connection, client */ net/ceph/messenger.c +8
-0@@ -29,6 +29,14 @@* the sender. */ +/* State values for ceph_connection->sock_state; NEW is assumed to be 0 */ + +#define CON_SOCK_STATE_NEW 0/* -> CLOSED */ +#define CON_SOCK_STATE_CLOSED 1/* -> CONNECTING */ +#define CON_SOCK_STATE_CONNECTING 2/* -> CONNECTED or -> CLOSING */ +#define CON_SOCK_STATE_CONNECTED 3/* -> CLOSING or -> CLOSED */ +#define CON_SOCK_STATE_CLOSING 4/* -> CLOSED */ +/* static tag bytes (protocol control messages) */ static char tag_msg =CEPH_MSGR_TAG_MSG ;static char
tag_ack=CEPH_MSGR_TAG_ACK;net/ceph/messenger.c +55
-0@@ -147,6 +155,54 @@ void ceph_msgr_flush(void) }EXPORT_SYMBOL (ceph_msgr_flush); +/* Connection socket state transition functions */ + +static void con_sock_state_init (struct ceph_connection *con) +{ +int old_state; + + old_state =atomic_xchg (&con->sock_state,CON_SOCK_STATE_CLOSED ); +if (WARN_ON (old_state !=CON_SOCK_STATE_NEW )) +printk ("%s: unexpected old state %d\n" , __func__, old_state); +} + +static void con_sock_state_connecting (struct ceph_connection *con) +{ +int old_state; + + old_state =atomic_xchg (&con->sock_state,CON_SOCK_STATE_CONNECTING ); +if (WARN_ON (old_state !=CON_SOCK_STATE_CLOSED )) +printk ("%s: unexpected old state %d\n" , __func__, old_state); +} + +static void con_sock_state_connected (struct ceph_connection *con) +{ +int old_state; + + old_state =atomic_xchg (&con->sock_state,CON_SOCK_STATE_CONNECTED ); +if (WARN_ON (old_state !=CON_SOCK_STATE_CONNECTING )) +printk ("%s: unexpected old state %d\n" , __func__, old_state); +} + +static void con_sock_state_closing (struct ceph_connection *con) +{ +int old_state; + + old_state =atomic_xchg (&con->sock_state,CON_SOCK_STATE_CLOSING ); +if (WARN_ON (old_state !=CON_SOCK_STATE_CONNECTING && + old_state !=CON_SOCK_STATE_CONNECTED )) +printk ("%s: unexpected old state %d\n" , __func__, old_state); +} + +static void con_sock_state_closed (struct ceph_connection *con) +{ +int old_state; + + old_state =atomic_xchg (&con->sock_state,CON_SOCK_STATE_CLOSED ); +if (WARN_ON (old_state !=CON_SOCK_STATE_CONNECTED && + old_state !=CON_SOCK_STATE_CLOSING )) +printk ("%s: unexpected old state %d\n" , __func__, old_state); +}/* * socket callback functions @@ -203,6 +259,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__); +con_sock_state_closing (con);if (test_and_set_bit (SOCK_CLOSED , &con->flags) ==0 ) {if (test_bit (CONNECTING , &con->state)) con->error_msg ="connection failed" ;@@ -213,6 +270,7 @@ static void ceph_sock_state_change(struct sock *sk)break ;case TCP_ESTABLISHED :dout ("%s TCP_ESTABLISHED\n" , __func__); +con_sock_state_connected (con);queue_con (con);break ;default :/* Everything else is uninteresting */ @@ -277,6 +335,7 @@ static int ceph_tcp_connect(struct ceph_connection *con)return ret; } con->sock = sock; +con_sock_state_connecting (con);return 0 ; }@@ -341,6 +400,7 @@ static int con_close_socket(struct ceph_connection *con) rc = con->sock->ops->shutdown (con->sock,SHUT_RDWR );sock_release (con->sock); con->sock =NULL ; +con_sock_state_closed (con);return rc; }@@ -460,6 +520,9 @@ void ceph_con_init(struct ceph_messenger *msgr, struct ceph_connection *con) memset(con, 0, sizeof(*con));atomic_set (&con->nref,1 ); con->msgr = msgr; + +con_sock_state_init (con); +mutex_init (&con->mutex);INIT_LIST_HEAD (&con->out_queue);
INIT_LIST_HEAD(&con->out_sent);Signature · 6 lines
1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/13] libceph: provide osd number when creating osd
In reply to Alex ElderLooks good!
Quoted 65 lines · Alex Elder
Pass the osd number to the
create_osd()routine, and move the initialization of fields that depend on it therein.Signed-off-by Alex Elder <[email protected]>--- net/ceph/osd_client.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)net/ceph/osd_client.c +2
-1@@ -624,7 +624,7 @@ static void osd_reset(struct ceph_connection *con)/* * Track open sessions with osds. */ -static struct ceph_osd *create_osd (struct ceph_osd_client *osdc) +static struct ceph_osd *create_osd (struct ceph_osd_client *osdc,int onum) {struct ceph_osd *osd;@@ -634,6 +634,7 @@ static struct ceph_osd *create_osd(struct ceph_osd_client *osdc)atomic_set (&osd->o_ref,1 ); osd->o_osdc = osdc; + osd->o_osd = onum;INIT_LIST_HEAD (&osd->o_requests);INIT_LIST_HEAD (&osd->o_linger_requests);net/ceph/osd_client.c +1
-0INIT_LIST_HEAD (&osd->o_osd_lru);@@ -643,6 +644,7 @@ static struct ceph_osd *create_osd(struct ceph_osd_client *osdc) osd->o_con.private = osd; osd->o_con.ops = &osd_con_ops; osd->o_con.peer_name.type =CEPH_ENTITY_TYPE_OSD ; + osd->o_con.peer_name.num =cpu_to_le64 (onum);INIT_LIST_HEAD (&osd->o_keepalive_item);net/ceph/osd_client.c +1
-3return osd;@@ -998,15 +1000,13 @@ static int __map_request(struct ceph_osd_client *osdc, req->r_osd =__lookup_osd (osdc, o);if (!req->r_osd && o >=0 ) { err = -ENOMEM ; - req->r_osd =create_osd (osdc); + req->r_osd =create_osd (osdc, o);if (!req->r_osd) {list_move (&req->r_req_lru_item, &osdc->req_notarget); goto out; }dout ("map_request osd %p is osd%d\n" , req->r_osd, o); - req->r_osd->o_osd = o; - req->r_osd->o_con.peer_name.num =cpu_to_le64 (o);__insert_osd (osdc, req->r_osd);ceph_con_open (&req->r_osd->o_con, &osdc->osdmap->osd_addr[o]);Signature · 6 lines
1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/13] libceph: init monitor connection when opening
In reply to Alex Elderyep!
Quoted 65 lines · Alex Elder
Hold off initializing a monitor client's connection until just before it gets opened for use.
Signed-off-by Alex Elder <[email protected]>--- net/ceph/mon_client.c | 13 ++++++------- 1 files changed, 6 insertions(+), 7 deletions(-)net/ceph/mon_client.c +6
-5@@ -119,6 +119,7 @@ static void __close_session(struct ceph_mon_client *monc)dout ("__close_session closing mon%d\n" , monc->cur_mon);ceph_con_revoke (&monc->con, monc->m_auth);ceph_con_close (&monc->con); + monc->con.private =NULL ; monc->cur_mon =-1 ; monc->pending_auth =0 ;ceph_auth_reset (monc->auth);@@ -141,9 +142,13 @@ static int __open_session(struct ceph_mon_client *monc) monc->sub_renew_after = jiffies;/* i.e., expired */ monc->want_next_osdmap = !!monc->want_next_osdmap; -dout ("open_session mon%d opening\n" , monc->cur_mon); +ceph_con_init (&monc->client->msgr, &monc->con); + monc->con.private = monc; + monc->con.ops = &mon_con_ops; monc->con.peer_name.type =CEPH_ENTITY_TYPE_MON ; monc->con.peer_name.num =cpu_to_le64 (monc->cur_mon); + +dout ("open_session mon%d opening\n" , monc->cur_mon);ceph_con_open (&monc->con, &monc->monmap->mon_inst[monc->cur_mon].addr);@@ -760,10 +765,6 @@ int ceph_monc_init(struct ceph_mon_client *monc, struct ceph_client *cl) goto out;/* connection */ -ceph_con_init (&monc->client->msgr, &monc->con); - monc->con.private = monc; - monc->con.ops = &mon_con_ops; -/* authentication */ monc->auth = ceph_auth_init(cl->options->name,net/ceph/mon_client.c +0
-2cl->options->key);@@ -836,8 +837,6 @@ void ceph_monc_stop(struct ceph_mon_client *monc)mutex_lock (&monc->mutex);__close_session (monc); - monc->con.private =NULL ; -mutex_unlock (&monc->mutex);ceph_auth_destroy (monc->auth);Signature · 6 lines
1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 12/13] libceph: fully initialize connection in con_init()
In reply to Alex ElderQuoted 127 lines · Alex Elder
Move the initialization of a ceph connection's private pointer, operations vector pointer, and peer name information into
ceph_con_init(). Rearrange the arguments so the connection pointer is first. Hide the byte-swapping of the peer entity number insideceph_con_init()Signed-off-by Alex Elder <[email protected]>--- fs/ceph/mds_client.c | 7 ++----- include/linux/ceph/messenger.h | 6 ++++-- net/ceph/messenger.c | 9 ++++++++- net/ceph/mon_client.c | 8 +++----- net/ceph/osd_client.c | 7 ++----- 5 files changed, 19 insertions(+), 18 deletions(-)fs/ceph/mds_client.c +2
-5@@ -394,11 +394,8 @@ static struct ceph_mds_session *register_session(struct ceph_mds_client *mdsc, s->s_seq =0 ;mutex_init (&s->s_mutex); -ceph_con_init (&mdsc->fsc->client->msgr, &s->s_con); - s->s_con.private = s; - s->s_con.ops = &mds_con_ops; - s->s_con.peer_name.type =CEPH_ENTITY_TYPE_MDS ; - s->s_con.peer_name.num =cpu_to_le64 (mds); +ceph_con_init (&s->s_con, s, &mds_con_ops, &mdsc->fsc->client->msgr, +CEPH_ENTITY_TYPE_MDS , mds);spin_lock_init (&s->s_gen_ttl_lock);fs/ceph/mds_client.c +0
-0s->s_cap_gen =0 ;include/linux/ceph/messenger.h +4
-2@@ -227,8 +227,10 @@ extern void ceph_messenger_init(struct ceph_messenger *msgr, u32 required_features, bool nocrc); -extern void ceph_con_init (struct ceph_messenger *msgr, -struct ceph_connection *con); +extern void ceph_con_init (struct ceph_connection *con,void *private, +const struct ceph_connection_operations *ops, +struct ceph_messenger *msgr,__u8 entity_type, +__u64 entity_num);extern void ceph_con_open (struct ceph_connection *con,struct ceph_entity_addr *addr);extern bool
ceph_con_opened(struct ceph_connection *con);net/ceph/messenger.c +8
-1@@ -514,15 +514,22 @@ void ceph_con_put(struct ceph_connection *con)/* * initialize a new connection. */ -void ceph_con_init (struct ceph_messenger *msgr,struct ceph_connection *con) +void ceph_con_init (struct ceph_connection *con,void *private, +const struct ceph_connection_operations *ops, +struct ceph_messenger *msgr,__u8 entity_type,__u64 entity_num) {dout ("con_init %p\n" , con);memset (con,0 ,sizeof (*con)); + con->private = private;atomic_set (&con->nref,1 ); + con->ops = ops; con->msgr = msgr;con_sock_state_init (con); + con->peer_name.type = (__u8 ) entity_type; + con->peer_name.num =cpu_to_le64 (entity_num); +mutex_init (&con->mutex);INIT_LIST_HEAD (&con->out_queue);INIT_LIST_HEAD (&con->out_sent);net/ceph/mon_client.c +3
-5@@ -142,11 +142,9 @@ static int __open_session(struct ceph_mon_client *monc) monc->sub_renew_after = jiffies;/* i.e., expired */ monc->want_next_osdmap = !!monc->want_next_osdmap; -ceph_con_init (&monc->client->msgr, &monc->con); - monc->con.private = monc; - monc->con.ops = &mon_con_ops; - monc->con.peer_name.type =CEPH_ENTITY_TYPE_MON ; - monc->con.peer_name.num =cpu_to_le64 (monc->cur_mon); +ceph_con_init (&monc->con, monc, &mon_con_ops, + &monc->client->msgr, +CEPH_ENTITY_TYPE_MON , monc->cur_mon);dout ("open_session mon%d opening\n" , monc->cur_mon); ceph_con_open(&monc->con,net/ceph/osd_client.c +2
-5@@ -640,11 +640,8 @@ static struct ceph_osd *create_osd(struct ceph_osd_client *osdc, int onum)INIT_LIST_HEAD (&osd->o_osd_lru); osd->o_incarnation =1 ; -ceph_con_init (&osdc->client->msgr, &osd->o_con); - osd->o_con.private = osd; - osd->o_con.ops = &osd_con_ops; - osd->o_con.peer_name.type =CEPH_ENTITY_TYPE_OSD ; - osd->o_con.peer_name.num =cpu_to_le64 (onum); +ceph_con_init (&osd->o_con, osd, &osd_con_ops, &osdc->client->msgr, +CEPH_ENTITY_TYPE_OSD , onum);INIT_LIST_HEAD (&osd->o_keepalive_item);return osd;
Signature · 6 lines
1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] libceph: set CLOSED state bit in con_init
In reply to Alex ElderQuoted 65 lines · Alex Elder
Once a connection is fully initialized, it is really in a CLOSED state, so make that explicit by setting the bit in its state field.
It is possible for a connection in NEGOTIATING state to get a failure, leading to
ceph_fault()and ultimatelyceph_con_close(). Clear that bits if it is set in that case, to reflect that the connection truly is closed and is no longer participating in a connect sequence.Issue a warning if
ceph_con_open()is called on a connection that is not in CLOSED state.Signed-off-by Alex Elder <[email protected]>--- net/ceph/messenger.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)net/ceph/messenger.c +5
-1@@ -452,10 +452,13 @@ void ceph_con_close(struct ceph_connection *con)dout ("con_close %p peer %s\n" , con,ceph_pr_addr (&con->peer_addr.in_addr));set_bit (CLOSED , &con->state);/* in case there's queued work */ +clear_bit (NEGOTIATING , &con->state);clear_bit (STANDBY , &con->state);/* avoid connect_seq bump */ +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 ;@@ -472,7 +475,8 @@ void ceph_con_open(struct ceph_connection *con, struct ceph_entity_addr *addr) {dout ("con_open %p %s\n" , con,ceph_pr_addr (&addr->in_addr));set_bit (OPENING , &con->state); -clear_bit (CLOSED , &con->state); +WARN_ON (!test_and_clear_bit (CLOSED , &con->state)); +memcpy (&con->peer_addr, addr,sizeof (*addr)); con->delay =0 ;/* reset backoff memory */ net/ceph/messenger.c +2
-0queue_con (con);@@ -534,6 +538,8 @@ void ceph_con_init(struct ceph_connection *con, void *private,INIT_LIST_HEAD (&con->out_queue);INIT_LIST_HEAD (&con->out_sent);INIT_DELAYED_WORK (&con->work, con_work); + +set_bit (CLOSED , &con->state); }EXPORT_SYMBOL (ceph_con_init);Signature · 7 lines
-- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/13] libceph: embed ceph connection structure in mon_client
In reply to Sage WeilQuoted 162 lines · Sage Weil
Quoted 149 lines · Alex Elder
A monitor client has a pointer to a ceph connection structure in it. This is the only one of the three ceph client types that do it this way; the OSD and MDS clients embed the connection into their main structures. There is always exactly one ceph connection for a monitor client, so there is no need to allocate it separate from the monitor client structure.
So switch the
ceph_mon_clientstructure to embed itsceph_connectionstructure.Signed-off-by Alex Elder<[email protected]>--- include/linux/ceph/mon_client.h | 2 +- net/ceph/mon_client.c | 47 ++++++++++++++++---------------------- 2 files changed, 21 insertions(+), 28 deletions(-)include/linux/ceph/mon_client.h +1
-1@@ -70,7 +70,7 @@ struct ceph_mon_client {bool hunting;int cur_mon;/* last monitor i contacted */ unsigned long sub_sent, sub_renew_after; -struct ceph_connection *con; +struct ceph_connection con;bool have_fsid;/* pending generic requests */ net/ceph/mon_client.c +2
-2@@ -106,9 +106,9 @@ static void __send_prepared_auth_request(struct ceph_mon_client *monc, int len) monc->pending_auth = 1; monc->m_auth->front.iov_len = len; monc->m_auth->hdr.front_len =cpu_to_le32 (len); -ceph_con_revoke (monc->con, monc->m_auth); +ceph_con_revoke (&monc->con, monc->m_auth);ceph_msg_get (monc->m_auth);/* keep our ref */ -ceph_con_send (monc->con, monc->m_auth); +ceph_con_send (&monc->con, monc->m_auth); }/*
net/ceph/mon_client.c +2
-2@@ -117,8 +117,8 @@ static void __send_prepared_auth_request(struct ceph_mon_client *monc, int len)static void __close_session (struct ceph_mon_client *monc) {dout ("__close_session closing mon%d\n" , monc->cur_mon); -ceph_con_revoke (monc->con, monc->m_auth); -ceph_con_close (monc->con); +ceph_con_revoke (&monc->con, monc->m_auth); +ceph_con_close (&monc->con); monc->cur_mon =-1 ; monc->pending_auth =0 ;ceph_auth_reset(monc->auth);net/ceph/mon_client.c +6
-6@@ -142,9 +142,9 @@ static int __open_session(struct ceph_mon_client *monc) monc->want_next_osdmap = !!monc->want_next_osdmap;dout ("open_session mon%d opening\n" , monc->cur_mon); - monc->con->peer_name.type =CEPH_ENTITY_TYPE_MON ; - monc->con->peer_name.num =cpu_to_le64 (monc->cur_mon); -ceph_con_open (monc->con, + monc->con.peer_name.type =CEPH_ENTITY_TYPE_MON ; + monc->con.peer_name.num =cpu_to_le64 (monc->cur_mon); +ceph_con_open (&monc->con, &monc->monmap->mon_inst[monc->cur_mon].addr);/* initiatiate authentication handshake */ @@ -226,8 +226,8 @@ static void __send_subscribe(struct ceph_mon_client *monc) msg->front.iov_len = p - msg->front.iov_base; msg->hdr.front_len =cpu_to_le32 (msg->front.iov_len); -ceph_con_revoke (monc->con, msg); -ceph_con_send (monc->con,ceph_msg_get (msg)); +ceph_con_revoke (&monc->con, msg); +ceph_con_send (&monc->con,ceph_msg_get (msg)); monc->sub_sent = jiffies |1 ;/* never 0 */ }@@ -247,7 +247,7 @@ static void handle_subscribe_ack(struct ceph_mon_client *monc, if (monc->hunting) {pr_info ("mon%d %s session established\n" , monc->cur_mon, -ceph_pr_addr (&monc->con->peer_addr.in_addr)); +ceph_pr_addr (&monc->con.peer_addr.in_addr)); monc->hunting = false; }dout("handle_subscribe_ack after %d seconds\n", seconds);net/ceph/mon_client.c +1
-1@@ -461,7 +461,7 @@ static int do_generic_request(struct ceph_mon_client *monc, req->request->hdr.tid =cpu_to_le64 (req->tid);__insert_generic_request (monc, req); monc->num_generic_requests++; -ceph_con_send (monc->con,ceph_msg_get (req->request)); +ceph_con_send (&monc->con,ceph_msg_get (req->request));mutex_unlock (&monc->mutex);err = wait_for_completion_interruptible(&req->completion);net/ceph/mon_client.c +7
-10@@ -684,8 +684,8 @@ static void __resend_generic_request(struct ceph_mon_client *monc)for (p =rb_first (&monc->generic_request_tree); p; p =rb_next (p)) { req = rb_entry(p, struct ceph_mon_generic_request, node); -ceph_con_revoke (monc->con, req->request); -ceph_con_send (monc->con,ceph_msg_get (req->request)); +ceph_con_revoke (&monc->con, req->request); +ceph_con_send (&monc->con,ceph_msg_get (req->request)); } }@@ -705,7 +705,7 @@ static void delayed_work(struct work_struct *work)__close_session (monc);__open_session (monc);/* continue hunting */ }else { -ceph_con_keepalive (monc->con); +ceph_con_keepalive (&monc->con);__validate_auth (monc);@@ -760,19 +760,16 @@ int ceph_monc_init(struct ceph_mon_client *monc, struct ceph_client *cl) goto out;/* connection */ - monc->con =kmalloc (sizeof (*monc->con),GFP_KERNEL ); -if (!monc->con) - goto out_monmap; -ceph_con_init (&monc->client->msgr, monc->con); - monc->con->private = monc; - monc->con->ops =&mon_con_ops; +ceph_con_init (&monc->client->msgr,&monc->con); + monc->con.private = monc; + monc->con.ops =&mon_con_ops;/* authentication */ monc->auth =ceph_auth_init (cl->options->name, cl->options->key);if (IS_ERR (monc->auth)) { err =PTR_ERR (monc->auth); - goto out_con; + goto out_monmap; } monc->auth->want_keys =
CEPH_ENTITY_TYPE_AUTH|CEPH_ENTITY_TYPE_MON|net/ceph/mon_client.c out_subscribe_ack: · line 828
Quoted 5 lines
net/ceph/mon_client.c +0
-2@@ -824,8 +821,6 @@ out_subscribe_ack:ceph_msg_put (monc->m_subscribe_ack); out_auth:ceph_auth_destroy (monc->auth); -out_con: - monc->con->ops->put (monc->con);AH!
This reminds me, these connections need to be refcounted. There's a ->get() and ->put() op defined so that you can refcount the containing structure. That means that this patch needs to alo change
Quoted 7 lines
static const struct
ceph_connection_operationsmon_con_ops= {
.get =ceph_con_get,
.put =ceph_con_put,in
mon_client.c. Hopefully themon_clientitself is refcounted, *or* we can ensure that it won't go away before the msgr workqueue is drained and the get/put ops can turn to no-ops.
Earlier I looked at the ref counting stuff a bit and stopped myself from going off on that tangent. But it didn't look like it was used consistently and made a note to myself to revisit it.
Quoted 8 lines
Also: when poking around, I noticed that
ceph_con_get()andput()are called directly fromosd_client.c... that's a bug! Those connections have a get and put op defined that twiddles the containingceph_osdstruct's ref count.I pushed several patches to your latest (wip-messenger-2) branch that fix these issues. Compile tested only! The first should probably be folded into this one, the others follow.
I'll look at your patches and incorporate them as appropriate. But at the moment I don't see them; whenever you are back online again perhaps you'll send me a link.
-Alex
Quoted 34 lines
Quoted 33 lines
net/ceph/mon_client.c +2
-5out_monmap:kfree (monc->monmap); out:@@ -841,9 +836,7 @@ void ceph_monc_stop(struct ceph_mon_client *monc)mutex_lock (&monc->mutex);__close_session (monc); - monc->con->private =NULL ; - monc->con->ops->put (monc->con); - monc->con =NULL ; + monc->con.private =NULL ;mutex_unlock (&monc->mutex);@@ -1021,7 +1014,7 @@ static void mon_fault(struct ceph_connection *con)if (!monc->hunting)pr_info ("mon%d %s session lost, " "hunting for new mon\n" , monc->cur_mon, -ceph_pr_addr (&monc->con->peer_addr.in_addr)); +ceph_pr_addr (&monc->con.peer_addr.in_addr));__close_session (monc);if (!monc->hunting) { --1.7.5.4
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/13] libceph: start separating connection flags from state
In reply to Sage WeilQuoted 41 lines · Sage Weil
Quoted 40 lines · Alex Elder
A
ceph_connectionholds 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<[email protected]>--- include/linux/ceph/messenger.h | 18 +++++++++---- net/ceph/messenger.c | 50 ++++++++++++++++++++-------------------- 2 files changed, 37 insertions(+), 31 deletions(-)include/linux/ceph/messenger.h +10
-3@@ -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
Quoted 8 lines
include/linux/ceph/messenger.h
Quoted 6 lines
* the
ceph_connectionaround to maintain shared
* state with the peer. */
#define CLOSED 10 /* we've closed the connection */include/linux/ceph/messenger.h +0
-2-#define SOCK_CLOSED 11/* socket state changed to closed */ #define OPENING 13/* open connection w/ (possibly new) peer */ -#define BACKOFF 15Later it might be work prefixing these with
FLAG_and/orSTATE_.
Absolutely, I'm saving that easy stuff for the end. I'll move the definitions into messenger.c as well if I can.
Quoted 1 line
Reviewed-by Sage Weil<[email protected]>
Thanks.
-Alex
Quoted 224 lines
Quoted 223 lines
/*
* A single connection with another host.include/linux/ceph/messenger.h +2
-1@@ -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 */ net/ceph/messenger.c +6
-6@@ -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); }/*
net/ceph/messenger.c +2
-2@@ -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); }/*
net/ceph/messenger.c +1
-1@@ -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;net/ceph/messenger.c +1
-1@@ -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)net/ceph/messenger.c +1
-1@@ -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 ;}net/ceph/messenger.c +8
-8@@ -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. */
net/ceph/messenger.c +4
-4@@ -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);net/ceph/messenger.c +2
-3@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/13] libceph: start tracking connection socket state
In reply to Sage WeilQuoted 38 lines · Sage Weil
Quoted 28 lines · Alex Elder
Start explicitly keeping track of the state of a ceph connection's socket, separate from the state of the connection itself. Create placeholder functions to encapsulate the state transitions.
-------- | NEW* | transient initial state -------- | con_sock_state_init() v ---------- | CLOSED | initialized, but no socket (and no ---------- TCP connection) ^ \ | \ con_sock_state_connecting() | ---------------------- | \ + con_sock_state_closed() \ |\ \ | \ \ | ----------- \ | | CLOSING | socket event; \ | ----------- await close \ | ^ | | | | | + con_sock_state_closing() | | / \ | | / --------------- | | / \ vQuoted 8 lines
| / -------------- | / -----------------| CONNECTING | socket created, TCP | | / -------------- connect initiated | | | con_sock_state_connected() | | v ------------- | CONNECTED | TCP connection established -------------
Quoted 1 line
Can we put this beautiful pictures in the header next to the states?
I can be quite the ASCII artist. Yes, I will add this, when I update the state definitions with better names and numbers.
-Alex
Quoted 170 lines
Reviewed-by Sage Weil<[email protected]>Quoted 167 lines
Make the socket state an atomic variable, reinforcing that it's a distinct transtion with no possible "intermediate/both" states. This is almost certainly overkill at this point, though the transitions into CONNECTED and CLOSING state do get called via socket callback (the rest of the transitions occur with the connection mutex held). We can back out the atomicity later.
Signed-off-by Alex Elder<[email protected]>--- include/linux/ceph/messenger.h | 8 ++++- net/ceph/messenger.c | 63 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-)include/linux/ceph/messenger.h +6
-2@@ -137,14 +137,18 @@ struct ceph_connection {const struct ceph_connection_operations *ops;struct ceph_messenger *msgr; + +atomic_t sock_state;struct socket *sock; +struct ceph_entity_addr peer_addr;/* peer address */ +struct ceph_entity_addr peer_addr_for_me; +unsigned long flags;unsigned long state;const char *error_msg;/* error message, if any */ -struct ceph_entity_addr peer_addr;/* peer address */ struct ceph_entity_name peer_name;/* peer name */ -struct ceph_entity_addr peer_addr_for_me; +unsigned peer_features;u32 connect_seq;/* identify the most recent connection attempt for this connection, client */ net/ceph/messenger.c +8
-0@@ -29,6 +29,14 @@* the sender. */ +/* State values for ceph_connection->sock_state; NEW is assumed to be 0 */ + +#define CON_SOCK_STATE_NEW 0/* -> CLOSED */ +#define CON_SOCK_STATE_CLOSED 1/* -> CONNECTING */ +#define CON_SOCK_STATE_CONNECTING 2/* -> CONNECTED or -> CLOSING */ +#define CON_SOCK_STATE_CONNECTED 3/* -> CLOSING or -> CLOSED */ +#define CON_SOCK_STATE_CLOSING 4/* -> CLOSED */ +/* static tag bytes (protocol control messages) */ static char tag_msg =CEPH_MSGR_TAG_MSG ;static char tag_ack = CEPH_MSGR_TAG_ACK;net/ceph/messenger.c +55
-0@@ -147,6 +155,54 @@ void ceph_msgr_flush(void) }EXPORT_SYMBOL (ceph_msgr_flush); +/* Connection socket state transition functions */ + +static void con_sock_state_init (struct ceph_connection *con) +{ +int old_state; + + old_state =atomic_xchg (&con->sock_state,CON_SOCK_STATE_CLOSED ); +if (WARN_ON (old_state !=CON_SOCK_STATE_NEW )) +printk ("%s: unexpected old state %d\n" , __func__, old_state); +} + +static void con_sock_state_connecting (struct ceph_connection *con) +{ +int old_state; + + old_state =atomic_xchg (&con->sock_state,CON_SOCK_STATE_CONNECTING ); +if (WARN_ON (old_state !=CON_SOCK_STATE_CLOSED )) +printk ("%s: unexpected old state %d\n" , __func__, old_state); +} + +static void con_sock_state_connected (struct ceph_connection *con) +{ +int old_state; + + old_state =atomic_xchg (&con->sock_state,CON_SOCK_STATE_CONNECTED ); +if (WARN_ON (old_state !=CON_SOCK_STATE_CONNECTING )) +printk ("%s: unexpected old state %d\n" , __func__, old_state); +} + +static void con_sock_state_closing (struct ceph_connection *con) +{ +int old_state; + + old_state =atomic_xchg (&con->sock_state,CON_SOCK_STATE_CLOSING ); +if (WARN_ON (old_state !=CON_SOCK_STATE_CONNECTING && + old_state !=CON_SOCK_STATE_CONNECTED )) +printk ("%s: unexpected old state %d\n" , __func__, old_state); +} + +static void con_sock_state_closed (struct ceph_connection *con) +{ +int old_state; + + old_state =atomic_xchg (&con->sock_state,CON_SOCK_STATE_CLOSED ); +if (WARN_ON (old_state !=CON_SOCK_STATE_CONNECTED && + old_state !=CON_SOCK_STATE_CLOSING )) +printk ("%s: unexpected old state %d\n" , __func__, old_state); +}/* * socket callback functions @@ -203,6 +259,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__); +con_sock_state_closing (con);if (test_and_set_bit (SOCK_CLOSED ,&con->flags) ==0 ) {if (test_bit (CONNECTING ,&con->state)) con->error_msg ="connection failed" ;@@ -213,6 +270,7 @@ static void ceph_sock_state_change(struct sock *sk)break ;case TCP_ESTABLISHED :dout ("%s TCP_ESTABLISHED\n" , __func__); +con_sock_state_connected (con);queue_con (con);break ;default :/* Everything else is uninteresting */ @@ -277,6 +335,7 @@ static int ceph_tcp_connect(struct ceph_connection *con)return ret; } con->sock = sock; +con_sock_state_connecting (con);return 0 ; }@@ -341,6 +400,7 @@ static int con_close_socket(struct ceph_connection *con) rc = con->sock->ops->shutdown (con->sock,SHUT_RDWR );sock_release (con->sock); con->sock =NULL ; +con_sock_state_closed (con);return rc; }@@ -460,6 +520,9 @@ void ceph_con_init(struct ceph_messenger *msgr, struct ceph_connection *con) memset(con, 0, sizeof(*con));atomic_set (&con->nref,1 ); con->msgr = msgr; + +con_sock_state_init (con); +mutex_init (&con->mutex);INIT_LIST_HEAD (&con->out_queue);
INIT_LIST_HEAD(&con->out_sent);
--
1.7.5.4--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/13] libceph: embed ceph connection structure in mon_client
In reply to Alex ElderQuoted 9 lines · Alex Elder
Quoted 8 lines · Sage Weil
Quoted 7 lines · Alex Elder
A monitor client has a pointer to a ceph connection structure in it. This is the only one of the three ceph client types that do it this way; the OSD and MDS clients embed the connection into their main structures. There is always exactly one ceph connection for a monitor client, so there is no need to allocate it separate from the monitor client structure.
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
So switch the
ceph_mon_clientstructure to embed itsceph_connectionstructure.Signed-off-by Alex Elder<[email protected]>
. . .
Quoted 19 lines
Quoted 19 lines
Quoted 12 lines
/* authentication */
monc->auth= ceph_auth_init(cl->options->name,cl->options->key);
if (IS_ERR(monc->auth)) {
err =PTR_ERR(monc->auth);
- gotoout_con;
+ gotoout_monmap;
}monc->auth->want_keys=CEPH_ENTITY_TYPE_AUTH|CEPH_ENTITY_TYPE_MON|net/ceph/mon_client.c +0
-0@@ -824,8 +821,6 @@ out_subscribe_ack:ceph_msg_put (monc->m_subscribe_ack);Quoted 4 lines
out_auth:ceph_auth_destroy(monc->auth);
-out_con:
-monc->con->ops->put(monc->con);AH!
Quoted 3 lines
Quoted 3 lines
This reminds me, these connections need to be refcounted. There's a ->get() and ->put() op defined so that you can refcount the containing structure. That means that this patch needs to alo change
Looking at this again. Why do they need to be refcounted? If this patch is correct in embedding the connection into the containing structure, then the last reference to the containing structure is coincident with with the last reference to the connection. And the other connections are already embedded into other containing structures.
So--again assuming it's OK to embed the connection--I would rather see the ->get and ->put methods for the connection go away entirely and have the containing structure take care of its own damned ref counting...
This actually gets into another thing I wanted to do anyway (while digging through raw memory trying to figure out what's going on). I want every ceph_message to point back to the connection it is associated with. That way there's no need for the OSD (for example) to keep track of the connection--a revoke is simply an operation on the message, which would already know the connection from which it is being revoked.
If you think the above approach is good I'll gladly do it now rather than later. I think it might eliminate the need for any need to reference count the connections.
Anyway, when I'm done (if I ever finish!) the state of the connection will tell you whether any legitimate uses of a connection remain.
-Alex
Quoted 63 lines
Quoted 7 lines
static const struct
ceph_connection_operationsmon_con_ops= { .get =ceph_con_get,
.put =ceph_con_put,in
mon_client.c. Hopefully themon_clientitself is refcounted, *or* we can ensure that it won't go away before the msgr workqueue is drained and the get/put ops can turn to no-ops.Earlier I looked at the ref counting stuff a bit and stopped myself from going off on that tangent. But it didn't look like it was used consistently and made a note to myself to revisit it.
Quoted 8 lines
Also: when poking around, I noticed that
ceph_con_get()andput()are called directly fromosd_client.c... that's a bug! Those connections have a get and put op defined that twiddles the containingceph_osdstruct's ref count.I pushed several patches to your latest (wip-messenger-2) branch that fix these issues. Compile tested only! The first should probably be folded into this one, the others follow.
I'll look at your patches and incorporate them as appropriate. But at the moment I don't see them; whenever you are back online again perhaps you'll send me a link.
-Alex
Quoted 34 lines
Quoted 33 lines
out_monmap:kfree(monc->monmap);
out:net/ceph/mon_client.c +1
-3@@ -841,9 +836,7 @@ void ceph_monc_stop(struct ceph_mon_client *monc)mutex_lock (&monc->mutex);__close_session (monc); - monc->con->private =NULL ; - monc->con->ops->put (monc->con); - monc->con =NULL ; + monc->con.private =NULL ;mutex_unlock (&monc->mutex);@@ -1021,7 +1014,7 @@ static void mon_fault(struct ceph_connection *con)if (!monc->hunting)pr_info("mon%d %s session lost, "
"hunting for new mon\n",monc->cur_mon,
-ceph_pr_addr(&monc->con->peer_addr.in_addr));
+ceph_pr_addr(&monc->con.peer_addr.in_addr));
__close_session(monc);
if (!monc->hunting) {
--
1.7.5.4--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/13] libceph: embed ceph connection structure in mon_client
Quoted 47 lines · Alex Elder
Quoted 9 lines · Alex Elder
Quoted 8 lines · Sage Weil
Quoted 7 lines · Alex Elder
A monitor client has a pointer to a ceph connection structure in it. This is the only one of the three ceph client types that do it this way; the OSD and MDS clients embed the connection into their main structures. There is always exactly one ceph connection for a monitor client, so there is no need to allocate it separate from the monitor client structure.
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
So switch the
ceph_mon_clientstructure to embed itsceph_connectionstructure.Signed-off-by Alex Elder<[email protected]>. . .
Quoted 19 lines
Quoted 19 lines
Quoted 12 lines
/* authentication */
monc->auth= ceph_auth_init(cl->options->name,cl->options->key);
if (IS_ERR(monc->auth)) {
err =PTR_ERR(monc->auth);
- gotoout_con;
+ gotoout_monmap;
}monc->auth->want_keys=CEPH_ENTITY_TYPE_AUTH|CEPH_ENTITY_TYPE_MON|net/ceph/mon_client.c +0
-0@@ -824,8 +821,6 @@ out_subscribe_ack:ceph_msg_put (monc->m_subscribe_ack);Quoted 4 lines
out_auth:ceph_auth_destroy(monc->auth);
-out_con:
-monc->con->ops->put(monc->con);AH!
Quoted 3 lines
Quoted 3 lines
This reminds me, these connections need to be refcounted. There's a ->get() and ->put() op defined so that you can refcount the containing structure. That means that this patch needs to alo change
Looking at this again. Why do they need to be refcounted? If this patch is correct in embedding the connection into the containing structure, then the last reference to the containing structure is coincident with with the last reference to the connection. And the other connections are already embedded into other containing structures.
Quoted 4 lines
So--again assuming it's OK to embed the connection--I would rather see the ->get and ->put methods for the connection go away entirely and have the containing structure take care of its own damned ref counting...
The problem is that socket events queue work, which can take a while, and race with, say, osd_client getting an osdmap and dropping it's struct ceph_osd. The ->get and ->put ops just twiddle the containing struct's refcount, in that case, so the con_work will find the (now closed) ceph_connection and do nothing...
Quoted 11 lines
This actually gets into another thing I wanted to do anyway (while digging through raw memory trying to figure out what's going on). I want every
ceph_messageto point back to the connection it is associated with. That way there's no need for the OSD (for example) to keep track of the connection--a revoke is simply an operation on the message, which would already know the connection from which it is being revoked.If you think the above approach is good I'll gladly do it now rather than later. I think it might eliminate the need for any need to reference count the connections.
That sounds reasonable.. but i'm pretty sure the con refcounts can't go away :)
sage
Quoted 76 lines
Anyway, when I'm done (if I ever finish!) the state of the connection will tell you whether any legitimate uses of a connection remain.
-Alex
Quoted 22 lines
Quoted 7 lines
static const struct
ceph_connection_operationsmon_con_ops= { .get =ceph_con_get,
.put =ceph_con_put,in
mon_client.c. Hopefully themon_clientitself is refcounted, *or* we can ensure that it won't go away before the msgr workqueue is drained and the get/put ops can turn to no-ops.Earlier I looked at the ref counting stuff a bit and stopped myself from going off on that tangent. But it didn't look like it was used consistently and made a note to myself to revisit it.
Quoted 8 lines
Also: when poking around, I noticed that
ceph_con_get()andput()are called directly fromosd_client.c... that's a bug! Those connections have a get and put op defined that twiddles the containingceph_osdstruct's ref count.I pushed several patches to your latest (wip-messenger-2) branch that fix these issues. Compile tested only! The first should probably be folded into this one, the others follow.
Quoted 41 lines
I'll look at your patches and incorporate them as appropriate. But at the moment I don't see them; whenever you are back online again perhaps you'll send me a link.
-Alex
Quoted 34 lines
Quoted 33 lines
out_monmap:kfree(monc->monmap);
out:net/ceph/mon_client.c +1
-3@@ -841,9 +836,7 @@ void ceph_monc_stop(struct ceph_mon_client *monc)mutex_lock (&monc->mutex);__close_session (monc); - monc->con->private =NULL ; - monc->con->ops->put (monc->con); - monc->con =NULL ; + monc->con.private =NULL ;mutex_unlock (&monc->mutex);@@ -1021,7 +1014,7 @@ static void mon_fault(struct ceph_connection *con)if (!monc->hunting)pr_info("mon%d %s session lost, "
"hunting for new mon\n",monc->cur_mon,
-ceph_pr_addr(&monc->con->peer_addr.in_addr));
+ceph_pr_addr(&monc->con.peer_addr.in_addr));
__close_session(monc);
if (!monc->hunting) {
--
1.7.5.4--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/13] libceph: embed ceph connection structure in mon_client
Quoted 5 lines · Sage Weil
The problem is that socket events queue work, which can take a while, and race with, say,
osd_clientgetting an osdmap and dropping it's structceph_osd. The ->get and ->put ops just twiddle the containing struct's refcount, in that case, so thecon_workwill find the (now closed)ceph_connectionand do nothing...
I think you're saying that the connection (or its socket) needs to be protected from its containing structure going away. So the connection needs to hold a reference to its container. If that's the case then the disposal of the ceph_osd needs to clean up the connection fully before it goes away.
Anyway, I think I see why there might be a need for the ref counts and they obviously won't go away if they're needed...
-Alex
Re: [PATCH 07/13] libceph: embed ceph connection structure in mon_client
Quoted 7 lines · Alex Elder
Quoted 5 lines · Sage Weil
The problem is that socket events queue work, which can take a while, and race with, say,
osd_clientgetting an osdmap and dropping it's structceph_osd. The ->get and ->put ops just twiddle the containing struct's refcount, in that case, so thecon_workwill find the (now closed)ceph_connectionand do nothing...
Quoted 5 lines
I think you're saying that the connection (or its socket) needs to be protected from its containing structure going away. So the connection needs to hold a reference to its container. If that's the case then the disposal of the
ceph_osdneeds to clean up the connection fully before it goes away.
Yeah. I think it happens already before we drop the ref:
static void __remove_osd(struct ceph_osd_client *osdc, struct ceph_osd *osd)
{
dout("__remove_osd %p\n", osd);
BUG_ON(!list_empty(&osd->o_requests));
rb_erase(&osd->o_node, &osdc->osds);
list_del_init(&osd->o_osd_lru);
ceph_con_close(&osd->o_con);
put_osd(osd);
}So it's just the con reference in the workqueue that matters.
sage
Quoted 11 lines
Anyway, I think I see why there might be a need for the ref counts and they obviously won't go away if they're needed...
-Alex
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/13] libceph: embed ceph connection structure in mon_client
In reply to Sage WeilQuoted 4 lines · Sage Weil
Also: when poking around, I noticed that
ceph_con_get()andput()are called directly fromosd_client.c... that's a bug! Those connections have a get and put op defined that twiddles the containingceph_osdstruct's ref count.
So are you saying that the calls in "osd_client.c" to ceph_con_get() and ceph_con_put() should instead be calls to get_osd_con() and put_osd_con(), respectively? (Or more generally con->ops->get() and con->ops->put()?)
-Alex
Re: [PATCH 07/13] libceph: embed ceph connection structure in mon_client
Quoted 10 lines · Alex Elder
Quoted 4 lines · Sage Weil
Also: when poking around, I noticed that
ceph_con_get()andput()are called directly fromosd_client.c... that's a bug! Those connections have a get and put op defined that twiddles the containingceph_osdstruct's ref count.So are you saying that the calls in "
osd_client.c" toceph_con_get()andceph_con_put()should instead be calls toget_osd_con()andput_osd_con(), respectively? (Or more generallycon->ops->get()andcon->ops->put()?)
Yeah.. one of the patches I pushed [er, and pushing now] fixes that.
sage
Re: [PATCH 03/13] libceph: delete useless SOCK_CLOSED manipulations
In reply to Alex ElderQuoted 6 lines · Alex Elder
In
con_close_socket(),SOCK_CLOSEDis set in the connection state, then cleared again after shutting down the socket. Nothing between the setting and clearing of that bit will ever be affected by it, so there's no point in setting/clearing it at all. So don't.Signed-off-by Alex Elder <[email protected]>
I am retracting this proposed change.
I believe it's possible for the con->sock->ops->shutdown() call to trigger a TCP_CLOSE socket state change event, which means that there *is* something that can be affected by that state bit being set.
-Alex
Quoted 21 lines
---
net/ceph/messenger.c| 2 --1 files changed, 0 insertions(+), 2 deletions(-)net/ceph/messenger.c +0
-2@@ -338,11 +338,9 @@ static int con_close_socket(struct ceph_connection *con)dout ("con_close_socket on %p sock %p\n" , con, con->sock);if (!con->sock)return 0 ; -set_bit (SOCK_CLOSED , &con->state); rc = con->sock->ops->shutdown (con->sock,SHUT_RDWR );sock_release (con->sock); con->sock =NULL ; -clear_bit (SOCK_CLOSED , &con->state);return rc; }
Re: [PATCH 09/13] libceph: start tracking connection socket state
In reply to Alex ElderQuoted 153 lines · Alex Elder
Start explicitly keeping track of the state of a ceph connection's socket, separate from the state of the connection itself. Create placeholder functions to encapsulate the state transitions.
--------
| NEW* | transient initial state
--------
|con_sock_state_init()
v
----------
| CLOSED | initialized, but no socket (and no
---------- TCP connection)
^ \| \ con_sock_state_connecting() | ---------------------- | \ + con_sock_state_closed() \ |\ \ | \ \ | ----------- \ | | CLOSING | socket event; \ | ----------- await close \ | ^ | | | | | + con_sock_state_closing() | | / \ | | / --------------- | | / \ v | / -------------- | / -----------------| CONNECTING | socket created, TCP | | / -------------- connect initiated | | | con_sock_state_connected() | | v-------------
| CONNECTED | TCP connection established
-------------Make the socket state an atomic variable, reinforcing that it's a distinct transtion with no possible "intermediate/both" states. This is almost certainly overkill at this point, though the transitions into CONNECTED and CLOSING state do get called via socket callback (the rest of the transitions occur with the connection mutex held). We can back out the atomicity later.
Signed-off-by Alex Elder <[email protected]>---
include/linux/ceph/messenger.h | 8 ++++-
net/ceph/messenger.c | 63
++++++++++++++++++++++++++++++++++++++++2 files changed, 69 insertions(+), 2 deletions(-)include/linux/ceph/messenger.h +5
-0@@ -137,14 +137,18 @@ struct ceph_connection { const struct ceph_connection_operations *ops; struct ceph_messenger *msgr; + + atomic_t sock_state; struct socket *sock; + struct ceph_entity_addr peer_addr; /* peer address */ + struct ceph_entity_addr peer_addr_for_me; + unsigned long flags; unsigned long state;const char
*error_msg; /* error message, if any */- struct
ceph_entity_addrpeer_addr; /* peer address */ structceph_entity_namepeer_name; /* peer name */
- structceph_entity_addrpeer_addr_for_me;
+
unsignedpeer_features;
u32connect_seq; /* identify the most recent connection attempt for this connection, client */net/ceph/messenger.c +5
-0@@ -29,6 +29,14 @@* the sender. */ +/* State values for ceph_connection->sock_state; NEW is assumed to be 0 */ + +#define CON_SOCK_STATE_NEW 0/* -> CLOSED */ +#define CON_SOCK_STATE_CLOSED 1/* -> CONNECTING */ +#define CON_SOCK_STATE_CONNECTING 2/* -> CONNECTED or -> CLOSING */
+#defineCON_SOCK_STATE_CONNECTED3 /* -> CLOSING or -> CLOSED */net/ceph/messenger.c +2
-0+#define CON_SOCK_STATE_CLOSING 4/* -> CLOSED */ +/* static tag bytes (protocol control messages) */ static char
tag_msg=CEPH_MSGR_TAG_MSG; static chartag_ack=CEPH_MSGR_TAG_ACK;net/ceph/messenger.c +27
-0@@ -147,6 +155,54 @@ void ceph_msgr_flush(void) } EXPORT_SYMBOL(ceph_msgr_flush); +/* Connection socket state transition functions */ + +static void con_sock_state_init (struct ceph_connection *con) +{ + int old_state; + + old_state = atomic_xchg(&con->sock_state, CON_SOCK_STATE_CLOSED); + if (WARN_ON(old_state != CON_SOCK_STATE_NEW)) + printk("%s: unexpected old state %d\n", __func__, old_state); +} + +static void con_sock_state_connecting (struct ceph_connection *con) +{ + int old_state; + + old_state = atomic_xchg(&con->sock_state,CON_SOCK_STATE_CONNECTING ); + if (WARN_ON(old_state != CON_SOCK_STATE_CLOSED)) + printk("%s: unexpected old state %d\n", __func__, old_state); +} + +static void con_sock_state_connected (struct ceph_connection *con) +{ + int old_state; + + old_state = atomic_xchg(&con->sock_state, CON_SOCK_STATE_CONNECTED); + if (WARN_ON(old_state != CON_SOCK_STATE_CONNECTING)) + printk("%s: unexpected old state %d\n", __func__,
old_state);net/ceph/messenger.c +10
-0+} + +static void con_sock_state_closing (struct ceph_connection *con) +{ + int old_state; + + old_state = atomic_xchg(&con->sock_state, CON_SOCK_STATE_CLOSING); + if (WARN_ON(old_state != CON_SOCK_STATE_CONNECTING && + old_state != CON_SOCK_STATE_CONNECTED)) + printk("%s: unexpected old state %d\n", __func__,
old_state);net/ceph/messenger.c +10
-0+} + +static void con_sock_state_closed (struct ceph_connection *con) +{ + int old_state; + + old_state = atomic_xchg(&con->sock_state, CON_SOCK_STATE_CLOSED); + if (WARN_ON(old_state != CON_SOCK_STATE_CONNECTED && + old_state != CON_SOCK_STATE_CLOSING)) + printk("%s: unexpected old state %d\n", __func__,
Quoted 3 lines
old_state);
+}
Hi Alex,
Single node setup can easily trigger above WARN_ON. I think this is because local TCP connection setup and shutdown is very fast, they finish immediately after sock->connect and sock->shutdown return.
Yan, Zheng
Quoted 48 lines
/*
* socket callback functionsnet/ceph/messenger.c +0
-0@@ -203,6 +259,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__);
+con_sock_state_closing(con);
if (test_and_set_bit(SOCK_CLOSED, &con->flags)== 0) { if (test_bit(CONNECTING, &con->state))con->error_msg= "connection failed";net/ceph/messenger.c +0
-0@@ -213,6 +270,7 @@ static void ceph_sock_state_change(struct sock *sk) break;case
TCP_ESTABLISHED:
dout("%s TCP_ESTABLISHED\n",__func__);
+con_sock_state_connected(con);queue_con(con);
break;
default: /* Everything else is uninteresting */net/ceph/messenger.c +5
-0@@ -277,6 +335,7 @@ static int ceph_tcp_connect(struct ceph_connection *con) return ret; } con->sock = sock; + con_sock_state_connecting(con); return 0; }@@ -341,6 +400,7 @@ static int con_close_socket(struct ceph_connection *con) rc = con->sock->ops->shutdown(con->sock, SHUT_RDWR); sock_release(con->sock); con->sock = NULL; + con_sock_state_closed(con); return rc; }@@ -460,6 +520,9 @@ void ceph_con_init(struct ceph_messenger *msgr, struct ceph_connection *con) memset(con, 0, sizeof(*con)); atomic_set(&con->nref, 1); con->msgr = msgr; + + con_sock_state_init(con); + mutex_init(&con->mutex); INIT_LIST_HEAD(&con->out_queue);INIT_LIST_HEAD(&con->out_sent);
--
1.7.5.4
Quoted 4 lines
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/13] libceph: start tracking connection socket state
Quoted 159 lines · Zheng
Quoted 153 lines · Alex Elder
Start explicitly keeping track of the state of a ceph connection's socket, separate from the state of the connection itself. Create placeholder functions to encapsulate the state transitions.
--------
| NEW* | transient initial state
--------
|con_sock_state_init()
v
----------
| CLOSED | initialized, but no socket (and no
---------- TCP connection)
^ \| \ con_sock_state_connecting() | ---------------------- | \ + con_sock_state_closed() \ |\ \ | \ \ | ----------- \ | | CLOSING | socket event; \ | ----------- await close \ | ^ | | | | | + con_sock_state_closing() | | / \ | | / --------------- | | / \ v | / -------------- | / -----------------| CONNECTING | socket created, TCP | | / -------------- connect initiated | | | con_sock_state_connected() | | v-------------
| CONNECTED | TCP connection established
-------------Make the socket state an atomic variable, reinforcing that it's a distinct transtion with no possible "intermediate/both" states. This is almost certainly overkill at this point, though the transitions into CONNECTED and CLOSING state do get called via socket callback (the rest of the transitions occur with the connection mutex held). We can back out the atomicity later.
Signed-off-by Alex Elder <[email protected]>---
include/linux/ceph/messenger.h | 8 ++++-
net/ceph/messenger.c | 63
++++++++++++++++++++++++++++++++++++++++2 files changed, 69 insertions(+), 2 deletions(-)include/linux/ceph/messenger.h +5
-0@@ -137,14 +137,18 @@ struct ceph_connection { const struct ceph_connection_operations *ops; struct ceph_messenger *msgr; + + atomic_t sock_state; struct socket *sock; + struct ceph_entity_addr peer_addr; /* peer address */ + struct ceph_entity_addr peer_addr_for_me; + unsigned long flags; unsigned long state;const char
*error_msg; /* error message, if any */- struct
ceph_entity_addrpeer_addr; /* peer address */ structceph_entity_namepeer_name; /* peer name */
- structceph_entity_addrpeer_addr_for_me;
+
unsignedpeer_features;
u32connect_seq; /* identify the most recent connection attempt for this connection, client */net/ceph/messenger.c +5
-0@@ -29,6 +29,14 @@* the sender. */ +/* State values for ceph_connection->sock_state; NEW is assumed to be 0 */ + +#define CON_SOCK_STATE_NEW 0/* -> CLOSED */ +#define CON_SOCK_STATE_CLOSED 1/* -> CONNECTING */ +#define CON_SOCK_STATE_CONNECTING 2/* -> CONNECTED or -> CLOSING */
+#defineCON_SOCK_STATE_CONNECTED3 /* -> CLOSING or -> CLOSED */net/ceph/messenger.c +2
-0+#define CON_SOCK_STATE_CLOSING 4/* -> CLOSED */ +/* static tag bytes (protocol control messages) */ static char
tag_msg=CEPH_MSGR_TAG_MSG; static chartag_ack=CEPH_MSGR_TAG_ACK;net/ceph/messenger.c +27
-0@@ -147,6 +155,54 @@ void ceph_msgr_flush(void) } EXPORT_SYMBOL(ceph_msgr_flush); +/* Connection socket state transition functions */ + +static void con_sock_state_init (struct ceph_connection *con) +{ + int old_state; + + old_state = atomic_xchg(&con->sock_state, CON_SOCK_STATE_CLOSED); + if (WARN_ON(old_state != CON_SOCK_STATE_NEW)) + printk("%s: unexpected old state %d\n", __func__, old_state); +} + +static void con_sock_state_connecting (struct ceph_connection *con) +{ + int old_state; + + old_state = atomic_xchg(&con->sock_state,CON_SOCK_STATE_CONNECTING ); + if (WARN_ON(old_state != CON_SOCK_STATE_CLOSED)) + printk("%s: unexpected old state %d\n", __func__, old_state); +} + +static void con_sock_state_connected (struct ceph_connection *con) +{ + int old_state; + + old_state = atomic_xchg(&con->sock_state, CON_SOCK_STATE_CONNECTED); + if (WARN_ON(old_state != CON_SOCK_STATE_CONNECTING)) + printk("%s: unexpected old state %d\n", __func__,
old_state);net/ceph/messenger.c +10
-0+} + +static void con_sock_state_closing (struct ceph_connection *con) +{ + int old_state; + + old_state = atomic_xchg(&con->sock_state, CON_SOCK_STATE_CLOSING); + if (WARN_ON(old_state != CON_SOCK_STATE_CONNECTING && + old_state != CON_SOCK_STATE_CONNECTED)) + printk("%s: unexpected old state %d\n", __func__,
old_state);net/ceph/messenger.c +10
-0+} + +static void con_sock_state_closed (struct ceph_connection *con) +{ + int old_state; + + old_state = atomic_xchg(&con->sock_state, CON_SOCK_STATE_CLOSED); + if (WARN_ON(old_state != CON_SOCK_STATE_CONNECTED && + old_state != CON_SOCK_STATE_CLOSING)) + printk("%s: unexpected old state %d\n", __func__,Quoted 3 lines
old_state);
+}Hi Alex,
Quoted 3 lines
Single node setup can easily trigger above
WARN_ON. I think this is because local TCP connection setup and shutdown is very fast, they finish immediately aftersock->connectandsock->shutdownreturn.
Yep. This was just fixed yesterday, in the testing-next branch, by 'libceph: transition socket state prior to actual connect'.
Are you still hitting the bio null deref?
Thanks-
sage
Quoted 61 lines
Yan, Zheng
Quoted 48 lines
/*
* socket callback functionsnet/ceph/messenger.c +0
-0@@ -203,6 +259,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__);
+con_sock_state_closing(con);
if (test_and_set_bit(SOCK_CLOSED, &con->flags)== 0) { if (test_bit(CONNECTING, &con->state))con->error_msg= "connection failed";net/ceph/messenger.c +0
-0@@ -213,6 +270,7 @@ static void ceph_sock_state_change(struct sock *sk) break;case
TCP_ESTABLISHED:
dout("%s TCP_ESTABLISHED\n",__func__);
+con_sock_state_connected(con);queue_con(con);
break;
default: /* Everything else is uninteresting */net/ceph/messenger.c +5
-0@@ -277,6 +335,7 @@ static int ceph_tcp_connect(struct ceph_connection *con) return ret; } con->sock = sock; + con_sock_state_connecting(con); return 0; }@@ -341,6 +400,7 @@ static int con_close_socket(struct ceph_connection *con) rc = con->sock->ops->shutdown(con->sock, SHUT_RDWR); sock_release(con->sock); con->sock = NULL; + con_sock_state_closed(con); return rc; }@@ -460,6 +520,9 @@ void ceph_con_init(struct ceph_messenger *msgr, struct ceph_connection *con) memset(con, 0, sizeof(*con)); atomic_set(&con->nref, 1); con->msgr = msgr; + + con_sock_state_init(con); + mutex_init(&con->mutex); INIT_LIST_HEAD(&con->out_queue);INIT_LIST_HEAD(&con->out_sent);
--
1.7.5.4Quoted 4 lines
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/13] libceph: start tracking connection socket state
Quoted 5 lines · Sage Weil
Yep. This was just fixed yesterday, in the testing-next branch, by 'libceph: transition socket state prior to actual connect'.
Are you still hitting the bio null deref?
No,
Cheers
Yan, Zheng
Re: [PATCH 09/13] libceph: start tracking connection socket state
Quoted 10 lines · Zheng
Quoted 5 lines · Sage Weil
Yep. This was just fixed yesterday, in the testing-next branch, by 'libceph: transition socket state prior to actual connect'.
Are you still hitting the bio null deref?
No,
Cheers
Yan, Zheng
Would you be able to narrow down exactly what fixed the bio null problem? Are you able to easily reproduce it? Are you running the master branch, or testing, or what?
Thank you.
-Alex
Re: [PATCH 09/13] libceph: start tracking connection socket state
Quoted 7 lines · Alex Elder
Quoted 6 lines · Zheng
Quoted 5 lines · Sage Weil
Yep. This was just fixed yesterday, in the testing-next branch, by 'libceph: transition socket state prior to actual connect'.
Are you still hitting the bio null deref?
Quoted 9 lines
Quoted 4 lines
No,
Cheers
Yan, ZhengWould you be able to narrow down exactly what fixed the bio null problem? Are you able to easily reproduce it? Are you running the master branch, or testing, or what?
The 'clear msg->bio_iter' patch. Without it, I always got below Oops when xfstest reached the 49th test case.
---
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.610673] libceph: osd3 10.239.36.78:6807 socket closed Jun 13 09:34:14 zyan5-desk kernel: [ 1192.612784] BUG: unable to handle kernel NULL pointer dereference at 0000000000000048 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.614482] IP: [<ffffffffa062bc98>] con_work+0x19a8/0x2c80 [libceph] Jun 13 09:34:14 zyan5-desk kernel: [ 1192.616120] PGD 137d91067 PUD 137d92067 PMD 0 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.617749] Oops: 0000 [#1] SMP Jun 13 09:34:14 zyan5-desk kernel: [ 1192.619404] CPU 6 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.621059] Pid: 8239, comm: kworker/6:6 Not tainted 3.5.0-rc2+ #91 Dell Inc. Studio XPS 8000/0X231R Jun 13 09:34:14 zyan5-desk kernel: [ 1192.622700] RIP: 0010:[<ffffffffa062bc98>] [<ffffffffa062bc98>] con_work+0x19a8/0x2c80 [libceph] Jun 13 09:34:14 zyan5-desk kernel: [ 1192.624371] RSP: 0018:ffff88000efb7cb0 EFLAGS: 00010246 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.626119] RAX: 0000000000000000 RBX: ffff880122370030 RCX: 000000000006a000 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.627694] RDX: 0000000000000000 RSI: 0000000000016000 RDI: ffff880122370420 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.629267] RBP: ffff88000efb7e00 R08: 00000000418d4bd6 R09: 0000000000000000 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.630811] R10: 0000000000000002 R11: 0000000000000dc7 R12: 0000000000080000 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.632441] R13: 0000000000000000 R14: ffff88003af1bd00 R15: ffffea00044aec40 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.633996] FS: 0000000000000000(0000) GS:ffff88013fd80000(0000) knlGS:0000000000000000 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.635621] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b Jun 13 09:34:14 zyan5-desk kernel: [ 1192.637239] CR2: 0000000000000048 CR3: 0000000137d90000 CR4: 00000000000007e0 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.638841] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.640493] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.642255] Process kworker/6:6 (pid: 8239, threadinfo ffff88000efb6000, task ffff880041242e60) Jun 13 09:34:14 zyan5-desk kernel: [ 1192.643917] Stack: Jun 13 09:34:14 zyan5-desk kernel: [ 1192.645588] ffff880000000001 ffffffff00000006 ffff88000efb7dac 0000000600000002 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.647270] ffff88000efb7d30 ffffffff810930d1 ffff88013b00a400 0000000000000087 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.648923] 0000000eed1e4f54 ffff880041242e60 ffff88000efb7d30 ffffffff00000000 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.650563] Call Trace: Jun 13 09:34:14 zyan5-desk kernel: [ 1192.652162] [<ffffffff810930d1>] ? update_curr+0x141/0x1f0 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.653688] [<ffffffffa062a2f0>] ? ceph_msg_new+0x2d0/0x2d0 [libceph] Jun 13 09:34:14 zyan5-desk kernel: [ 1192.655203] [<ffffffff81075f6d>] process_one_work+0x11d/0x470 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.656714] [<ffffffff81077069>] worker_thread+0x159/0x340 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.658278] [<ffffffff81076f10>] ? manage_workers+0x230/0x230 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.659606] [<ffffffff8107bee3>] kthread+0x93/0xa0 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.660932] [<ffffffff8160d9e4>] kernel_thread_helper+0x4/0x10 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.662293] [<ffffffff8107be50>] ? kthread_freezable_should_stop+0x70/0x70 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.663670] [<ffffffff8160d9e0>] ? gs_change+0x13/0x13 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.665027] Code: ef fb ff ff 0f 1f 80 00 00 00 00 49 83 be 90 00 00 00 00 0f 84 7a 01 00 00 49 63 86 a0 00 00 00 49 8b 96 98 00 00 00 48 c1 e0 04 <48> 03 42 48 4c 8b 38 8b 48 0c 8b 50 08 e9 08 f8 ff ff 49 89 86 Jun 13 09:34:14 zyan5-desk kernel: [ 1192.666570] RIP [<ffffffffa062bc98>] con_work+0x19a8/0x2c80 [libceph] Jun 13 09:34:14 zyan5-desk kernel: [ 1192.668232] RSP <ffff88000efb7cb0> Jun 13 09:34:14 zyan5-desk kernel: [ 1192.669795] CR2: 0000000000000048