* Four miscellaneous patches
@ 2013-02-22 17:15 Alex Elder
2013-02-22 17:17 ` [PATCH] rbd: ignore zero-length requests Alex Elder
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Alex Elder @ 2013-02-22 17:15 UTC (permalink / raw)
To: ceph-devel
The following patches address some issues that were found while
investigating a kernel rbd client performance issue this week.
These patches are available in the branch "test/wip-4234,5,7,8"
in the ceph-client git repository.
-Alex
[PATCH] rbd: ignore zero-length requests
[PATCH] rbd: barriers are hard
[PATCH] rbd: normalize dout() calls
[PATCH] libceph: define connection flag helpers
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] rbd: ignore zero-length requests
2013-02-22 17:15 Four miscellaneous patches Alex Elder
@ 2013-02-22 17:17 ` Alex Elder
2013-02-22 17:17 ` [PATCH] rbd: barriers are hard Alex Elder
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Alex Elder @ 2013-02-22 17:17 UTC (permalink / raw)
To: ceph-devel
The old request code simply ignored zero-length requests. We should
still operate that same way to avoid any changes in behavior. We
can implement handling for special zero-length requests separately
(see http://tracker.ceph.com/issues/4236).
Add some assertions based on this new constraint.
This resolves:
http://tracker.ceph.com/issues/4237
Signed-off-by: Alex Elder <elder@inktank.com>
---
drivers/block/rbd.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index b0eea3e..3cc003b 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1560,6 +1560,7 @@ static int rbd_img_request_fill_bio(struct
rbd_img_request *img_request,
image_offset = img_request->offset;
rbd_assert(image_offset == bio_list->bi_sector << SECTOR_SHIFT);
resid = img_request->length;
+ rbd_assert(resid > 0);
while (resid) {
const char *object_name;
unsigned int clone_size;
@@ -1627,8 +1628,10 @@ static void rbd_img_obj_callback(struct
rbd_obj_request *obj_request)
bool more = true;
img_request = obj_request->img_request;
+
rbd_assert(img_request != NULL);
rbd_assert(img_request->rq != NULL);
+ rbd_assert(img_request->obj_request_count > 0);
rbd_assert(which != BAD_WHICH);
rbd_assert(which < img_request->obj_request_count);
rbd_assert(which >= img_request->next_completion);
@@ -1918,6 +1921,19 @@ static void rbd_request_fn(struct request_queue *q)
/* Ignore any non-FS requests that filter through. */
if (rq->cmd_type != REQ_TYPE_FS) {
+ dout("%s: non-fs request type %d\n", __func__,
+ (int) rq->cmd_type);
+ __blk_end_request_all(rq, 0);
+ continue;
+ }
+
+ /* Ignore/skip any zero-length requests */
+
+ offset = (u64) blk_rq_pos(rq) << SECTOR_SHIFT;
+ length = (u64) blk_rq_bytes(rq);
+
+ if (!length) {
+ dout("%s: zero-length request\n", __func__);
__blk_end_request_all(rq, 0);
continue;
}
@@ -1947,9 +1963,6 @@ static void rbd_request_fn(struct request_queue *q)
goto end_request;
}
- offset = (u64) blk_rq_pos(rq) << SECTOR_SHIFT;
- length = (u64) blk_rq_bytes(rq);
-
result = -EINVAL;
if (WARN_ON(offset && length > U64_MAX - offset + 1))
goto end_request; /* Shouldn't happen */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] rbd: barriers are hard
2013-02-22 17:15 Four miscellaneous patches Alex Elder
2013-02-22 17:17 ` [PATCH] rbd: ignore zero-length requests Alex Elder
@ 2013-02-22 17:17 ` Alex Elder
2013-02-22 17:17 ` [PATCH] rbd: normalize dout() calls Alex Elder
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Alex Elder @ 2013-02-22 17:17 UTC (permalink / raw)
To: ceph-devel
Let's go shopping!
I'm afraid this may not have gotten it right:
07741308 rbd: add barriers near done flag operations
The smp_wmb() should have been done *before* setting the done flag,
to ensure all other data was valid before marking the object request
done.
Switch to use atomic_inc_return() here to set the done flag, which
allows us to verify we don't mark something done more than once.
Doing this also implies general barriers before and after the call.
And although a read memory barrier might have been sufficient before
reading the done flag, convert this to a full memory barrier just
to put this issue to bed.
This resolves:
http://tracker.ceph.com/issues/4238
Signed-off-by: Alex Elder <elder@inktank.com>
---
drivers/block/rbd.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 3cc003b..bd6078b 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1226,13 +1226,22 @@ static void obj_request_done_init(struct
rbd_obj_request *obj_request)
static void obj_request_done_set(struct rbd_obj_request *obj_request)
{
- atomic_set(&obj_request->done, 1);
- smp_wmb();
+ int done;
+
+ done = atomic_inc_return(&obj_request->done);
+ if (done > 1) {
+ struct rbd_img_request *img_request = obj_request->img_request;
+ struct rbd_device *rbd_dev;
+
+ rbd_dev = img_request ? img_request->rbd_dev : NULL;
+ rbd_warn(rbd_dev, "obj_request %p was already done\n",
+ obj_request);
+ }
}
static bool obj_request_done_test(struct rbd_obj_request *obj_request)
{
- smp_rmb();
+ smp_mb();
return atomic_read(&obj_request->done) != 0;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] rbd: normalize dout() calls
2013-02-22 17:15 Four miscellaneous patches Alex Elder
2013-02-22 17:17 ` [PATCH] rbd: ignore zero-length requests Alex Elder
2013-02-22 17:17 ` [PATCH] rbd: barriers are hard Alex Elder
@ 2013-02-22 17:17 ` Alex Elder
2013-02-22 17:17 ` [PATCH] libceph: define connection flag helpers Alex Elder
2013-02-25 19:11 ` Four miscellaneous patches Josh Durgin
4 siblings, 0 replies; 6+ messages in thread
From: Alex Elder @ 2013-02-22 17:17 UTC (permalink / raw)
To: ceph-devel
Add dout() calls to facilitate tracing of image and object requests.
Change a few existing calls so they use __func__ rather than the
hard-coded function name. Have calls always add ":" after the name
of the function, and prefix pointer values with a consistent tag
indicating what it represents. (Note that there remain some older
dout() calls that are left untouched by this patch.)
Issue a warning if rbd_osd_write_callback() ever gets a short write.
This resolves:
http://tracker.ceph.com/issues/4235
Signed-off-by: Alex Elder <elder@inktank.com>
---
drivers/block/rbd.c | 66
+++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 61 insertions(+), 5 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index bd6078b..a9c86ca 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -443,7 +443,7 @@ static struct rbd_client *rbd_client_create(struct
ceph_options *ceph_opts)
struct rbd_client *rbdc;
int ret = -ENOMEM;
- dout("rbd_client_create\n");
+ dout("%s:\n", __func__);
rbdc = kmalloc(sizeof(struct rbd_client), GFP_KERNEL);
if (!rbdc)
goto out_opt;
@@ -467,8 +467,8 @@ static struct rbd_client *rbd_client_create(struct
ceph_options *ceph_opts)
spin_unlock(&rbd_client_list_lock);
mutex_unlock(&ctl_mutex);
+ dout("%s: rbdc %p\n", __func__, rbdc);
- dout("rbd_client_create created %p\n", rbdc);
return rbdc;
out_err:
@@ -479,6 +479,8 @@ out_mutex:
out_opt:
if (ceph_opts)
ceph_destroy_options(ceph_opts);
+ dout("%s: error %d\n", __func__, ret);
+
return ERR_PTR(ret);
}
@@ -605,7 +607,7 @@ static void rbd_client_release(struct kref *kref)
{
struct rbd_client *rbdc = container_of(kref, struct rbd_client, kref);
- dout("rbd_release_client %p\n", rbdc);
+ dout("%s: rbdc %p\n", __func__, rbdc);
spin_lock(&rbd_client_list_lock);
list_del(&rbdc->node);
spin_unlock(&rbd_client_list_lock);
@@ -1064,6 +1066,8 @@ out_err:
static void rbd_obj_request_get(struct rbd_obj_request *obj_request)
{
+ dout("%s: obj %p (was %d)\n", __func__, obj_request,
+ atomic_read(&obj_request->kref.refcount));
kref_get(&obj_request->kref);
}
@@ -1071,11 +1075,15 @@ static void rbd_obj_request_destroy(struct kref
*kref);
static void rbd_obj_request_put(struct rbd_obj_request *obj_request)
{
rbd_assert(obj_request != NULL);
+ dout("%s: obj %p (was %d)\n", __func__, obj_request,
+ atomic_read(&obj_request->kref.refcount));
kref_put(&obj_request->kref, rbd_obj_request_destroy);
}
static void rbd_img_request_get(struct rbd_img_request *img_request)
{
+ dout("%s: img %p (was %d)\n", __func__, img_request,
+ atomic_read(&img_request->kref.refcount));
kref_get(&img_request->kref);
}
@@ -1083,6 +1091,8 @@ static void rbd_img_request_destroy(struct kref
*kref);
static void rbd_img_request_put(struct rbd_img_request *img_request)
{
rbd_assert(img_request != NULL);
+ dout("%s: img %p (was %d)\n", __func__, img_request,
+ atomic_read(&img_request->kref.refcount));
kref_put(&img_request->kref, rbd_img_request_destroy);
}
@@ -1097,6 +1107,8 @@ static inline void rbd_img_obj_request_add(struct
rbd_img_request *img_request,
rbd_assert(obj_request->which != BAD_WHICH);
img_request->obj_request_count++;
list_add_tail(&obj_request->links, &img_request->obj_requests);
+ dout("%s: img %p obj %p w=%u\n", __func__, img_request, obj_request,
+ obj_request->which);
}
static inline void rbd_img_obj_request_del(struct rbd_img_request
*img_request,
@@ -1104,6 +1116,8 @@ static inline void rbd_img_obj_request_del(struct
rbd_img_request *img_request,
{
rbd_assert(obj_request->which != BAD_WHICH);
+ dout("%s: img %p obj %p w=%u\n", __func__, img_request, obj_request,
+ obj_request->which);
list_del(&obj_request->links);
rbd_assert(img_request->obj_request_count > 0);
img_request->obj_request_count--;
@@ -1200,11 +1214,14 @@ static void rbd_osd_req_op_destroy(struct
ceph_osd_req_op *op)
static int rbd_obj_request_submit(struct ceph_osd_client *osdc,
struct rbd_obj_request *obj_request)
{
+ dout("%s: osdc %p obj %p\n", __func__, osdc, obj_request);
+
return ceph_osdc_start_request(osdc, obj_request->osd_req, false);
}
static void rbd_img_request_complete(struct rbd_img_request *img_request)
{
+ dout("%s: img %p\n", __func__, img_request);
if (img_request->callback)
img_request->callback(img_request);
else
@@ -1215,6 +1232,8 @@ static void rbd_img_request_complete(struct
rbd_img_request *img_request)
static int rbd_obj_request_wait(struct rbd_obj_request *obj_request)
{
+ dout("%s: obj %p\n", __func__, obj_request);
+
return wait_for_completion_interruptible(&obj_request->completion);
}
@@ -1248,11 +1267,14 @@ static bool obj_request_done_test(struct
rbd_obj_request *obj_request)
static void rbd_osd_trivial_callback(struct rbd_obj_request *obj_request,
struct ceph_osd_op *op)
{
+ dout("%s: obj %p\n", __func__, obj_request);
obj_request_done_set(obj_request);
}
static void rbd_obj_request_complete(struct rbd_obj_request *obj_request)
{
+ dout("%s: obj %p cb %p\n", __func__, obj_request,
+ obj_request->callback);
if (obj_request->callback)
obj_request->callback(obj_request);
else
@@ -1270,6 +1292,8 @@ static void rbd_osd_read_callback(struct
rbd_obj_request *obj_request,
*/
xferred = le64_to_cpu(op->extent.length);
rbd_assert(xferred < (u64) UINT_MAX);
+ dout("%s: obj %p result %d %llu/%llu\n", __func__, obj_request,
+ obj_request->result, xferred, obj_request->length);
if (obj_request->result == (s32) -ENOENT) {
zero_bio_chain(obj_request->bio_list, 0);
obj_request->result = 0;
@@ -1284,7 +1308,22 @@ static void rbd_osd_read_callback(struct
rbd_obj_request *obj_request,
static void rbd_osd_write_callback(struct rbd_obj_request *obj_request,
struct ceph_osd_op *op)
{
+
obj_request->xferred = le64_to_cpu(op->extent.length);
+ dout("%s: obj %p result %d %llu/%llu\n", __func__, obj_request,
+ obj_request->result, obj_request->xferred, obj_request->length);
+
+ /* A short write really shouldn't occur. Warn if we see one */
+
+ if (obj_request->xferred != obj_request->length) {
+ struct rbd_img_request *img_request = obj_request->img_request;
+ struct rbd_device *rbd_dev;
+
+ rbd_dev = img_request ? img_request->rbd_dev : NULL;
+ rbd_warn(rbd_dev, "wrote %llu want %llu\n",
+ obj_request->xferred, obj_request->length);
+ }
+
obj_request_done_set(obj_request);
}
@@ -1295,6 +1334,7 @@ static void rbd_osd_write_callback(struct
rbd_obj_request *obj_request,
static void rbd_osd_stat_callback(struct rbd_obj_request *obj_request,
struct ceph_osd_op *op)
{
+ dout("%s: obj %p\n", __func__, obj_request);
obj_request_done_set(obj_request);
}
@@ -1307,6 +1347,7 @@ static void rbd_osd_req_callback(struct
ceph_osd_request *osd_req,
u32 num_ops;
u16 opcode;
+ dout("%s: osd_req %p msg %p\n", __func__, osd_req, msg);
rbd_assert(osd_req == obj_request->osd_req);
rbd_assert(!!obj_request->img_request ^
(obj_request->which == BAD_WHICH));
@@ -1453,6 +1494,9 @@ static struct rbd_obj_request
*rbd_obj_request_create(const char *object_name,
init_completion(&obj_request->completion);
kref_init(&obj_request->kref);
+ dout("%s: \"%s\" %llu/%llu %d -> obj %p\n", __func__, object_name,
+ offset, length, (int)type, obj_request);
+
return obj_request;
}
@@ -1462,6 +1506,8 @@ static void rbd_obj_request_destroy(struct kref *kref)
obj_request = container_of(kref, struct rbd_obj_request, kref);
+ dout("%s: obj %p\n", __func__, obj_request);
+
rbd_assert(obj_request->img_request == NULL);
rbd_assert(obj_request->which == BAD_WHICH);
@@ -1531,6 +1577,10 @@ struct rbd_img_request
*rbd_img_request_create(struct rbd_device *rbd_dev,
rbd_img_request_get(img_request); /* Avoid a warning */
rbd_img_request_put(img_request); /* TEMPORARY */
+ dout("%s: rbd_dev %p %s %llu/%llu -> img %p\n", __func__, rbd_dev,
+ write_request ? "write" : "read", offset, length,
+ img_request);
+
return img_request;
}
@@ -1542,6 +1592,8 @@ static void rbd_img_request_destroy(struct kref *kref)
img_request = container_of(kref, struct rbd_img_request, kref);
+ dout("%s: img %p\n", __func__, img_request);
+
for_each_obj_request_safe(img_request, obj_request, next_obj_request)
rbd_img_obj_request_del(img_request, obj_request);
rbd_assert(img_request->obj_request_count == 0);
@@ -1563,6 +1615,8 @@ static int rbd_img_request_fill_bio(struct
rbd_img_request *img_request,
u64 resid;
u16 opcode;
+ dout("%s: img %p bio %p\n", __func__, img_request, bio_list);
+
opcode = img_request->write_request ? CEPH_OSD_OP_WRITE
: CEPH_OSD_OP_READ;
bio_offset = 0;
@@ -1638,6 +1692,7 @@ static void rbd_img_obj_callback(struct
rbd_obj_request *obj_request)
img_request = obj_request->img_request;
+ dout("%s: img %p obj %p\n", __func__, img_request, obj_request);
rbd_assert(img_request != NULL);
rbd_assert(img_request->rq != NULL);
rbd_assert(img_request->obj_request_count > 0);
@@ -1685,6 +1740,7 @@ static int rbd_img_request_submit(struct
rbd_img_request *img_request)
struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
struct rbd_obj_request *obj_request;
+ dout("%s: img %p\n", __func__, img_request);
for_each_obj_request(img_request, obj_request) {
int ret;
@@ -1745,7 +1801,7 @@ static void rbd_watch_cb(u64 ver, u64 notify_id,
u8 opcode, void *data)
if (!rbd_dev)
return;
- dout("rbd_watch_cb %s notify_id=%llu opcode=%u\n",
+ dout("%s: \"%s\" notify_id %llu opcode %u\n", __func__,
rbd_dev->header_name, (unsigned long long) notify_id,
(unsigned int) opcode);
rc = rbd_dev_refresh(rbd_dev, &hver);
@@ -3371,7 +3427,7 @@ static int rbd_dev_snaps_register(struct
rbd_device *rbd_dev)
struct rbd_snap *snap;
int ret = 0;
- dout("%s called\n", __func__);
+ dout("%s:\n", __func__);
if (WARN_ON(!device_is_registered(&rbd_dev->dev)))
return -EIO;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] libceph: define connection flag helpers
2013-02-22 17:15 Four miscellaneous patches Alex Elder
` (2 preceding siblings ...)
2013-02-22 17:17 ` [PATCH] rbd: normalize dout() calls Alex Elder
@ 2013-02-22 17:17 ` Alex Elder
2013-02-25 19:11 ` Four miscellaneous patches Josh Durgin
4 siblings, 0 replies; 6+ messages in thread
From: Alex Elder @ 2013-02-22 17:17 UTC (permalink / raw)
To: ceph-devel
Define and use functions that encapsulate operations performed on
a connection's flags.
This resolves:
http://tracker.ceph.com/issues/4234
Signed-off-by: Alex Elder <elder@inktank.com>
---
net/ceph/messenger.c | 107
++++++++++++++++++++++++++++++++++++--------------
1 file changed, 78 insertions(+), 29 deletions(-)
diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index 8a62a55..771d4c9 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -98,6 +98,57 @@
#define CON_FLAG_SOCK_CLOSED 3 /* socket state changed to closed */
#define CON_FLAG_BACKOFF 4 /* need to retry queuing delayed
work */
+static bool con_flag_valid(unsigned long con_flag)
+{
+ switch (con_flag) {
+ case CON_FLAG_LOSSYTX:
+ case CON_FLAG_KEEPALIVE_PENDING:
+ case CON_FLAG_WRITE_PENDING:
+ case CON_FLAG_SOCK_CLOSED:
+ case CON_FLAG_BACKOFF:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static void con_flag_clear(struct ceph_connection *con, unsigned long
con_flag)
+{
+ BUG_ON(!con_flag_valid(con_flag));
+
+ clear_bit(con_flag, &con->flags);
+}
+
+static void con_flag_set(struct ceph_connection *con, unsigned long
con_flag)
+{
+ BUG_ON(!con_flag_valid(con_flag));
+
+ set_bit(con_flag, &con->flags);
+}
+
+static bool con_flag_test(struct ceph_connection *con, unsigned long
con_flag)
+{
+ BUG_ON(!con_flag_valid(con_flag));
+
+ return test_bit(con_flag, &con->flags);
+}
+
+static bool con_flag_test_and_clear(struct ceph_connection *con,
+ unsigned long con_flag)
+{
+ BUG_ON(!con_flag_valid(con_flag));
+
+ return test_and_clear_bit(con_flag, &con->flags);
+}
+
+static bool con_flag_test_and_set(struct ceph_connection *con,
+ unsigned long con_flag)
+{
+ BUG_ON(!con_flag_valid(con_flag));
+
+ return test_and_set_bit(con_flag, &con->flags);
+}
+
/* static tag bytes (protocol control messages) */
static char tag_msg = CEPH_MSGR_TAG_MSG;
static char tag_ack = CEPH_MSGR_TAG_ACK;
@@ -309,7 +360,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(CON_FLAG_WRITE_PENDING, &con->flags)) {
+ if (con_flag_test(con, CON_FLAG_WRITE_PENDING)) {
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);
@@ -334,7 +385,7 @@ static void ceph_sock_state_change(struct sock *sk)
case TCP_CLOSE_WAIT:
dout("%s TCP_CLOSE_WAIT\n", __func__);
con_sock_state_closing(con);
- set_bit(CON_FLAG_SOCK_CLOSED, &con->flags);
+ con_flag_set(con, CON_FLAG_SOCK_CLOSED);
queue_con(con);
break;
case TCP_ESTABLISHED:
@@ -475,7 +526,7 @@ static int con_close_socket(struct ceph_connection *con)
* received a socket close event before we had the chance to
* shut the socket down.
*/
- clear_bit(CON_FLAG_SOCK_CLOSED, &con->flags);
+ con_flag_clear(con, CON_FLAG_SOCK_CLOSED);
con_sock_state_closed(con);
return rc;
@@ -539,11 +590,10 @@ void ceph_con_close(struct ceph_connection *con)
ceph_pr_addr(&con->peer_addr.in_addr));
con->state = CON_STATE_CLOSED;
- clear_bit(CON_FLAG_LOSSYTX, &con->flags); /* so we retry next connect */
- clear_bit(CON_FLAG_KEEPALIVE_PENDING, &con->flags);
- clear_bit(CON_FLAG_WRITE_PENDING, &con->flags);
- clear_bit(CON_FLAG_KEEPALIVE_PENDING, &con->flags);
- clear_bit(CON_FLAG_BACKOFF, &con->flags);
+ con_flag_clear(con, CON_FLAG_LOSSYTX); /* so we retry next connect */
+ con_flag_clear(con, CON_FLAG_KEEPALIVE_PENDING);
+ con_flag_clear(con, CON_FLAG_WRITE_PENDING);
+ con_flag_clear(con, CON_FLAG_BACKOFF);
reset_connection(con);
con->peer_global_seq = 0;
@@ -799,7 +849,7 @@ static void prepare_write_message(struct
ceph_connection *con)
/* no, queue up footer too and be done */
prepare_write_message_footer(con);
- set_bit(CON_FLAG_WRITE_PENDING, &con->flags);
+ con_flag_set(con, CON_FLAG_WRITE_PENDING);
}
/*
@@ -820,7 +870,7 @@ static void prepare_write_ack(struct ceph_connection
*con)
&con->out_temp_ack);
con->out_more = 1; /* more will follow.. eventually.. */
- set_bit(CON_FLAG_WRITE_PENDING, &con->flags);
+ con_flag_set(con, CON_FLAG_WRITE_PENDING);
}
/*
@@ -831,7 +881,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(CON_FLAG_WRITE_PENDING, &con->flags);
+ con_flag_set(con, CON_FLAG_WRITE_PENDING);
}
/*
@@ -874,7 +924,7 @@ static void prepare_write_banner(struct
ceph_connection *con)
&con->msgr->my_enc_addr);
con->out_more = 0;
- set_bit(CON_FLAG_WRITE_PENDING, &con->flags);
+ con_flag_set(con, CON_FLAG_WRITE_PENDING);
}
static int prepare_write_connect(struct ceph_connection *con)
@@ -924,7 +974,7 @@ static int prepare_write_connect(struct
ceph_connection *con)
auth->authorizer_buf);
con->out_more = 0;
- set_bit(CON_FLAG_WRITE_PENDING, &con->flags);
+ con_flag_set(con, CON_FLAG_WRITE_PENDING);
return 0;
}
@@ -1644,7 +1694,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(CON_FLAG_LOSSYTX, &con->flags);
+ con_flag_set(con, CON_FLAG_LOSSYTX);
con->delay = 0; /* reset backoff memory */
@@ -2081,15 +2131,14 @@ do_next:
prepare_write_ack(con);
goto more;
}
- if (test_and_clear_bit(CON_FLAG_KEEPALIVE_PENDING,
- &con->flags)) {
+ if (con_flag_test_and_clear(con, CON_FLAG_KEEPALIVE_PENDING)) {
prepare_write_keepalive(con);
goto more;
}
}
/* Nothing to do! */
- clear_bit(CON_FLAG_WRITE_PENDING, &con->flags);
+ con_flag_clear(con, CON_FLAG_WRITE_PENDING);
dout("try_write nothing else to write.\n");
ret = 0;
out:
@@ -2269,7 +2318,7 @@ static void queue_con(struct ceph_connection *con)
static bool con_sock_closed(struct ceph_connection *con)
{
- if (!test_and_clear_bit(CON_FLAG_SOCK_CLOSED, &con->flags))
+ if (!con_flag_test_and_clear(con, CON_FLAG_SOCK_CLOSED))
return false;
#define CASE(x) \
@@ -2310,14 +2359,14 @@ restart:
if (con_sock_closed(con))
goto fault;
- if (test_and_clear_bit(CON_FLAG_BACKOFF, &con->flags)) {
+ if (con_flag_test_and_clear(con, CON_FLAG_BACKOFF)) {
dout("con_work %p backing off\n", con);
ret = queue_con_delay(con, round_jiffies_relative(con->delay));
if (ret) {
dout("con_work %p FAILED to back off %lu\n", con,
con->delay);
BUG_ON(ret == -ENOENT);
- set_bit(CON_FLAG_BACKOFF, &con->flags);
+ con_flag_set(con, CON_FLAG_BACKOFF);
}
goto done;
}
@@ -2382,7 +2431,7 @@ static void ceph_fault(struct ceph_connection *con)
con_close_socket(con);
- if (test_bit(CON_FLAG_LOSSYTX, &con->flags)) {
+ if (con_flag_test(con, CON_FLAG_LOSSYTX)) {
dout("fault on LOSSYTX channel, marking CLOSED\n");
con->state = CON_STATE_CLOSED;
goto out_unlock;
@@ -2402,9 +2451,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(CON_FLAG_KEEPALIVE_PENDING, &con->flags)) {
+ !con_flag_test(con, CON_FLAG_KEEPALIVE_PENDING)) {
dout("fault %p setting STANDBY clearing WRITE_PENDING\n", con);
- clear_bit(CON_FLAG_WRITE_PENDING, &con->flags);
+ con_flag_clear(con, CON_FLAG_WRITE_PENDING);
con->state = CON_STATE_STANDBY;
} else {
/* retry after a delay. */
@@ -2413,7 +2462,7 @@ static void ceph_fault(struct ceph_connection *con)
con->delay = BASE_DELAY_INTERVAL;
else if (con->delay < MAX_DELAY_INTERVAL)
con->delay *= 2;
- set_bit(CON_FLAG_BACKOFF, &con->flags);
+ con_flag_set(con, CON_FLAG_BACKOFF);
queue_con(con);
}
@@ -2470,8 +2519,8 @@ static void clear_standby(struct ceph_connection *con)
dout("clear_standby %p and ++connect_seq\n", con);
con->state = CON_STATE_PREOPEN;
con->connect_seq++;
- WARN_ON(test_bit(CON_FLAG_WRITE_PENDING, &con->flags));
- WARN_ON(test_bit(CON_FLAG_KEEPALIVE_PENDING, &con->flags));
+ WARN_ON(con_flag_test(con, CON_FLAG_WRITE_PENDING));
+ WARN_ON(con_flag_test(con, CON_FLAG_KEEPALIVE_PENDING));
}
}
@@ -2512,7 +2561,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 */
- if (test_and_set_bit(CON_FLAG_WRITE_PENDING, &con->flags) == 0)
+ if (con_flag_test_and_set(con, CON_FLAG_WRITE_PENDING) == 0)
queue_con(con);
}
EXPORT_SYMBOL(ceph_con_send);
@@ -2601,8 +2650,8 @@ void ceph_con_keepalive(struct ceph_connection *con)
mutex_lock(&con->mutex);
clear_standby(con);
mutex_unlock(&con->mutex);
- if (test_and_set_bit(CON_FLAG_KEEPALIVE_PENDING, &con->flags) == 0 &&
- test_and_set_bit(CON_FLAG_WRITE_PENDING, &con->flags) == 0)
+ if (con_flag_test_and_set(con, CON_FLAG_KEEPALIVE_PENDING) == 0 &&
+ con_flag_test_and_set(con, CON_FLAG_WRITE_PENDING) == 0)
queue_con(con);
}
EXPORT_SYMBOL(ceph_con_keepalive);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Four miscellaneous patches
2013-02-22 17:15 Four miscellaneous patches Alex Elder
` (3 preceding siblings ...)
2013-02-22 17:17 ` [PATCH] libceph: define connection flag helpers Alex Elder
@ 2013-02-25 19:11 ` Josh Durgin
4 siblings, 0 replies; 6+ messages in thread
From: Josh Durgin @ 2013-02-25 19:11 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
On 02/22/2013 09:15 AM, Alex Elder wrote:
> The following patches address some issues that were found while
> investigating a kernel rbd client performance issue this week.
>
> These patches are available in the branch "test/wip-4234,5,7,8"
> in the ceph-client git repository.
>
> -Alex
>
> [PATCH] rbd: ignore zero-length requests
> [PATCH] rbd: barriers are hard
> [PATCH] rbd: normalize dout() calls
> [PATCH] libceph: define connection flag helpers
These all look good to me. The dout calls should help
debugging a lot.
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-02-25 19:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-22 17:15 Four miscellaneous patches Alex Elder
2013-02-22 17:17 ` [PATCH] rbd: ignore zero-length requests Alex Elder
2013-02-22 17:17 ` [PATCH] rbd: barriers are hard Alex Elder
2013-02-22 17:17 ` [PATCH] rbd: normalize dout() calls Alex Elder
2013-02-22 17:17 ` [PATCH] libceph: define connection flag helpers Alex Elder
2013-02-25 19:11 ` Four miscellaneous patches Josh Durgin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.