Commit 5185352c163a72cf969b2fbbfb89801b398896fd
1 parent
d79698da32
Exists in
master
and in
4 other branches
libceph: fix msgpool
There were several problems here:
1- we weren't tagging allocations with the pool, so they were never
returned to the pool.
2- msgpool_put didn't add back to the mempool, even it were called.
3- msgpool_release didn't clear the pool pointer, so it would have looped
had #1 not been broken.
These may or may not have been responsible for #1136 or #1381 (BUG due to
non-empty mempool on umount). I can't seem to trigger the crash now using
the method I was using before.
Signed-off-by: Sage Weil <sage@newdream.net>
Showing 1 changed file with 29 additions and 11 deletions Side-by-side Diff
net/ceph/msgpool.c
| ... | ... | @@ -7,27 +7,37 @@ |
| 7 | 7 | |
| 8 | 8 | #include <linux/ceph/msgpool.h> |
| 9 | 9 | |
| 10 | -static void *alloc_fn(gfp_t gfp_mask, void *arg) | |
| 10 | +static void *msgpool_alloc(gfp_t gfp_mask, void *arg) | |
| 11 | 11 | { |
| 12 | 12 | struct ceph_msgpool *pool = arg; |
| 13 | - void *p; | |
| 13 | + struct ceph_msg *msg; | |
| 14 | 14 | |
| 15 | - p = ceph_msg_new(0, pool->front_len, gfp_mask); | |
| 16 | - if (!p) | |
| 17 | - pr_err("msgpool %s alloc failed\n", pool->name); | |
| 18 | - return p; | |
| 15 | + msg = ceph_msg_new(0, pool->front_len, gfp_mask); | |
| 16 | + if (!msg) { | |
| 17 | + dout("msgpool_alloc %s failed\n", pool->name); | |
| 18 | + } else { | |
| 19 | + dout("msgpool_alloc %s %p\n", pool->name, msg); | |
| 20 | + msg->pool = pool; | |
| 21 | + } | |
| 22 | + return msg; | |
| 19 | 23 | } |
| 20 | 24 | |
| 21 | -static void free_fn(void *element, void *arg) | |
| 25 | +static void msgpool_free(void *element, void *arg) | |
| 22 | 26 | { |
| 23 | - ceph_msg_put(element); | |
| 27 | + struct ceph_msgpool *pool = arg; | |
| 28 | + struct ceph_msg *msg = element; | |
| 29 | + | |
| 30 | + dout("msgpool_release %s %p\n", pool->name, msg); | |
| 31 | + msg->pool = NULL; | |
| 32 | + ceph_msg_put(msg); | |
| 24 | 33 | } |
| 25 | 34 | |
| 26 | 35 | int ceph_msgpool_init(struct ceph_msgpool *pool, |
| 27 | 36 | int front_len, int size, bool blocking, const char *name) |
| 28 | 37 | { |
| 38 | + dout("msgpool %s init\n", name); | |
| 29 | 39 | pool->front_len = front_len; |
| 30 | - pool->pool = mempool_create(size, alloc_fn, free_fn, pool); | |
| 40 | + pool->pool = mempool_create(size, msgpool_alloc, msgpool_free, pool); | |
| 31 | 41 | if (!pool->pool) |
| 32 | 42 | return -ENOMEM; |
| 33 | 43 | pool->name = name; |
| 34 | 44 | |
| 35 | 45 | |
| ... | ... | @@ -36,14 +46,17 @@ |
| 36 | 46 | |
| 37 | 47 | void ceph_msgpool_destroy(struct ceph_msgpool *pool) |
| 38 | 48 | { |
| 49 | + dout("msgpool %s destroy\n", pool->name); | |
| 39 | 50 | mempool_destroy(pool->pool); |
| 40 | 51 | } |
| 41 | 52 | |
| 42 | 53 | struct ceph_msg *ceph_msgpool_get(struct ceph_msgpool *pool, |
| 43 | 54 | int front_len) |
| 44 | 55 | { |
| 56 | + struct ceph_msg *msg; | |
| 57 | + | |
| 45 | 58 | if (front_len > pool->front_len) { |
| 46 | - pr_err("msgpool_get pool %s need front %d, pool size is %d\n", | |
| 59 | + dout("msgpool_get %s need front %d, pool size is %d\n", | |
| 47 | 60 | pool->name, front_len, pool->front_len); |
| 48 | 61 | WARN_ON(1); |
| 49 | 62 | |
| 50 | 63 | |
| 51 | 64 | |
| ... | ... | @@ -51,15 +64,20 @@ |
| 51 | 64 | return ceph_msg_new(0, front_len, GFP_NOFS); |
| 52 | 65 | } |
| 53 | 66 | |
| 54 | - return mempool_alloc(pool->pool, GFP_NOFS); | |
| 67 | + msg = mempool_alloc(pool->pool, GFP_NOFS); | |
| 68 | + dout("msgpool_get %s %p\n", pool->name, msg); | |
| 69 | + return msg; | |
| 55 | 70 | } |
| 56 | 71 | |
| 57 | 72 | void ceph_msgpool_put(struct ceph_msgpool *pool, struct ceph_msg *msg) |
| 58 | 73 | { |
| 74 | + dout("msgpool_put %s %p\n", pool->name, msg); | |
| 75 | + | |
| 59 | 76 | /* reset msg front_len; user may have changed it */ |
| 60 | 77 | msg->front.iov_len = pool->front_len; |
| 61 | 78 | msg->hdr.front_len = cpu_to_le32(pool->front_len); |
| 62 | 79 | |
| 63 | 80 | kref_init(&msg->kref); /* retake single ref */ |
| 81 | + mempool_free(msg, pool->pool); | |
| 64 | 82 | } |