From: Ilya Dryomov <idryomov@gmail.com>
To: Jeff Layton <jlayton@kernel.org>
Cc: Ceph Development <ceph-devel@vger.kernel.org>,
"Yan, Zheng" <ukernel@gmail.com>
Subject: Re: [PATCH] ceph: break up send_cap_msg
Date: Wed, 7 Oct 2020 19:33:08 +0200 [thread overview]
Message-ID: <CAOi1vP-uJnznmKLSSRymj1FDXWKf4uD1NLuGVse5Nokr=4JQ=w@mail.gmail.com> (raw)
In-Reply-To: <20201007122536.13354-1-jlayton@kernel.org>
On Wed, Oct 7, 2020 at 2:25 PM Jeff Layton <jlayton@kernel.org> wrote:
>
> Push the allocation of the msg and the send into the caller. Rename
> the function to marshal_cap_msg and make it void return.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> fs/ceph/caps.c | 61 +++++++++++++++++++++++++-------------------------
> 1 file changed, 30 insertions(+), 31 deletions(-)
>
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index 4e84b39a6ebd..6f4adfaf761f 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -1222,36 +1222,29 @@ struct cap_msg_args {
> };
>
> /*
> - * Build and send a cap message to the given MDS.
> - *
> - * Caller should be holding s_mutex.
> + * cap struct size + flock buffer size + inline version + inline data size +
> + * osd_epoch_barrier + oldest_flush_tid
> */
> -static int send_cap_msg(struct cap_msg_args *arg)
> +#define CAP_MSG_SIZE (sizeof(struct ceph_mds_caps) + \
> + 4 + 8 + 4 + 4 + 8 + 4 + 4 + 4 + 8 + 8 + 4)
> +
> +/* Marshal up the cap msg to the MDS */
> +static void marshal_cap_msg(struct ceph_msg *msg, struct cap_msg_args *arg)
Nit: functions like this usually have "encode" or "build" in their
names across the codebase, so I'd go with "encode_cap_msg".
> {
> struct ceph_mds_caps *fc;
> - struct ceph_msg *msg;
> void *p;
> - size_t extra_len;
> struct ceph_osd_client *osdc = &arg->session->s_mdsc->fsc->client->osdc;
>
> - dout("send_cap_msg %s %llx %llx caps %s wanted %s dirty %s"
> + dout("%s %s %llx %llx caps %s wanted %s dirty %s"
> " seq %u/%u tid %llu/%llu mseq %u follows %lld size %llu/%llu"
> - " xattr_ver %llu xattr_len %d\n", ceph_cap_op_name(arg->op),
> - arg->cid, arg->ino, ceph_cap_string(arg->caps),
> - ceph_cap_string(arg->wanted), ceph_cap_string(arg->dirty),
> - arg->seq, arg->issue_seq, arg->flush_tid, arg->oldest_flush_tid,
> - arg->mseq, arg->follows, arg->size, arg->max_size,
> - arg->xattr_version,
> + " xattr_ver %llu xattr_len %d\n", __func__,
> + ceph_cap_op_name(arg->op), arg->cid, arg->ino,
> + ceph_cap_string(arg->caps), ceph_cap_string(arg->wanted),
> + ceph_cap_string(arg->dirty), arg->seq, arg->issue_seq,
> + arg->flush_tid, arg->oldest_flush_tid, arg->mseq, arg->follows,
> + arg->size, arg->max_size, arg->xattr_version,
> arg->xattr_buf ? (int)arg->xattr_buf->vec.iov_len : 0);
>
> - /* flock buffer size + inline version + inline data size +
> - * osd_epoch_barrier + oldest_flush_tid */
> - extra_len = 4 + 8 + 4 + 4 + 8 + 4 + 4 + 4 + 8 + 8 + 4;
> - msg = ceph_msg_new(CEPH_MSG_CLIENT_CAPS, sizeof(*fc) + extra_len,
> - GFP_NOFS, false);
> - if (!msg)
> - return -ENOMEM;
> -
> msg->hdr.version = cpu_to_le16(10);
> msg->hdr.tid = cpu_to_le64(arg->flush_tid);
>
> @@ -1323,9 +1316,6 @@ static int send_cap_msg(struct cap_msg_args *arg)
>
> /* Advisory flags (version 10) */
> ceph_encode_32(&p, arg->flags);
> -
> - ceph_con_send(&arg->session->s_con, msg);
> - return 0;
> }
>
> /*
> @@ -1456,22 +1446,24 @@ static void __prep_cap(struct cap_msg_args *arg, struct ceph_cap *cap,
> */
> static void __send_cap(struct cap_msg_args *arg, struct ceph_inode_info *ci)
> {
> + struct ceph_msg *msg;
> struct inode *inode = &ci->vfs_inode;
> - int ret;
>
> - ret = send_cap_msg(arg);
> - if (ret < 0) {
> - pr_err("error sending cap msg, ino (%llx.%llx) "
> - "flushing %s tid %llu, requeue\n",
> + msg = ceph_msg_new(CEPH_MSG_CLIENT_CAPS, CAP_MSG_SIZE, GFP_NOFS, false);
> + if (!msg) {
> + pr_err("error allocating cap msg: ino (%llx.%llx) "
> + "flushing %s tid %llu, requeuing cap.\n",
Don't break new user-visible strings. This makes grepping harder than
it should be.
Thanks,
Ilya
next prev parent reply other threads:[~2020-10-07 17:33 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-07 12:25 [PATCH] ceph: break up send_cap_msg Jeff Layton
2020-10-07 17:33 ` Ilya Dryomov [this message]
2020-10-07 17:55 ` Jeff Layton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAOi1vP-uJnznmKLSSRymj1FDXWKf4uD1NLuGVse5Nokr=4JQ=w@mail.gmail.com' \
--to=idryomov@gmail.com \
--cc=ceph-devel@vger.kernel.org \
--cc=jlayton@kernel.org \
--cc=ukernel@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).