Re: [PATCH 09/13] libceph: start tracking connection socket state

2012-06-12 Thread Alex Elder
On 06/12/2012 12:02 AM, Yan, Zheng wrote:
 On 06/12/2012 01:00 PM, Sage Weil wrote:
 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

--
To unsubscribe from this list: send the line unsubscribe ceph-devel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/13] libceph: start tracking connection socket state

2012-06-12 Thread Yan, Zheng
On 06/13/2012 12:58 AM, Alex Elder wrote:
 On 06/12/2012 12:02 AM, Yan, Zheng wrote:
 On 06/12/2012 01:00 PM, Sage Weil wrote:
 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?
 
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 0048
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.614482] IP: [a062bc98] 
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:  [#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:[a062bc98]  [a062bc98] con_work+0x19a8/0x2c80 [libceph]
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.624371] RSP: 0018:88000efb7cb0  
EFLAGS: 00010246
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.626119] RAX:  RBX: 
880122370030 RCX: 0006a000
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.627694] RDX:  RSI: 
00016000 RDI: 880122370420
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.629267] RBP: 88000efb7e00 R08: 
418d4bd6 R09: 
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.630811] R10: 0002 R11: 
0dc7 R12: 0008
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.632441] R13:  R14: 
88003af1bd00 R15: ea00044aec40
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.633996] FS:  () 
GS:88013fd8() knlGS:
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.635621] CS:  0010 DS:  ES:  
CR0: 8005003b
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.637239] CR2: 0048 CR3: 
000137d9 CR4: 07e0
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.638841] DR0:  DR1: 
 DR2: 
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.640493] DR3:  DR6: 
0ff0 DR7: 0400
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.642255] Process kworker/6:6 (pid: 
8239, threadinfo 88000efb6000, task 880041242e60)
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.643917] Stack:
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.645588]  8801 
0006 88000efb7dac 00060002
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.647270]  88000efb7d30 
810930d1 88013b00a400 0087
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.648923]  000eed1e4f54 
880041242e60 88000efb7d30 
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.650563] Call Trace:
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.652162]  [810930d1] ? 
update_curr+0x141/0x1f0
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.653688]  [a062a2f0] ? 
ceph_msg_new+0x2d0/0x2d0 [libceph]
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.655203]  [81075f6d] 
process_one_work+0x11d/0x470
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.656714]  [81077069] 
worker_thread+0x159/0x340
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.658278]  [81076f10] ? 
manage_workers+0x230/0x230
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.659606]  [8107bee3] 
kthread+0x93/0xa0
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.660932]  [8160d9e4] 
kernel_thread_helper+0x4/0x10
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.662293]  [8107be50] ? 
kthread_freezable_should_stop+0x70/0x70
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.663670]  [8160d9e0] ? 
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  [a062bc98] 
con_work+0x19a8/0x2c80 [libceph]
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.668232]  RSP 88000efb7cb0
Jun 13 09:34:14 zyan5-desk kernel: [ 1192.669795] CR2: 0048

--
To unsubscribe from this list: send the line unsubscribe ceph-devel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/13] libceph: start tracking connection socket state

2012-06-11 Thread Yan, Zheng
On Thu, May 31, 2012 at 3:35 AM, Alex Elder el...@inktank.com wrote:
 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 el...@inktank.com
 ---
  include/linux/ceph/messenger.h |    8 -
  net/ceph/messenger.c           |   63
 
  2 files changed, 69 insertions(+), 2 deletions(-)

 diff --git a/include/linux/ceph/messenger.h b/include/linux/ceph/messenger.h
 index 920235e..5e852f4 100644
 --- a/include/linux/ceph/messenger.h
 +++ b/include/linux/ceph/messenger.h
 @@ -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 */
 diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
 index 29055df..7e11b07 100644
 --- a/net/ceph/messenger.c
 +++ b/net/ceph/messenger.c
 @@ -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;
 @@ -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 = 

Re: [PATCH 09/13] libceph: start tracking connection socket state

2012-06-11 Thread Sage Weil
On Tue, 12 Jun 2012, Yan, Zheng wrote:
 On Thu, May 31, 2012 at 3:35 AM, Alex Elder el...@inktank.com wrote:
  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 el...@inktank.com
  ---
   include/linux/ceph/messenger.h |    8 -
   net/ceph/messenger.c           |   63
  
   2 files changed, 69 insertions(+), 2 deletions(-)
 
  diff --git a/include/linux/ceph/messenger.h b/include/linux/ceph/messenger.h
  index 920235e..5e852f4 100644
  --- a/include/linux/ceph/messenger.h
  +++ b/include/linux/ceph/messenger.h
  @@ -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 */
  diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
  index 29055df..7e11b07 100644
  --- a/net/ceph/messenger.c
  +++ b/net/ceph/messenger.c
  @@ -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;
  @@ -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 

Re: [PATCH 09/13] libceph: start tracking connection socket state

2012-06-11 Thread Yan, Zheng
On 06/12/2012 01:00 PM, Sage Weil wrote:
 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


--
To unsubscribe from this list: send the line unsubscribe ceph-devel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/13] libceph: start tracking connection socket state

2012-06-01 Thread Alex Elder

On 05/31/2012 11:28 PM, Sage Weil wrote:

On Wed, 30 May 2012, Alex Elder wrote:

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
 -


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


Reviewed-by: Sage Weils...@inktank.com



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 Elderel...@inktank.com
---
  include/linux/ceph/messenger.h |8 -
  net/ceph/messenger.c   |   63

  2 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/include/linux/ceph/messenger.h b/include/linux/ceph/messenger.h
index 920235e..5e852f4 100644
--- a/include/linux/ceph/messenger.h
+++ b/include/linux/ceph/messenger.h
@@ -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 */
diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index 29055df..7e11b07 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -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;
@@ -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);
+  

Re: [PATCH 09/13] libceph: start tracking connection socket state

2012-05-31 Thread Sage Weil
On Wed, 30 May 2012, Alex Elder wrote:
 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
 -

Can we put this beautiful pictures in the header next to the states?

Reviewed-by: Sage Weil s...@inktank.com

 
 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 el...@inktank.com
 ---
  include/linux/ceph/messenger.h |8 -
  net/ceph/messenger.c   |   63
 
  2 files changed, 69 insertions(+), 2 deletions(-)
 
 diff --git a/include/linux/ceph/messenger.h b/include/linux/ceph/messenger.h
 index 920235e..5e852f4 100644
 --- a/include/linux/ceph/messenger.h
 +++ b/include/linux/ceph/messenger.h
 @@ -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 */
 diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
 index 29055df..7e11b07 100644
 --- a/net/ceph/messenger.c
 +++ b/net/ceph/messenger.c
 @@ -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_CLOSED1   /* - CONNECTING */
 +#define CON_SOCK_STATE_CONNECTING2   /* - 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;
 @@ -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 

[PATCH 09/13] libceph: start tracking connection socket state

2012-05-30 Thread 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 el...@inktank.com
---
 include/linux/ceph/messenger.h |8 -
 net/ceph/messenger.c   |   63 


 2 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/include/linux/ceph/messenger.h b/include/linux/ceph/messenger.h
index 920235e..5e852f4 100644
--- a/include/linux/ceph/messenger.h
+++ b/include/linux/ceph/messenger.h
@@ -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 */
diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index 29055df..7e11b07 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -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;
@@ -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 !=