On Wed, Jul 1, 2020 at 5:54 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> ceph_osdc_new_request is cephfs specific. Move it and calc_layout into
> ceph.ko. Also, calc_layout can be void return.
>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> fs/ceph/Makefile | 2 +-
> fs/ceph/addr.c | 1 +
> fs/ceph/file.c | 1 +
> fs/ceph/osdc.c | 113 +++++++++++++++++++++++++++++++
> fs/ceph/osdc.h | 16 +++++
> include/linux/ceph/osd_client.h | 10 ---
> net/ceph/osd_client.c | 115 --------------------------------
> 7 files changed, 132 insertions(+), 126 deletions(-)
> create mode 100644 fs/ceph/osdc.c
> create mode 100644 fs/ceph/osdc.h
>
> diff --git a/fs/ceph/Makefile b/fs/ceph/Makefile
> index 50c635dc7f71..f2ec52fa8d37 100644
> --- a/fs/ceph/Makefile
> +++ b/fs/ceph/Makefile
> @@ -8,7 +8,7 @@ obj-$(CONFIG_CEPH_FS) += ceph.o
> ceph-y := super.o inode.o dir.o file.o locks.o addr.o ioctl.o \
> export.o caps.o snap.o xattr.o quota.o io.o \
> mds_client.o mdsmap.o strings.o ceph_frag.o \
> - debugfs.o util.o metric.o
> + debugfs.o util.o metric.o osdc.o
>
> ceph-$(CONFIG_CEPH_FSCACHE) += cache.o
> ceph-$(CONFIG_CEPH_FS_POSIX_ACL) += acl.o
> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
> index 01ad09733ac7..1a3cc1875a31 100644
> --- a/fs/ceph/addr.c
> +++ b/fs/ceph/addr.c
> @@ -19,6 +19,7 @@
> #include "metric.h"
> #include <linux/ceph/osd_client.h>
> #include <linux/ceph/striper.h>
> +#include "osdc.h"
>
> /*
> * Ceph address space ops.
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index 160644ddaeed..b697a1f3c56e 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -18,6 +18,7 @@
> #include "cache.h"
> #include "io.h"
> #include "metric.h"
> +#include "osdc.h"
>
> static __le32 ceph_flags_sys2wire(u32 flags)
> {
> diff --git a/fs/ceph/osdc.c b/fs/ceph/osdc.c
> new file mode 100644
> index 000000000000..303e39fce3d4
> --- /dev/null
> +++ b/fs/ceph/osdc.c
> @@ -0,0 +1,113 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/ceph/ceph_debug.h>
> +#include <linux/ceph/libceph.h>
> +#include <linux/ceph/osd_client.h>
> +#include <linux/ceph/striper.h>
> +#include "osdc.h"
> +
> +/*
> + * calculate the mapping of a file extent onto an object, and fill out the
> + * request accordingly. shorten extent as necessary if it crosses an
> + * object boundary.
> + *
> + * fill osd op in request message.
> + */
> +static void calc_layout(struct ceph_file_layout *layout, u64 off, u64 *plen,
> + u64 *objnum, u64 *objoff, u64 *objlen)
> +{
> + u64 orig_len = *plen;
> + u32 xlen;
> +
> + /* object extent? */
> + ceph_calc_file_object_mapping(layout, off, orig_len, objnum,
> + objoff, &xlen);
> + *objlen = xlen;
> + if (*objlen < orig_len) {
> + *plen = *objlen;
> + dout(" skipping last %llu, final file extent %llu~%llu\n",
> + orig_len - *plen, off, *plen);
> + }
> +
> + dout("calc_layout objnum=%llx %llu~%llu\n", *objnum, *objoff, *objlen);
> +}
> +
> +/*
> + * build new request AND message, calculate layout, and adjust file
> + * extent as needed.
> + *
> + * if the file was recently truncated, we include information about its
> + * old and new size so that the object can be updated appropriately. (we
> + * avoid synchronously deleting truncated objects because it's slow.)
> + */
> +struct ceph_osd_request *ceph_osdc_new_request(struct ceph_osd_client *osdc,
> + struct ceph_file_layout *layout,
> + struct ceph_vino vino,
> + u64 off, u64 *plen,
> + unsigned int which, int num_ops,
> + int opcode, int flags,
> + struct ceph_snap_context *snapc,
> + u32 truncate_seq,
> + u64 truncate_size,
> + bool use_mempool)
> +{
> + struct ceph_osd_request *req;
> + u64 objnum = 0;
> + u64 objoff = 0;
> + u64 objlen = 0;
> + int r;
> +
> + BUG_ON(opcode != CEPH_OSD_OP_READ && opcode != CEPH_OSD_OP_WRITE &&
> + opcode != CEPH_OSD_OP_ZERO && opcode != CEPH_OSD_OP_TRUNCATE &&
> + opcode != CEPH_OSD_OP_CREATE && opcode != CEPH_OSD_OP_DELETE);
> +
> + req = ceph_osdc_alloc_request(osdc, snapc, num_ops, use_mempool,
> + GFP_NOFS);
> + if (!req)
> + return ERR_PTR(-ENOMEM);
> +
> + /* calculate max write size */
> + calc_layout(layout, off, plen, &objnum, &objoff, &objlen);
> +
> + if (opcode == CEPH_OSD_OP_CREATE || opcode == CEPH_OSD_OP_DELETE) {
> + osd_req_op_init(req, which, opcode, 0);
> + } else {
> + u32 object_size = layout->object_size;
> + u32 object_base = off - objoff;
> + if (!(truncate_seq == 1 && truncate_size == -1ULL)) {
> + if (truncate_size <= object_base) {
> + truncate_size = 0;
> + } else {
> + truncate_size -= object_base;
> + if (truncate_size > object_size)
> + truncate_size = object_size;
> + }
> + }
> + osd_req_op_extent_init(req, which, opcode, objoff, objlen,
> + truncate_size, truncate_seq);
> + }
> +
> + req->r_base_oloc.pool = layout->pool_id;
> + req->r_base_oloc.pool_ns = ceph_try_get_string(layout->pool_ns);
> + req->r_flags = flags | osdc->client->options->read_from_replica;
> + ceph_oid_printf(&req->r_base_oid, "%llx.%08llx", vino.ino, objnum);
> +
> + req->r_snapid = vino.snap;
> + if (flags & CEPH_OSD_FLAG_WRITE)
> + req->r_data_offset = off;
> +
> + if (num_ops > 1)
> + /*
> + * This is a special case for ceph_writepages_start(), but it
> + * also covers ceph_uninline_data(). If more multi-op request
> + * use cases emerge, we will need a separate helper.
> + */
> + r = ceph_osdc_alloc_num_messages(req, GFP_NOFS, num_ops, 0);
> + else
> + r = ceph_osdc_alloc_messages(req, GFP_NOFS);
> + if (r) {
> + ceph_osdc_put_request(req);
> + return ERR_PTR(r);
> + }
> +
> + return req;
> +}
> diff --git a/fs/ceph/osdc.h b/fs/ceph/osdc.h
> new file mode 100644
> index 000000000000..b03e5f9ef733
> --- /dev/null
> +++ b/fs/ceph/osdc.h
> @@ -0,0 +1,16 @@
> +#ifndef _FS_CEPH_OSDC_H
> +#define _FS_CEPH_OSDC_H
> +
> +#include <linux/ceph/osd_client.h>
> +
> +struct ceph_osd_request *ceph_osdc_new_request(struct ceph_osd_client *osdc,
> + struct ceph_file_layout *layout,
> + struct ceph_vino vino,
> + u64 off, u64 *plen,
> + unsigned int which, int num_ops,
> + int opcode, int flags,
> + struct ceph_snap_context *snapc,
> + u32 truncate_seq,
> + u64 truncate_size,
> + bool use_mempool);
> +#endif /* _FS_CEPH_OSDC_H */
> diff --git a/include/linux/ceph/osd_client.h b/include/linux/ceph/osd_client.h
> index 71b7610c3a3c..f59eb192c472 100644
> --- a/include/linux/ceph/osd_client.h
> +++ b/include/linux/ceph/osd_client.h
> @@ -486,16 +486,6 @@ int ceph_osdc_alloc_num_messages(struct ceph_osd_request *req, gfp_t gfp,
> int num_reply_data_items);
> int ceph_osdc_alloc_messages(struct ceph_osd_request *req, gfp_t gfp);
>
> -extern struct ceph_osd_request *ceph_osdc_new_request(struct ceph_osd_client *,
> - struct ceph_file_layout *layout,
> - struct ceph_vino vino,
> - u64 offset, u64 *len,
> - unsigned int which, int num_ops,
> - int opcode, int flags,
> - struct ceph_snap_context *snapc,
> - u32 truncate_seq, u64 truncate_size,
> - bool use_mempool);
> -
> extern void ceph_osdc_get_request(struct ceph_osd_request *req);
> extern void ceph_osdc_put_request(struct ceph_osd_request *req);
> void ceph_osdc_init_request(struct ceph_osd_request *req,
> diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
> index 7be78fa6e2c3..5e54971bb7b2 100644
> --- a/net/ceph/osd_client.c
> +++ b/net/ceph/osd_client.c
> @@ -93,33 +93,6 @@ static inline void verify_osd_locked(struct ceph_osd *osd) { }
> static inline void verify_lreq_locked(struct ceph_osd_linger_request *lreq) { }
> #endif
>
> -/*
> - * calculate the mapping of a file extent onto an object, and fill out the
> - * request accordingly. shorten extent as necessary if it crosses an
> - * object boundary.
> - *
> - * fill osd op in request message.
> - */
> -static int calc_layout(struct ceph_file_layout *layout, u64 off, u64 *plen,
> - u64 *objnum, u64 *objoff, u64 *objlen)
> -{
> - u64 orig_len = *plen;
> - u32 xlen;
> -
> - /* object extent? */
> - ceph_calc_file_object_mapping(layout, off, orig_len, objnum,
> - objoff, &xlen);
> - *objlen = xlen;
> - if (*objlen < orig_len) {
> - *plen = *objlen;
> - dout(" skipping last %llu, final file extent %llu~%llu\n",
> - orig_len - *plen, off, *plen);
> - }
> -
> - dout("calc_layout objnum=%llx %llu~%llu\n", *objnum, *objoff, *objlen);
> - return 0;
> -}
> -
> static void ceph_osd_data_init(struct ceph_osd_data *osd_data)
> {
> memset(osd_data, 0, sizeof (*osd_data));
> @@ -1056,94 +1029,6 @@ static u32 osd_req_encode_op(struct ceph_osd_op *dst,
> return src->indata_len;
> }
>
> -/*
> - * build new request AND message, calculate layout, and adjust file
> - * extent as needed.
> - *
> - * if the file was recently truncated, we include information about its
> - * old and new size so that the object can be updated appropriately. (we
> - * avoid synchronously deleting truncated objects because it's slow.)
> - */
> -struct ceph_osd_request *ceph_osdc_new_request(struct ceph_osd_client *osdc,
> - struct ceph_file_layout *layout,
> - struct ceph_vino vino,
> - u64 off, u64 *plen,
> - unsigned int which, int num_ops,
> - int opcode, int flags,
> - struct ceph_snap_context *snapc,
> - u32 truncate_seq,
> - u64 truncate_size,
> - bool use_mempool)
> -{
> - struct ceph_osd_request *req;
> - u64 objnum = 0;
> - u64 objoff = 0;
> - u64 objlen = 0;
> - int r;
> -
> - BUG_ON(opcode != CEPH_OSD_OP_READ && opcode != CEPH_OSD_OP_WRITE &&
> - opcode != CEPH_OSD_OP_ZERO && opcode != CEPH_OSD_OP_TRUNCATE &&
> - opcode != CEPH_OSD_OP_CREATE && opcode != CEPH_OSD_OP_DELETE);
> -
> - req = ceph_osdc_alloc_request(osdc, snapc, num_ops, use_mempool,
> - GFP_NOFS);
> - if (!req) {
> - r = -ENOMEM;
> - goto fail;
> - }
> -
> - /* calculate max write size */
> - r = calc_layout(layout, off, plen, &objnum, &objoff, &objlen);
> - if (r)
> - goto fail;
> -
> - if (opcode == CEPH_OSD_OP_CREATE || opcode == CEPH_OSD_OP_DELETE) {
> - osd_req_op_init(req, which, opcode, 0);
> - } else {
> - u32 object_size = layout->object_size;
> - u32 object_base = off - objoff;
> - if (!(truncate_seq == 1 && truncate_size == -1ULL)) {
> - if (truncate_size <= object_base) {
> - truncate_size = 0;
> - } else {
> - truncate_size -= object_base;
> - if (truncate_size > object_size)
> - truncate_size = object_size;
> - }
> - }
> - osd_req_op_extent_init(req, which, opcode, objoff, objlen,
> - truncate_size, truncate_seq);
> - }
> -
> - req->r_base_oloc.pool = layout->pool_id;
> - req->r_base_oloc.pool_ns = ceph_try_get_string(layout->pool_ns);
> - ceph_oid_printf(&req->r_base_oid, "%llx.%08llx", vino.ino, objnum);
> - req->r_flags = flags | osdc->client->options->read_from_replica;
> -
> - req->r_snapid = vino.snap;
> - if (flags & CEPH_OSD_FLAG_WRITE)
> - req->r_data_offset = off;
> -
> - if (num_ops > 1)
> - /*
> - * This is a special case for ceph_writepages_start(), but it
> - * also covers ceph_uninline_data(). If more multi-op request
> - * use cases emerge, we will need a separate helper.
> - */
> - r = ceph_osdc_alloc_num_messages(req, GFP_NOFS, num_ops, 0);
> - else
> - r = ceph_osdc_alloc_messages(req, GFP_NOFS);
> - if (r)
> - goto fail;
> -
> - return req;
> -
> -fail:
> - ceph_osdc_put_request(req);
> - return ERR_PTR(r);
> -}
> -EXPORT_SYMBOL(ceph_osdc_new_request);
> -
> /*
> * We keep osd requests in an rbtree, sorted by ->r_tid.
> */
Do you have plans to refactor ceph_osdc_new_request() or change
it significantly?
I don't think the fact that it is used only by fs/ceph justifies
moving it to fs/ceph, particularly given that you had to export a
rather sensitive OSD client helper in the process. Also, you are
creating a new file for it, which indicates that it doesn't fit
into any of the existing files and reinforces my feeling that it
probably doesn't belong in fs/ceph.
Thanks,
Ilya
