On Tue, Apr 17, 2018 at 12:15 PM, Chengguang Xu <cgxu519@xxxxxxx> wrote:
> Do memory allocation first, so that avoid unnecessary
> initialization of newly allocated message in error case.
>
> Signed-off-by: Chengguang Xu <cgxu519@xxxxxxx>
> ---
> net/ceph/messenger.c | 42 ++++++++++++++++++++----------------------
> 1 file changed, 20 insertions(+), 22 deletions(-)
>
> diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
> index fcb40c1..5a8c5cd 100644
> --- a/net/ceph/messenger.c
> +++ b/net/ceph/messenger.c
> @@ -3331,6 +3331,16 @@ void ceph_msg_data_add_bvecs(struct ceph_msg *msg,
> EXPORT_SYMBOL(ceph_msg_data_add_bvecs);
>
> /*
> + * Free a generically kmalloc'd message.
> + */
> +static void ceph_msg_free(struct ceph_msg *m)
> +{
> + dout("%s %p\n", __func__, m);
> + kvfree(m->front.iov_base);
> + kmem_cache_free(ceph_msg_cache, m);
> +}
> +
> +/*
> * construct a new message with given type, size
> * the new msg has a ref count of 1.
> */
> @@ -3343,14 +3353,6 @@ struct ceph_msg *ceph_msg_new(int type, int front_len, gfp_t flags,
> if (m == NULL)
> goto out;
>
> - m->hdr.type = cpu_to_le16(type);
> - m->hdr.priority = cpu_to_le16(CEPH_MSG_PRIO_DEFAULT);
> - m->hdr.front_len = cpu_to_le32(front_len);
> -
> - INIT_LIST_HEAD(&m->list_head);
> - kref_init(&m->kref);
> - INIT_LIST_HEAD(&m->data);
> -
> /* front */
> if (front_len) {
> m->front.iov_base = ceph_kvmalloc(front_len, flags);
> @@ -3359,16 +3361,23 @@ struct ceph_msg *ceph_msg_new(int type, int front_len, gfp_t flags,
> front_len);
> goto out2;
> }
> - } else {
> - m->front.iov_base = NULL;
> }
> +
> + m->hdr.type = cpu_to_le16(type);
> + m->hdr.priority = cpu_to_le16(CEPH_MSG_PRIO_DEFAULT);
> + m->hdr.front_len = cpu_to_le32(front_len);
> +
> + INIT_LIST_HEAD(&m->list_head);
> + kref_init(&m->kref);
> + INIT_LIST_HEAD(&m->data);
> +
> m->front_alloc_len = m->front.iov_len = front_len;
>
> dout("ceph_msg_new %p front %d\n", m, front_len);
> return m;
>
> out2:
> - ceph_msg_put(m);
> + ceph_msg_free(m);
> out:
> if (!can_fail) {
> pr_err("msg_new can't create type %d front %d\n", type,
> @@ -3467,17 +3476,6 @@ static int ceph_con_in_msg_alloc(struct ceph_connection *con, int *skip)
> return ret;
> }
>
> -
> -/*
> - * Free a generically kmalloc'd message.
> - */
> -static void ceph_msg_free(struct ceph_msg *m)
> -{
> - dout("%s %p\n", __func__, m);
> - kvfree(m->front.iov_base);
> - kmem_cache_free(ceph_msg_cache, m);
> -}
> -
> static void ceph_msg_release(struct kref *kref)
> {
> struct ceph_msg *m = container_of(kref, struct ceph_msg, kref);
Hi Chengguang,
I don't think this buys anything. The difference is literally a few
integer/pointer assignments in the error case, which means a failed OSD
or MDS request.
Thanks,
Ilya
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
