* [PATCH 1/4] ceph: make use of "else" where appropriate
2012-02-29 3:09 [PATCH 0/4] ceph: minor cleanups Alex Elder
@ 2012-02-29 3:12 ` Alex Elder
2012-02-29 3:12 ` [PATCH 2/4] ceph: kill addr_str_lock spinlock; use atomic instead Alex Elder
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Alex Elder @ 2012-02-29 3:12 UTC (permalink / raw)
To: ceph-devel
Rearrange ceph_tcp_connect() a bit, making use of "else" rather than
re-testing a value with consecutive "if" statements. Don't record a
connection's socket pointer unless the connect operation is
successful.
Signed-off-by: Alex Elder <elder@dreamhost.com>
---
net/ceph/messenger.c | 11 ++++-------
1 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index 068cb7e..c9a413a 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -247,7 +247,6 @@ static struct socket *ceph_tcp_connect(struct
ceph_connection *con)
IPPROTO_TCP, &sock);
if (ret)
return ERR_PTR(ret);
- con->sock = sock;
sock->sk->sk_allocation = GFP_NOFS;
#ifdef CONFIG_LOCKDEP
@@ -264,18 +263,16 @@ static struct socket *ceph_tcp_connect(struct
ceph_connection *con)
dout("connect %s EINPROGRESS sk_state = %u\n",
ceph_pr_addr(&con->peer_addr.in_addr),
sock->sk->sk_state);
- ret = 0;
- }
- if (ret < 0) {
+ } else if (ret < 0) {
pr_err("connect %s error %d\n",
ceph_pr_addr(&con->peer_addr.in_addr), ret);
sock_release(sock);
- con->sock = NULL;
con->error_msg = "connect error";
- }
- if (ret < 0)
return ERR_PTR(ret);
+ }
+ con->sock = sock;
+
return sock;
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/4] ceph: kill addr_str_lock spinlock; use atomic instead
2012-02-29 3:09 [PATCH 0/4] ceph: minor cleanups Alex Elder
2012-02-29 3:12 ` [PATCH 1/4] ceph: make use of "else" where appropriate Alex Elder
@ 2012-02-29 3:12 ` Alex Elder
2012-02-29 3:12 ` [PATCH 3/4] ceph: eliminate some needless casts Alex Elder
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Alex Elder @ 2012-02-29 3:12 UTC (permalink / raw)
To: ceph-devel
A spinlock is used to protect a value used for selecting an array
index for a string used for formatting a socket address for human
consumption. The index is reset to 0 if it ever reaches the maximum
index value.
Instead, use an ever-increasing atomic variable as a sequence
number, and compute the array index by masking off all but the
sequence number's lowest bits. Make the number of entries in the
array a power of two to allow the use of such a mask (to avoid jumps
in the index value when the sequence number wraps).
The length of these strings is somewhat arbitrarily set at 60 bytes.
The worst-case length of a string produced is 54 bytes, for an IPv6
address that can't be shortened, e.g.:
[1234:5678:9abc:def0:1111:2222:123.234.210.100]:32767
Change it so we arbitrarily use 64 bytes instead; if nothing else
it will make the array of these line up better in hex dumps.
Rename a few things to reinforce the distinction between the number
of strings in the array and the length of individual strings.
Signed-off-by: Alex Elder <elder@newdream.net>
---
net/ceph/messenger.c | 21 ++++++++++-----------
1 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index c9a413a..8edfd1a 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -44,13 +44,16 @@ static void con_work(struct work_struct *);
static void ceph_fault(struct ceph_connection *con);
/*
- * nicely render a sockaddr as a string.
+ * Nicely render a sockaddr as a string. An array of formatted
+ * strings is used, to approximate reentrancy.
*/
-#define MAX_ADDR_STR 20
-#define MAX_ADDR_STR_LEN 60
-static char addr_str[MAX_ADDR_STR][MAX_ADDR_STR_LEN];
-static DEFINE_SPINLOCK(addr_str_lock);
-static int last_addr_str;
+#define ADDR_STR_COUNT_LOG 5 /* log2(# address strings in array) */
+#define ADDR_STR_COUNT (1 << ADDR_STR_COUNT_LOG)
+#define ADDR_STR_COUNT_MASK (ADDR_STR_COUNT - 1)
+#define MAX_ADDR_STR_LEN 64 /* 54 is enough */
+
+static char addr_str[ADDR_STR_COUNT][MAX_ADDR_STR_LEN];
+static atomic_t addr_str_seq = ATOMIC_INIT(0);
static struct page *zero_page; /* used in certain error cases */
static void *zero_page_address; /* kernel virtual addr of zero_page */
@@ -62,11 +65,7 @@ const char *ceph_pr_addr(const struct
sockaddr_storage *ss)
struct sockaddr_in *in4 = (void *)ss;
struct sockaddr_in6 *in6 = (void *)ss;
- spin_lock(&addr_str_lock);
- i = last_addr_str++;
- if (last_addr_str == MAX_ADDR_STR)
- last_addr_str = 0;
- spin_unlock(&addr_str_lock);
+ i = atomic_inc_return(&addr_str_seq) & ADDR_STR_COUNT_MASK;
s = addr_str[i];
switch (ss->ss_family) {
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/4] ceph: eliminate some needless casts
2012-02-29 3:09 [PATCH 0/4] ceph: minor cleanups Alex Elder
2012-02-29 3:12 ` [PATCH 1/4] ceph: make use of "else" where appropriate Alex Elder
2012-02-29 3:12 ` [PATCH 2/4] ceph: kill addr_str_lock spinlock; use atomic instead Alex Elder
@ 2012-02-29 3:12 ` Alex Elder
2012-02-29 3:12 ` [PATCH 4/4] ceph: eliminate some abusive casts Alex Elder
2012-03-02 19:25 ` [PATCH 0/4] ceph: minor cleanups Sage Weil
4 siblings, 0 replies; 6+ messages in thread
From: Alex Elder @ 2012-02-29 3:12 UTC (permalink / raw)
To: ceph-devel
This eliminates type casts in some places where they are not
required.
Signed-off-by: Alex Elder <elder@newdream.net>
---
net/ceph/messenger.c | 21 ++++++++++-----------
1 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index 8edfd1a..e95756a 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -70,13 +70,13 @@ const char *ceph_pr_addr(const struct
sockaddr_storage *ss)
switch (ss->ss_family) {
case AF_INET:
- snprintf(s, MAX_ADDR_STR_LEN, "%pI4:%u", &in4->sin_addr,
- (unsigned int)ntohs(in4->sin_port));
+ snprintf(s, MAX_ADDR_STR_LEN, "%pI4:%hu", &in4->sin_addr,
+ ntohs(in4->sin_port));
break;
case AF_INET6:
- snprintf(s, MAX_ADDR_STR_LEN, "[%pI6c]:%u", &in6->sin6_addr,
- (unsigned int)ntohs(in6->sin6_port));
+ snprintf(s, MAX_ADDR_STR_LEN, "[%pI6c]:%hu", &in6->sin6_addr,
+ ntohs(in6->sin6_port));
break;
default:
@@ -155,8 +155,8 @@ 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)
{
- struct ceph_connection *con =
- (struct ceph_connection *)sk->sk_user_data;
+ 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",
con, con->state);
@@ -185,8 +185,7 @@ static void ceph_write_space(struct sock *sk)
/* socket's state has changed */
static void ceph_state_change(struct sock *sk)
{
- struct ceph_connection *con =
- (struct ceph_connection *)sk->sk_user_data;
+ struct ceph_connection *con = sk->sk_user_data;
dout("ceph_state_change %p state = %lu sk_state = %u\n",
con, con->state, sk->sk_state);
@@ -221,7 +220,7 @@ static void set_sock_callbacks(struct socket *sock,
struct ceph_connection *con)
{
struct sock *sk = sock->sk;
- sk->sk_user_data = (void *)con;
+ 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;
@@ -548,7 +547,7 @@ static void prepare_write_message(struct
ceph_connection *con)
/* fill in crc (except data pages), footer */
con->out_msg->hdr.crc =
- cpu_to_le32(crc32c(0, (void *)&m->hdr,
+ cpu_to_le32(crc32c(0, &m->hdr,
sizeof(m->hdr) - sizeof(m->hdr.crc)));
con->out_msg->footer.flags = CEPH_MSG_FOOTER_COMPLETE;
con->out_msg->footer.front_crc =
@@ -1643,7 +1642,7 @@ static int read_partial_message(struct
ceph_connection *con)
return ret;
con->in_base_pos += ret;
if (con->in_base_pos == sizeof(con->in_hdr)) {
- u32 crc = crc32c(0, (void *)&con->in_hdr,
+ u32 crc = crc32c(0, &con->in_hdr,
sizeof(con->in_hdr) - sizeof(con->in_hdr.crc));
if (crc != le32_to_cpu(con->in_hdr.crc)) {
pr_err("read_partial_message bad hdr "
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/4] ceph: eliminate some abusive casts
2012-02-29 3:09 [PATCH 0/4] ceph: minor cleanups Alex Elder
` (2 preceding siblings ...)
2012-02-29 3:12 ` [PATCH 3/4] ceph: eliminate some needless casts Alex Elder
@ 2012-02-29 3:12 ` Alex Elder
2012-03-02 19:25 ` [PATCH 0/4] ceph: minor cleanups Sage Weil
4 siblings, 0 replies; 6+ messages in thread
From: Alex Elder @ 2012-02-29 3:12 UTC (permalink / raw)
To: ceph-devel
This fixes some spots where a type cast to (void *) was used as
as a universal type hiding mechanism. Instead, properly cast the
type to the intended target type.
Signed-off-by: Alex Elder <elder@newdream.net>
---
net/ceph/messenger.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index e95756a..c410c55 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -62,8 +62,8 @@ const char *ceph_pr_addr(const struct sockaddr_storage
*ss)
{
int i;
char *s;
- struct sockaddr_in *in4 = (void *)ss;
- struct sockaddr_in6 *in6 = (void *)ss;
+ struct sockaddr_in *in4 = (struct sockaddr_in *) ss;
+ struct sockaddr_in6 *in6 = (struct sockaddr_in6 *) ss;
i = atomic_inc_return(&addr_str_seq) & ADDR_STR_COUNT_MASK;
s = addr_str[i];
@@ -1108,8 +1108,8 @@ static void addr_set_port(struct sockaddr_storage
*ss, int p)
static int ceph_pton(const char *str, size_t len, struct
sockaddr_storage *ss,
char delim, const char **ipend)
{
- struct sockaddr_in *in4 = (void *)ss;
- struct sockaddr_in6 *in6 = (void *)ss;
+ struct sockaddr_in *in4 = (struct sockaddr_in *) ss;
+ struct sockaddr_in6 *in6 = (struct sockaddr_in6 *) ss;
memset(ss, 0, sizeof(*ss));
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/4] ceph: minor cleanups
2012-02-29 3:09 [PATCH 0/4] ceph: minor cleanups Alex Elder
` (3 preceding siblings ...)
2012-02-29 3:12 ` [PATCH 4/4] ceph: eliminate some abusive casts Alex Elder
@ 2012-03-02 19:25 ` Sage Weil
4 siblings, 0 replies; 6+ messages in thread
From: Sage Weil @ 2012-03-02 19:25 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
This set looks good,
Reviewed-by: Sage Weil <sage@newdream.net>
On Tue, 28 Feb 2012, Alex Elder wrote:
> These are mostly small cleanups. Only the second one has any
> significant changes in it--using a masked atomic rather than
> a spinlocked mod operation for determining an index to use
> on a fixed-size array.
>
> -Alex
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread