On Mon, 12 Mar 2012, Alex Elder wrote:
> There are helpers defined in "include/linux/ceph/osdmap.h" for
> accessing the fields of an on-disk ceph file layout structure.
> Use them--consistently--throughout the code.
> 
> Also define CEPH_FILE_LAYOUT_PG_PREFERRED_NONE, to represent
> symbolically the explicit "no preferred PG" value.
> 
> Make a few casts explicit too (to make it more obvious it's
> occuring).  This produces some long lines, but they go away in an
> upcoming patch.
> 
> Signed-off-by: Alex Elder <[email protected]>
> ---
>  drivers/block/rbd.c          |    2 +-
>  fs/ceph/ioctl.c              |   24 ++++++++++++------------
>  fs/ceph/xattr.c              |    5 +++--
>  include/linux/ceph/ceph_fs.h |    3 ++-
>  net/ceph/ceph_fs.c           |    6 +++---
>  net/ceph/osdmap.c            |   25 +++++++++++++------------
>  6 files changed, 34 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 6bbd5af..e9521ed 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -936,7 +936,7 @@ static int rbd_do_request(struct request *rq,
>       layout->fl_stripe_unit = cpu_to_le32(1 << RBD_MAX_OBJ_ORDER);
>       layout->fl_stripe_count = cpu_to_le32(1);
>       layout->fl_object_size = cpu_to_le32(1 << RBD_MAX_OBJ_ORDER);
> -     layout->fl_pg_preferred = cpu_to_le32(-1);
> +     layout->fl_pg_preferred = CEPH_FILE_LAYOUT_PG_PREFERRED_NONE;
>       layout->fl_pg_pool = cpu_to_le32(dev->poolid);
>       ceph_calc_raw_layout(osdc, layout, snapid, ofs, &len, &bno,
>                               req, ops);
> diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c
> index 23cb78b..75cff03 100644
> --- a/fs/ceph/ioctl.c
> +++ b/fs/ceph/ioctl.c
> @@ -22,12 +22,12 @@ static long ceph_ioctl_get_layout(struct file *file, void
> __user *arg)
> 
>       err = ceph_do_getattr(file->f_dentry->d_inode, CEPH_STAT_CAP_LAYOUT);
>       if (!err) {
> -             l.stripe_unit = ceph_file_layout_stripe_unit(&ci->i_layout);
> -             l.stripe_count = ceph_file_layout_stripe_count(&ci->i_layout);
> -             l.object_size = ceph_file_layout_object_size(&ci->i_layout);
> -             l.data_pool = le32_to_cpu(ci->i_layout.fl_pg_pool);
> +             l.stripe_unit = (__u64)
> ceph_file_layout_stripe_unit(&ci->i_layout);
> +             l.stripe_count = (__u64)
> ceph_file_layout_stripe_count(&ci->i_layout);
> +             l.object_size = (__u64)
> ceph_file_layout_object_size(&ci->i_layout);
> +             l.data_pool = (__u64) ceph_file_layout_pg_pool(&ci->i_layout);
>               l.preferred_osd =
> -                     (s32)le32_to_cpu(ci->i_layout.fl_pg_preferred);
> +                     (__s64) ceph_file_layout_pg_preferred(&ci->i_layout);
>               if (copy_to_user(arg, &l, sizeof(l)))
>                       return -EFAULT;
>       }
> @@ -52,12 +52,12 @@ static long ceph_ioctl_set_layout(struct file *file, void
> __user *arg)
>       /* validate changed params against current layout */
>       err = ceph_do_getattr(file->f_dentry->d_inode, CEPH_STAT_CAP_LAYOUT);
>       if (!err) {
> -             nl.stripe_unit = ceph_file_layout_stripe_unit(&ci->i_layout);
> -             nl.stripe_count =
> ceph_file_layout_stripe_count(&ci->i_layout);
> -             nl.object_size = ceph_file_layout_object_size(&ci->i_layout);
> -             nl.data_pool = le32_to_cpu(ci->i_layout.fl_pg_pool);
> +             nl.stripe_unit = (__u64)
> ceph_file_layout_stripe_unit(&ci->i_layout);
> +             nl.stripe_count = (__u64)
> ceph_file_layout_stripe_count(&ci->i_layout);
> +             nl.object_size = (__u64)
> ceph_file_layout_object_size(&ci->i_layout);
> +             nl.data_pool = (__u64)
> ceph_file_layout_pg_pool(&ci->i_layout);
>               nl.preferred_osd =
> -
> (s32)le32_to_cpu(ci->i_layout.fl_pg_preferred);
> +                             (__s64)
> ceph_file_layout_pg_preferred(&ci->i_layout);
>       } else
>               return err;
> 
> @@ -203,8 +203,8 @@ static long ceph_ioctl_get_dataloc(struct file *file, void
> __user *arg)
>       ceph_calc_file_object_mapping(&ci->i_layout, dl.file_offset, &len,
>                                     &dl.object_no, &dl.object_offset,
> &olen);
>       dl.file_offset -= dl.object_offset;
> -     dl.object_size = ceph_file_layout_object_size(&ci->i_layout);
> -     dl.block_size = ceph_file_layout_stripe_unit(&ci->i_layout);
> +     dl.object_size = (__u64) ceph_file_layout_object_size(&ci->i_layout);
> +     dl.block_size = (__u64) ceph_file_layout_stripe_unit(&ci->i_layout);
> 
>       /* block_offset = object_offset % block_size */
>       tmp = dl.object_offset;
> diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
> index 75960b1..bfd5b93 100644
> --- a/fs/ceph/xattr.c
> +++ b/fs/ceph/xattr.c
> @@ -115,11 +115,12 @@ static size_t ceph_vxattrcb_file_layout(struct
> ceph_inode_info *ci, char *val,
> 
>       ret = snprintf(val, size,
>               "chunk_bytes=%lld\nstripe_count=%lld\nobject_size=%lld\n",
> -             (unsigned long long)ceph_file_layout_su(&ci->i_layout),
> +             (unsigned long
> long)ceph_file_layout_stripe_unit(&ci->i_layout),
>               (unsigned long
> long)ceph_file_layout_stripe_count(&ci->i_layout),
>               (unsigned long
> long)ceph_file_layout_object_size(&ci->i_layout));
> 
> -     if (ceph_file_layout_pg_preferred(&ci->i_layout) >= 0) {
> +     if (ceph_file_layout_pg_preferred(&ci->i_layout) !=
> +                     CEPH_FILE_LAYOUT_PG_PREFERRED_NONE) {

The function returns cpu order, #define is little endian

>               val += ret;
>               size -= ret;
>               ret += snprintf(val, size, "preferred_osd=%lld\n",
> diff --git a/include/linux/ceph/ceph_fs.h b/include/linux/ceph/ceph_fs.h
> index 2645704..cf86032 100644
> --- a/include/linux/ceph/ceph_fs.h
> +++ b/include/linux/ceph/ceph_fs.h
> @@ -73,7 +73,8 @@ struct ceph_file_layout {
>   * file layout helpers
>   */
> 
> -#define CEPH_MIN_STRIPE_UNIT 65536
> +#define CEPH_MIN_STRIPE_UNIT                 65536
> +#define CEPH_FILE_LAYOUT_PG_PREFERRED_NONE   ((__s32) cpu_to_le32(-1))
>  #define ceph_file_layout_stripe_unit(l) \
>               ((__s32) le32_to_cpu((l)->fl_stripe_unit))
>  #define ceph_file_layout_stripe_count(l) \
> diff --git a/net/ceph/ceph_fs.c b/net/ceph/ceph_fs.c
> index 41466cc..c3197b9 100644
> --- a/net/ceph/ceph_fs.c
> +++ b/net/ceph/ceph_fs.c
> @@ -9,9 +9,9 @@
>   */
>  int ceph_file_layout_is_valid(const struct ceph_file_layout *layout)
>  {
> -     __u32 su = le32_to_cpu(layout->fl_stripe_unit);
> -     __u32 sc = le32_to_cpu(layout->fl_stripe_count);
> -     __u32 os = le32_to_cpu(layout->fl_object_size);
> +     __u32 su = (__u32) ceph_file_layout_stripe_unit(layout);
> +     __u32 sc = (__u32) ceph_file_layout_stripe_count(layout);
> +     __u32 os = (__u32) ceph_file_layout_object_size(layout);
> 
>       /* stripe unit, object size must be non-zero, 64k increment */
>       if (!su || (su & (CEPH_MIN_STRIPE_UNIT-1)))
> diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
> index 29ad46e..f5f6e41 100644
> --- a/net/ceph/osdmap.c
> +++ b/net/ceph/osdmap.c
> @@ -945,9 +945,9 @@ void ceph_calc_file_object_mapping(struct ceph_file_layout
> *layout,
>                                  u64 *ono,
>                                  u64 *oxoff, u64 *oxlen)
>  {
> -     u32 osize = le32_to_cpu(layout->fl_object_size);
> -     u32 su = le32_to_cpu(layout->fl_stripe_unit);
> -     u32 sc = le32_to_cpu(layout->fl_stripe_count);
> +     u32 osize = (u32) ceph_file_layout_object_size(layout);
> +     u32 su = (u32) ceph_file_layout_stripe_unit(layout);
> +     u32 sc = (u32) ceph_file_layout_stripe_count(layout);
>       u32 bl, stripeno, stripepos, objsetno;
>       u32 su_per_object;
>       u64 t, su_offset;
> @@ -1000,8 +1000,8 @@ int ceph_calc_object_layout(struct ceph_object_layout
> *ol,
>  {
>       unsigned num, num_mask;
>       struct ceph_pg pgid;
> -     s32 preferred = (s32)le32_to_cpu(fl->fl_pg_preferred);
> -     int poolid = le32_to_cpu(fl->fl_pg_pool);
> +     s32 preferred = (s32) ceph_file_layout_pg_preferred(fl);
> +     int poolid = (int) ceph_file_layout_pg_pool(fl);
>       struct ceph_pg_pool_info *pool;
>       unsigned ps;
> 
> @@ -1011,23 +1011,24 @@ int ceph_calc_object_layout(struct ceph_object_layout
> *ol,
>       if (!pool)
>               return -EIO;
>       ps = ceph_str_hash(pool->v.object_hash, oid, strlen(oid));
> -     if (preferred >= 0) {
> +
> +     if (preferred == CEPH_FILE_LAYOUT_PG_PREFERRED_NONE) {

Here, too.  I'd just stick with >= ...

> +             num = le32_to_cpu(pool->v.pg_num);
> +             num_mask = pool->pg_num_mask;
> +     } else {
>               ps += preferred;
>               num = le32_to_cpu(pool->v.lpg_num);
>               num_mask = pool->lpg_num_mask;
> -     } else {
> -             num = le32_to_cpu(pool->v.pg_num);
> -             num_mask = pool->pg_num_mask;
>       }
> 
>       pgid.ps = cpu_to_le16(ps);
>       pgid.preferred = cpu_to_le16(preferred);
>       pgid.pool = fl->fl_pg_pool;
> -     if (preferred >= 0)
> +     if (preferred == CEPH_FILE_LAYOUT_PG_PREFERRED_NONE)

and here.

BTW, Yehuda was periodically running the kernel code through sparse, which 
picks up all the endianness stuff.  That would be a worthy exercise to 
document and do from time to time.

> +             dout("calc_object_layout '%s' pgid %d.%x\n", oid, poolid, ps);
> +     else
>               dout("calc_object_layout '%s' pgid %d.%xp%d\n", oid, poolid,
> ps,
>                    (int)preferred);
> -     else
> -             dout("calc_object_layout '%s' pgid %d.%x\n", oid, poolid, ps);
> 
>       ol->ol_pgid = pgid;
>       ol->ol_stripe_unit = fl->fl_object_stripe_unit;
> -- 
> 1.7.5.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to