* [PATCH 1/5] ceph: move file layout helpers to ceph_fs.h
2012-03-12 22:50 [PATCH 0/5] ceph: ceph file layout helper fixes Alex Elder
@ 2012-03-12 22:57 ` Alex Elder
2012-03-12 22:57 ` [PATCH 2/5] ceph: make ceph file layout helpers take a pointer Alex Elder
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Alex Elder @ 2012-03-12 22:57 UTC (permalink / raw)
To: ceph-devel
The helper macros for accessing fields from struct ceph_file_layout
(which is defined in "include/linux/ceph/ceph_fs.h") are defined
instead in "include/linux/ceph/osdmap.h".
Move them, so they are defined in the same place as the structure
they apply to.
Signed-off-by: Alex Elder <elder@dreamhost.com>
---
include/linux/ceph/ceph_fs.h | 29 +++++++++++++++++++++++++++++
include/linux/ceph/osdmap.h | 29 -----------------------------
2 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/include/linux/ceph/ceph_fs.h b/include/linux/ceph/ceph_fs.h
index b8c6069..62e3bb3 100644
--- a/include/linux/ceph/ceph_fs.h
+++ b/include/linux/ceph/ceph_fs.h
@@ -69,7 +69,36 @@ struct ceph_file_layout {
__le32 fl_pg_pool; /* namespace, crush ruleset, rep level */
} __attribute__ ((packed));
+/*
+ * file layout helpers
+ */
+
#define CEPH_MIN_STRIPE_UNIT 65536
+#define ceph_file_layout_su(l) ((__s32)le32_to_cpu((l).fl_stripe_unit))
+#define ceph_file_layout_stripe_count(l) \
+ ((__s32)le32_to_cpu((l).fl_stripe_count))
+#define ceph_file_layout_object_size(l)
((__s32)le32_to_cpu((l).fl_object_size))
+#define ceph_file_layout_cas_hash(l) ((__s32)le32_to_cpu((l).fl_cas_hash))
+#define ceph_file_layout_object_su(l) \
+ ((__s32)le32_to_cpu((l).fl_object_stripe_unit))
+#define ceph_file_layout_pg_preferred(l) \
+ ((__s32)le32_to_cpu((l).fl_pg_preferred))
+#define ceph_file_layout_pg_pool(l) \
+ ((__s32)le32_to_cpu((l).fl_pg_pool))
+
+static inline unsigned ceph_file_layout_stripe_width(struct
ceph_file_layout *l)
+{
+ return le32_to_cpu(l->fl_stripe_unit) *
+ le32_to_cpu(l->fl_stripe_count);
+}
+
+/* "period" == bytes before i start on a new set of objects */
+static inline unsigned ceph_file_layout_period(struct ceph_file_layout *l)
+{
+ return le32_to_cpu(l->fl_object_size) *
+ le32_to_cpu(l->fl_stripe_count);
+}
+
int ceph_file_layout_is_valid(const struct ceph_file_layout *layout);
diff --git a/include/linux/ceph/osdmap.h b/include/linux/ceph/osdmap.h
index ba4c205..fb588b7 100644
--- a/include/linux/ceph/osdmap.h
+++ b/include/linux/ceph/osdmap.h
@@ -55,35 +55,6 @@ struct ceph_osdmap {
struct crush_map *crush;
};
-/*
- * file layout helpers
- */
-#define ceph_file_layout_su(l) ((__s32)le32_to_cpu((l).fl_stripe_unit))
-#define ceph_file_layout_stripe_count(l) \
- ((__s32)le32_to_cpu((l).fl_stripe_count))
-#define ceph_file_layout_object_size(l)
((__s32)le32_to_cpu((l).fl_object_size))
-#define ceph_file_layout_cas_hash(l) ((__s32)le32_to_cpu((l).fl_cas_hash))
-#define ceph_file_layout_object_su(l) \
- ((__s32)le32_to_cpu((l).fl_object_stripe_unit))
-#define ceph_file_layout_pg_preferred(l) \
- ((__s32)le32_to_cpu((l).fl_pg_preferred))
-#define ceph_file_layout_pg_pool(l) \
- ((__s32)le32_to_cpu((l).fl_pg_pool))
-
-static inline unsigned ceph_file_layout_stripe_width(struct
ceph_file_layout *l)
-{
- return le32_to_cpu(l->fl_stripe_unit) *
- le32_to_cpu(l->fl_stripe_count);
-}
-
-/* "period" == bytes before i start on a new set of objects */
-static inline unsigned ceph_file_layout_period(struct ceph_file_layout *l)
-{
- return le32_to_cpu(l->fl_object_size) *
- le32_to_cpu(l->fl_stripe_count);
-}
-
-
static inline int ceph_osd_is_up(struct ceph_osdmap *map, int osd)
{
return (osd < map->max_osd) && (map->osd_state[osd] & CEPH_OSD_UP);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/5] ceph: make ceph file layout helpers take a pointer
2012-03-12 22:50 [PATCH 0/5] ceph: ceph file layout helper fixes Alex Elder
2012-03-12 22:57 ` [PATCH 1/5] ceph: move file layout helpers to ceph_fs.h Alex Elder
@ 2012-03-12 22:57 ` Alex Elder
2012-03-12 22:57 ` [PATCH 3/5] ceph: make use of ceph file layout helpers Alex Elder
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Alex Elder @ 2012-03-12 22:57 UTC (permalink / raw)
To: ceph-devel
Make the helpers for accessing the fields of an on-disk ceph file
layout structure take the address of a layout structure rather than
the structure itself. It's more useful (and to my eye a bit more
normal).
Use the result in ceph_file_layout_stripe_width() and
ceph_file_layout_period(), since it's now possible.
Rename ceph_file_layout_su() to be ceph_file_layout_stripe_unit().
Signed-off-by: Alex Elder <elder@dreamhost.com>
---
fs/ceph/ioctl.c | 16 ++++++++--------
fs/ceph/xattr.c | 10 +++++-----
include/linux/ceph/ceph_fs.h | 28 +++++++++++++++-------------
3 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c
index 790914a59..23cb78b 100644
--- a/fs/ceph/ioctl.c
+++ b/fs/ceph/ioctl.c
@@ -22,9 +22,9 @@ 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_su(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.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.preferred_osd =
(s32)le32_to_cpu(ci->i_layout.fl_pg_preferred);
@@ -52,9 +52,9 @@ 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_su(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.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.preferred_osd =
(s32)le32_to_cpu(ci->i_layout.fl_pg_preferred);
@@ -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_su(ci->i_layout);
+ dl.object_size = ceph_file_layout_object_size(&ci->i_layout);
+ dl.block_size = 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 715cfc6..75960b1 100644
--- a/fs/ceph/xattr.c
+++ b/fs/ceph/xattr.c
@@ -115,16 +115,16 @@ 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_count(ci->i_layout),
- (unsigned long long)ceph_file_layout_object_size(ci->i_layout));
+ (unsigned long long)ceph_file_layout_su(&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) >= 0) {
val += ret;
size -= ret;
ret += snprintf(val, size, "preferred_osd=%lld\n",
(unsigned long long)ceph_file_layout_pg_preferred(
- ci->i_layout));
+ &ci->i_layout));
}
return ret;
diff --git a/include/linux/ceph/ceph_fs.h b/include/linux/ceph/ceph_fs.h
index 62e3bb3..2645704 100644
--- a/include/linux/ceph/ceph_fs.h
+++ b/include/linux/ceph/ceph_fs.h
@@ -74,32 +74,34 @@ struct ceph_file_layout {
*/
#define CEPH_MIN_STRIPE_UNIT 65536
-#define ceph_file_layout_su(l) ((__s32)le32_to_cpu((l).fl_stripe_unit))
+#define ceph_file_layout_stripe_unit(l) \
+ ((__s32) le32_to_cpu((l)->fl_stripe_unit))
#define ceph_file_layout_stripe_count(l) \
- ((__s32)le32_to_cpu((l).fl_stripe_count))
-#define ceph_file_layout_object_size(l)
((__s32)le32_to_cpu((l).fl_object_size))
-#define ceph_file_layout_cas_hash(l) ((__s32)le32_to_cpu((l).fl_cas_hash))
-#define ceph_file_layout_object_su(l) \
- ((__s32)le32_to_cpu((l).fl_object_stripe_unit))
+ ((__s32) le32_to_cpu((l)->fl_stripe_count))
+#define ceph_file_layout_object_size(l) \
+ ((__s32) le32_to_cpu((l)->fl_object_size))
+#define ceph_file_layout_cas_hash(l) \
+ ((__s32) le32_to_cpu((l)->fl_cas_hash))
+#define ceph_file_layout_object_stripe_unit(l) \
+ ((__s32) le32_to_cpu((l)->fl_object_stripe_unit))
#define ceph_file_layout_pg_preferred(l) \
- ((__s32)le32_to_cpu((l).fl_pg_preferred))
+ ((__s32) le32_to_cpu((l)->fl_pg_preferred))
#define ceph_file_layout_pg_pool(l) \
- ((__s32)le32_to_cpu((l).fl_pg_pool))
+ ((__s32) le32_to_cpu((l)->fl_pg_pool))
static inline unsigned ceph_file_layout_stripe_width(struct
ceph_file_layout *l)
{
- return le32_to_cpu(l->fl_stripe_unit) *
- le32_to_cpu(l->fl_stripe_count);
+ return (unsigned) (ceph_file_layout_stripe_unit(l) *
+ ceph_file_layout_stripe_count(l));
}
/* "period" == bytes before i start on a new set of objects */
static inline unsigned ceph_file_layout_period(struct ceph_file_layout *l)
{
- return le32_to_cpu(l->fl_object_size) *
- le32_to_cpu(l->fl_stripe_count);
+ return (unsigned) (ceph_file_layout_object_size(l) *
+ ceph_file_layout_stripe_count(l));
}
-
int ceph_file_layout_is_valid(const struct ceph_file_layout *layout);
struct ceph_dir_layout {
--
1.7.5.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/5] ceph: make use of ceph file layout helpers
2012-03-12 22:50 [PATCH 0/5] ceph: ceph file layout helper fixes Alex Elder
2012-03-12 22:57 ` [PATCH 1/5] ceph: move file layout helpers to ceph_fs.h Alex Elder
2012-03-12 22:57 ` [PATCH 2/5] ceph: make ceph file layout helpers take a pointer Alex Elder
@ 2012-03-12 22:57 ` Alex Elder
2012-03-16 5:59 ` Sage Weil
2012-03-12 22:57 ` [PATCH 4/5] ceph: use 64-bit math in ceph_calc_file_object_mapping() Alex Elder
2012-03-12 22:57 ` [PATCH 5/5] ceph: fix up the types of the file layout helpers Alex Elder
4 siblings, 1 reply; 12+ messages in thread
From: Alex Elder @ 2012-03-12 22:57 UTC (permalink / raw)
To: ceph-devel
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 <elder@dreamhost.com>
---
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) {
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) {
+ 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)
+ 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 3/5] ceph: make use of ceph file layout helpers
2012-03-12 22:57 ` [PATCH 3/5] ceph: make use of ceph file layout helpers Alex Elder
@ 2012-03-16 5:59 ` Sage Weil
2012-03-30 19:55 ` Alex Elder
0 siblings, 1 reply; 12+ messages in thread
From: Sage Weil @ 2012-03-16 5:59 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
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 <elder@dreamhost.com>
> ---
> 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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/5] ceph: make use of ceph file layout helpers
2012-03-16 5:59 ` Sage Weil
@ 2012-03-30 19:55 ` Alex Elder
0 siblings, 0 replies; 12+ messages in thread
From: Alex Elder @ 2012-03-30 19:55 UTC (permalink / raw)
To: Sage Weil; +Cc: ceph-devel
On 03/16/2012 12:59 AM, Sage Weil wrote:
> 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<elder@dreamhost.com>
A few comments below. I'm about to re-post this series.
. . .
>> 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
I will fix this--throughout. I'm going to define the constant
in CPU order instead (and fix the one place that depended on it
being in little endian order).
>> 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
. . .
>> @@ -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>= ...
No, I intentionally defined this constant to have a specific
meaning, and I don't want any sense that it is greater than
or less than something--just distinguished from all other
valid preferred PG values.
>
>> + 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 {
. . .
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/5] ceph: use 64-bit math in ceph_calc_file_object_mapping()
2012-03-12 22:50 [PATCH 0/5] ceph: ceph file layout helper fixes Alex Elder
` (2 preceding siblings ...)
2012-03-12 22:57 ` [PATCH 3/5] ceph: make use of ceph file layout helpers Alex Elder
@ 2012-03-12 22:57 ` Alex Elder
2012-03-16 6:17 ` Sage Weil
2012-03-12 22:57 ` [PATCH 5/5] ceph: fix up the types of the file layout helpers Alex Elder
4 siblings, 1 reply; 12+ messages in thread
From: Alex Elder @ 2012-03-12 22:57 UTC (permalink / raw)
To: ceph-devel
Convert ceph_calc_file_object_mapping() so it uses 64-bit variables
and divide operators. This is in anticipation of the next patch,
to keep the the changes it incurs more focused.
I made most of the variable names more verbose, and because it now
uses do_div() some of the calculations get done in a different
order. Net result is that the code does what it did before, but it
looks quite a bit different...
Signed-off-by: Alex Elder <elder@dreamhost.com>
---
net/ceph/osdmap.c | 112
+++++++++++++++++++++++++++++++++--------------------
1 files changed, 70 insertions(+), 42 deletions(-)
diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
index f5f6e41..26c30e7 100644
--- a/net/ceph/osdmap.c
+++ b/net/ceph/osdmap.c
@@ -933,59 +933,87 @@ bad:
/*
- * calculate file layout from given offset, length.
- * fill in correct oid, logical length, and object extent
- * offset, length.
+ * calculate file layout from given offset, length. fill in correct
+ * object number, logical length, and object extent offset and length.
*
- * for now, we write only a single su, until we can
- * pass a stride back to the caller.
+ * for now, we write only a single stripe_unit, until we can pass a
+ * stride back to the caller.
*/
void ceph_calc_file_object_mapping(struct ceph_file_layout *layout,
- u64 off, u64 *plen,
- u64 *ono,
- u64 *oxoff, u64 *oxlen)
+ u64 off,
+ u64 *plen,
+ u64 *object_num,
+ u64 *object_ext_off,
+ u64 *object_ext_len)
{
- 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;
-
- dout("mapping %llu~%llu osize %u fl_su %u\n", off, *plen,
- osize, su);
- su_per_object = osize / su;
- dout("osize %u / su %u = su_per_object %u\n", osize, su,
- su_per_object);
-
- BUG_ON((su & ~PAGE_MASK) != 0);
- /* bl = *off / su; */
- t = off;
- do_div(t, su);
- bl = t;
- dout("off %llu / su %u = bl %u\n", off, su, bl);
-
- stripeno = bl / sc;
- stripepos = bl % sc;
- objsetno = stripeno / su_per_object;
-
- *ono = objsetno * sc + stripepos;
- dout("objset %u * sc %u = ono %u\n", objsetno, sc, (unsigned)*ono);
-
- /* *oxoff = *off % layout->fl_stripe_unit; # offset in su */
- t = off;
- su_offset = do_div(t, su);
- *oxoff = su_offset + (stripeno % su_per_object) * su;
+ u64 object_size = (u64) ceph_file_layout_object_size(layout);
+ u64 stripe_unit = (u64) ceph_file_layout_stripe_unit(layout);
+ u64 stripe_count = (u64) ceph_file_layout_stripe_count(layout);
+ u64 stripe_unit_per_object;
+ u64 stripe_unit_num;
+ u64 stripe_unit_offset;
+ u64 stripe_num;
+ u64 stripe_pos; /* Which object within an object set */
+ u64 obj_set_num;
+ u64 obj_stripe_unit_num; /* Which stripe_unit within object */
+
+ BUG_ON((stripe_unit & ~PAGE_MASK) != 0);
+
+ dout("mapping %llu~%llu object_size %llu fl_stripe_unit %llu\n",
+ off, *plen, object_size, stripe_unit);
+
+ /* stripe_unit_per_object = object_size / stripe_unit; */
+ stripe_unit_per_object = object_size;
+ do_div(stripe_unit_per_object, stripe_unit);
+ dout("object_size %llu / stripe_unit %llu "
+ "= stripe_unit_per_object %llu\n",
+ object_size, stripe_unit, stripe_unit_per_object);
+
+ /*
+ * stripe_unit_num = off / stripe_unit;
+ * stripe_unit_offset = off % stripe_unit;
+ */
+ stripe_unit_num = off;
+ stripe_unit_offset = do_div(stripe_unit_num, stripe_unit);
+ dout("off %llu / stripe_unit %llu = "
+ "stripe_unit_num %llu rem stripe_unit_offset = %llu\n",
+ off, stripe_unit, stripe_unit_num, stripe_unit_offset);
+
+ /*
+ * stripe_num = stripe_unit_num / stripe_count;
+ * stripe_pos = stripe_unit_num % stripe_count;
+ */
+ stripe_num = stripe_unit_num;
+ stripe_pos = do_div(stripe_num, stripe_count);
+ dout("stripe_unit_num %llu / stripe_count %llu = "
+ "stripe_num %llu rem stripe_pos %llu\n",
+ stripe_unit_num, stripe_count, stripe_num, stripe_pos);
+
+ /*
+ * obj_set_num = stripe_num / stripe_unit_per_object;
+ * obj_stripe_unit_num = stripe_num % stripe_unit_per_object;
+ */
+ obj_set_num = stripe_num;
+ obj_stripe_unit_num = do_div(obj_set_num, stripe_unit_per_object);
+
+ *object_num = obj_set_num * stripe_count + stripe_pos;
+ dout("obj_set_num %llu * stripe_count %llu = object_num %llu\n",
+ obj_set_num, stripe_count, *object_num);
+ *object_ext_off = stripe_unit * obj_stripe_unit_num
+ + stripe_unit_offset;
+ dout("obj_stripe_unit_num %llu * stripe_unit %llu = "
+ "object_ext_off %llu\n",
+ obj_stripe_unit_num, stripe_unit, *object_ext_off);
/*
* Calculate the length of the extent being written to the selected
* object. This is the minimum of the full length requested (plen) or
* the remainder of the current stripe being written to.
*/
- *oxlen = min_t(u64, *plen, su - su_offset);
- *plen = *oxlen;
+ *object_ext_len = min_t(u64, *plen, stripe_unit - stripe_unit_offset);
+ *plen = *object_ext_len;
- dout(" obj extent %llu~%llu\n", *oxoff, *oxlen);
+ dout(" obj extent %llu~%llu\n", *object_ext_off, *object_ext_len);
}
EXPORT_SYMBOL(ceph_calc_file_object_mapping);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 4/5] ceph: use 64-bit math in ceph_calc_file_object_mapping()
2012-03-12 22:57 ` [PATCH 4/5] ceph: use 64-bit math in ceph_calc_file_object_mapping() Alex Elder
@ 2012-03-16 6:17 ` Sage Weil
2012-03-16 13:25 ` Alex Elder
0 siblings, 1 reply; 12+ messages in thread
From: Sage Weil @ 2012-03-16 6:17 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
On Mon, 12 Mar 2012, Alex Elder wrote:
> Convert ceph_calc_file_object_mapping() so it uses 64-bit variables
> and divide operators. This is in anticipation of the next patch,
> to keep the the changes it incurs more focused.
>
> I made most of the variable names more verbose, and because it now
> uses do_div() some of the calculations get done in a different
> order. Net result is that the code does what it did before, but it
> looks quite a bit different...
I didn't double check your math, but he code certainly looks nicer. It
would be nice to have a test that runs some tool that verifies re-read
data (iozone?) run when non-default file layouts. Say, 1MB stripes over 9
13MB objects, something like that. The 'cephfs' command line utility can
be used to set the default layout for new files under a
subdirectory/subtree, so running it on . in a workunit bash script (e.g.
based on ceph.git/qa/workunits/suites/iozone.sh) should do the trick.
('cephfs' only works on the kclient, though, so it needs to be a distinct
script that we don't already run on ceph-fuse... probably stick it in the
qa/workunits/kclient directory).
Not that this code was thoroughly tested with weird layouts before or
anything :), but it would be nice to see that before we merge this in.
The preferred osd stuff I'm less worried about. It would actually be nice
to rip all that out, assuming we can get a hadoop person to convince us
that it doesn't matter. :/
sage
>
> Signed-off-by: Alex Elder <elder@dreamhost.com>
> ---
> net/ceph/osdmap.c | 112
> +++++++++++++++++++++++++++++++++--------------------
> 1 files changed, 70 insertions(+), 42 deletions(-)
>
> diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
> index f5f6e41..26c30e7 100644
> --- a/net/ceph/osdmap.c
> +++ b/net/ceph/osdmap.c
> @@ -933,59 +933,87 @@ bad:
>
>
> /*
> - * calculate file layout from given offset, length.
> - * fill in correct oid, logical length, and object extent
> - * offset, length.
> + * calculate file layout from given offset, length. fill in correct
> + * object number, logical length, and object extent offset and length.
> *
> - * for now, we write only a single su, until we can
> - * pass a stride back to the caller.
> + * for now, we write only a single stripe_unit, until we can pass a
> + * stride back to the caller.
> */
> void ceph_calc_file_object_mapping(struct ceph_file_layout *layout,
> - u64 off, u64 *plen,
> - u64 *ono,
> - u64 *oxoff, u64 *oxlen)
> + u64 off,
> + u64 *plen,
> + u64 *object_num,
> + u64 *object_ext_off,
> + u64 *object_ext_len)
> {
> - 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;
> -
> - dout("mapping %llu~%llu osize %u fl_su %u\n", off, *plen,
> - osize, su);
> - su_per_object = osize / su;
> - dout("osize %u / su %u = su_per_object %u\n", osize, su,
> - su_per_object);
> -
> - BUG_ON((su & ~PAGE_MASK) != 0);
> - /* bl = *off / su; */
> - t = off;
> - do_div(t, su);
> - bl = t;
> - dout("off %llu / su %u = bl %u\n", off, su, bl);
> -
> - stripeno = bl / sc;
> - stripepos = bl % sc;
> - objsetno = stripeno / su_per_object;
> -
> - *ono = objsetno * sc + stripepos;
> - dout("objset %u * sc %u = ono %u\n", objsetno, sc, (unsigned)*ono);
> -
> - /* *oxoff = *off % layout->fl_stripe_unit; # offset in su */
> - t = off;
> - su_offset = do_div(t, su);
> - *oxoff = su_offset + (stripeno % su_per_object) * su;
> + u64 object_size = (u64) ceph_file_layout_object_size(layout);
> + u64 stripe_unit = (u64) ceph_file_layout_stripe_unit(layout);
> + u64 stripe_count = (u64) ceph_file_layout_stripe_count(layout);
> + u64 stripe_unit_per_object;
> + u64 stripe_unit_num;
> + u64 stripe_unit_offset;
> + u64 stripe_num;
> + u64 stripe_pos; /* Which object within an object set
> */
> + u64 obj_set_num;
> + u64 obj_stripe_unit_num; /* Which stripe_unit within object */
> +
> + BUG_ON((stripe_unit & ~PAGE_MASK) != 0);
> +
> + dout("mapping %llu~%llu object_size %llu fl_stripe_unit %llu\n",
> + off, *plen, object_size, stripe_unit);
> +
> + /* stripe_unit_per_object = object_size / stripe_unit; */
> + stripe_unit_per_object = object_size;
> + do_div(stripe_unit_per_object, stripe_unit);
> + dout("object_size %llu / stripe_unit %llu "
> + "= stripe_unit_per_object %llu\n",
> + object_size, stripe_unit, stripe_unit_per_object);
> +
> + /*
> + * stripe_unit_num = off / stripe_unit;
> + * stripe_unit_offset = off % stripe_unit;
> + */
> + stripe_unit_num = off;
> + stripe_unit_offset = do_div(stripe_unit_num, stripe_unit);
> + dout("off %llu / stripe_unit %llu = "
> + "stripe_unit_num %llu rem stripe_unit_offset =
> %llu\n",
> + off, stripe_unit, stripe_unit_num, stripe_unit_offset);
> +
> + /*
> + * stripe_num = stripe_unit_num / stripe_count;
> + * stripe_pos = stripe_unit_num % stripe_count;
> + */
> + stripe_num = stripe_unit_num;
> + stripe_pos = do_div(stripe_num, stripe_count);
> + dout("stripe_unit_num %llu / stripe_count %llu = "
> + "stripe_num %llu rem stripe_pos %llu\n",
> + stripe_unit_num, stripe_count, stripe_num, stripe_pos);
> +
> + /*
> + * obj_set_num = stripe_num / stripe_unit_per_object;
> + * obj_stripe_unit_num = stripe_num % stripe_unit_per_object;
> + */
> + obj_set_num = stripe_num;
> + obj_stripe_unit_num = do_div(obj_set_num, stripe_unit_per_object);
> +
> + *object_num = obj_set_num * stripe_count + stripe_pos;
> + dout("obj_set_num %llu * stripe_count %llu = object_num %llu\n",
> + obj_set_num, stripe_count, *object_num);
> + *object_ext_off = stripe_unit * obj_stripe_unit_num
> + + stripe_unit_offset;
> + dout("obj_stripe_unit_num %llu * stripe_unit %llu = "
> + "object_ext_off %llu\n",
> + obj_stripe_unit_num, stripe_unit, *object_ext_off);
>
> /*
> * Calculate the length of the extent being written to the selected
> * object. This is the minimum of the full length requested (plen) or
> * the remainder of the current stripe being written to.
> */
> - *oxlen = min_t(u64, *plen, su - su_offset);
> - *plen = *oxlen;
> + *object_ext_len = min_t(u64, *plen, stripe_unit - stripe_unit_offset);
> + *plen = *object_ext_len;
>
> - dout(" obj extent %llu~%llu\n", *oxoff, *oxlen);
> + dout(" obj extent %llu~%llu\n", *object_ext_off, *object_ext_len);
> }
> EXPORT_SYMBOL(ceph_calc_file_object_mapping);
>
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 4/5] ceph: use 64-bit math in ceph_calc_file_object_mapping()
2012-03-16 6:17 ` Sage Weil
@ 2012-03-16 13:25 ` Alex Elder
0 siblings, 0 replies; 12+ messages in thread
From: Alex Elder @ 2012-03-16 13:25 UTC (permalink / raw)
To: Sage Weil; +Cc: ceph-devel
On 03/16/2012 01:17 AM, Sage Weil wrote:
> On Mon, 12 Mar 2012, Alex Elder wrote:
>> Convert ceph_calc_file_object_mapping() so it uses 64-bit variables
>> and divide operators. This is in anticipation of the next patch,
>> to keep the the changes it incurs more focused.
>>
>> I made most of the variable names more verbose, and because it now
>> uses do_div() some of the calculations get done in a different
>> order. Net result is that the code does what it did before, but it
>> looks quite a bit different...
>
> I didn't double check your math, but he code certainly looks nicer. It
> would be nice to have a test that runs some tool that verifies re-read
> data (iozone?) run when non-default file layouts. Say, 1MB stripes over 9
> 13MB objects, something like that. The 'cephfs' command line utility can
> be used to set the default layout for new files under a
> subdirectory/subtree, so running it on . in a workunit bash script (e.g.
> based on ceph.git/qa/workunits/suites/iozone.sh) should do the trick.
> ('cephfs' only works on the kclient, though, so it needs to be a distinct
> script that we don't already run on ceph-fuse... probably stick it in the
> qa/workunits/kclient directory).
>
> Not that this code was thoroughly tested with weird layouts before or
> anything :), but it would be nice to see that before we merge this in.
>
> The preferred osd stuff I'm less worried about. It would actually be nice
> to rip all that out, assuming we can get a hadoop person to convince us
> that it doesn't matter. :/
Wow, nice, thorough review. Thank you!
I'll also think about and work on how to test these changes. I'm
sure I'll need some more input on that (not necessarily from you).
Anyway, thanks again for the review. I will re-check the whole
series, given the new insights you have provided, and will re-post it.
-Alex
> sage
>
>>
>> Signed-off-by: Alex Elder<elder@dreamhost.com>
>> ---
>> net/ceph/osdmap.c | 112
>> +++++++++++++++++++++++++++++++++--------------------
>> 1 files changed, 70 insertions(+), 42 deletions(-)
>>
>> diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
>> index f5f6e41..26c30e7 100644
>> --- a/net/ceph/osdmap.c
>> +++ b/net/ceph/osdmap.c
>> @@ -933,59 +933,87 @@ bad:
>>
>>
>> /*
>> - * calculate file layout from given offset, length.
>> - * fill in correct oid, logical length, and object extent
>> - * offset, length.
>> + * calculate file layout from given offset, length. fill in correct
>> + * object number, logical length, and object extent offset and length.
>> *
>> - * for now, we write only a single su, until we can
>> - * pass a stride back to the caller.
>> + * for now, we write only a single stripe_unit, until we can pass a
>> + * stride back to the caller.
>> */
>> void ceph_calc_file_object_mapping(struct ceph_file_layout *layout,
>> - u64 off, u64 *plen,
>> - u64 *ono,
>> - u64 *oxoff, u64 *oxlen)
>> + u64 off,
>> + u64 *plen,
>> + u64 *object_num,
>> + u64 *object_ext_off,
>> + u64 *object_ext_len)
>> {
>> - 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;
>> -
>> - dout("mapping %llu~%llu osize %u fl_su %u\n", off, *plen,
>> - osize, su);
>> - su_per_object = osize / su;
>> - dout("osize %u / su %u = su_per_object %u\n", osize, su,
>> - su_per_object);
>> -
>> - BUG_ON((su& ~PAGE_MASK) != 0);
>> - /* bl = *off / su; */
>> - t = off;
>> - do_div(t, su);
>> - bl = t;
>> - dout("off %llu / su %u = bl %u\n", off, su, bl);
>> -
>> - stripeno = bl / sc;
>> - stripepos = bl % sc;
>> - objsetno = stripeno / su_per_object;
>> -
>> - *ono = objsetno * sc + stripepos;
>> - dout("objset %u * sc %u = ono %u\n", objsetno, sc, (unsigned)*ono);
>> -
>> - /* *oxoff = *off % layout->fl_stripe_unit; # offset in su */
>> - t = off;
>> - su_offset = do_div(t, su);
>> - *oxoff = su_offset + (stripeno % su_per_object) * su;
>> + u64 object_size = (u64) ceph_file_layout_object_size(layout);
>> + u64 stripe_unit = (u64) ceph_file_layout_stripe_unit(layout);
>> + u64 stripe_count = (u64) ceph_file_layout_stripe_count(layout);
>> + u64 stripe_unit_per_object;
>> + u64 stripe_unit_num;
>> + u64 stripe_unit_offset;
>> + u64 stripe_num;
>> + u64 stripe_pos; /* Which object within an object set
>> */
>> + u64 obj_set_num;
>> + u64 obj_stripe_unit_num; /* Which stripe_unit within object */
>> +
>> + BUG_ON((stripe_unit& ~PAGE_MASK) != 0);
>> +
>> + dout("mapping %llu~%llu object_size %llu fl_stripe_unit %llu\n",
>> + off, *plen, object_size, stripe_unit);
>> +
>> + /* stripe_unit_per_object = object_size / stripe_unit; */
>> + stripe_unit_per_object = object_size;
>> + do_div(stripe_unit_per_object, stripe_unit);
>> + dout("object_size %llu / stripe_unit %llu "
>> + "= stripe_unit_per_object %llu\n",
>> + object_size, stripe_unit, stripe_unit_per_object);
>> +
>> + /*
>> + * stripe_unit_num = off / stripe_unit;
>> + * stripe_unit_offset = off % stripe_unit;
>> + */
>> + stripe_unit_num = off;
>> + stripe_unit_offset = do_div(stripe_unit_num, stripe_unit);
>> + dout("off %llu / stripe_unit %llu = "
>> + "stripe_unit_num %llu rem stripe_unit_offset =
>> %llu\n",
>> + off, stripe_unit, stripe_unit_num, stripe_unit_offset);
>> +
>> + /*
>> + * stripe_num = stripe_unit_num / stripe_count;
>> + * stripe_pos = stripe_unit_num % stripe_count;
>> + */
>> + stripe_num = stripe_unit_num;
>> + stripe_pos = do_div(stripe_num, stripe_count);
>> + dout("stripe_unit_num %llu / stripe_count %llu = "
>> + "stripe_num %llu rem stripe_pos %llu\n",
>> + stripe_unit_num, stripe_count, stripe_num, stripe_pos);
>> +
>> + /*
>> + * obj_set_num = stripe_num / stripe_unit_per_object;
>> + * obj_stripe_unit_num = stripe_num % stripe_unit_per_object;
>> + */
>> + obj_set_num = stripe_num;
>> + obj_stripe_unit_num = do_div(obj_set_num, stripe_unit_per_object);
>> +
>> + *object_num = obj_set_num * stripe_count + stripe_pos;
>> + dout("obj_set_num %llu * stripe_count %llu = object_num %llu\n",
>> + obj_set_num, stripe_count, *object_num);
>> + *object_ext_off = stripe_unit * obj_stripe_unit_num
>> + + stripe_unit_offset;
>> + dout("obj_stripe_unit_num %llu * stripe_unit %llu = "
>> + "object_ext_off %llu\n",
>> + obj_stripe_unit_num, stripe_unit, *object_ext_off);
>>
>> /*
>> * Calculate the length of the extent being written to the selected
>> * object. This is the minimum of the full length requested (plen) or
>> * the remainder of the current stripe being written to.
>> */
>> - *oxlen = min_t(u64, *plen, su - su_offset);
>> - *plen = *oxlen;
>> + *object_ext_len = min_t(u64, *plen, stripe_unit - stripe_unit_offset);
>> + *plen = *object_ext_len;
>>
>> - dout(" obj extent %llu~%llu\n", *oxoff, *oxlen);
>> + dout(" obj extent %llu~%llu\n", *object_ext_off, *object_ext_len);
>> }
>> EXPORT_SYMBOL(ceph_calc_file_object_mapping);
>>
>> --
>> 1.7.5.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 5/5] ceph: fix up the types of the file layout helpers
2012-03-12 22:50 [PATCH 0/5] ceph: ceph file layout helper fixes Alex Elder
` (3 preceding siblings ...)
2012-03-12 22:57 ` [PATCH 4/5] ceph: use 64-bit math in ceph_calc_file_object_mapping() Alex Elder
@ 2012-03-12 22:57 ` Alex Elder
2012-03-16 6:10 ` Sage Weil
4 siblings, 1 reply; 12+ messages in thread
From: Alex Elder @ 2012-03-12 22:57 UTC (permalink / raw)
To: ceph-devel
The helper macros for accessing file layout fields of an on-disk
ceph file layout structure cast their results to type (__s32). This
is a bit strange, since (with one exception--fl_pg_preferred):
- there is no need for negative values; and
- all users of these macros are assigning their result to
64-bit variables.
So just make these macros return a 64-bit unsigned type.
The exception is the preferred placement group, which remains a
signed 32-bit value. A placement group id encodes the preferred
primary OSD in a 16-bit value, and there's no sense at this point
getting too far away from that.
Signed-off-by: Alex Elder <elder@dreamhost.com>
---
fs/ceph/inode.c | 6 +++++-
fs/ceph/ioctl.c | 22 +++++++++++-----------
fs/ceph/xattr.c | 16 ++++++++--------
include/linux/ceph/ceph_fs.h | 26 ++++++++++++--------------
net/ceph/ceph_fs.c | 6 +++---
net/ceph/osdmap.c | 25 +++++++++++++------------
6 files changed, 52 insertions(+), 49 deletions(-)
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index d291928..1828fb9 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -568,6 +568,7 @@ static int fill_inode(struct inode *inode,
struct ceph_buffer *xattr_blob = NULL;
int err = 0;
int queue_trunc = 0;
+ __u64 stripe_unit;
dout("fill_inode %p ino %llx.%llx v %llu had %llu\n",
inode, ceph_vinop(inode), le64_to_cpu(info->version),
@@ -643,7 +644,10 @@ static int fill_inode(struct inode *inode,
}
ci->i_layout = info->layout;
- inode->i_blkbits = fls(le32_to_cpu(info->layout.fl_stripe_unit)) - 1;
+
+ stripe_unit = ceph_file_layout_stripe_unit(&info->layout);
+ BUG_ON(stripe_unit > (__u64) INT_MAX);
+ inode->i_blkbits = fls((int) stripe_unit) - 1;
/* xattrs */
/* note that if i_xattrs.len <= 4, i_xattrs.data will still be NULL. */
diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c
index 75cff03..2fde342 100644
--- a/fs/ceph/ioctl.c
+++ b/fs/ceph/ioctl.c
@@ -22,10 +22,10 @@ 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 = (__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.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 = ceph_file_layout_pg_pool(&ci->i_layout);
l.preferred_osd =
(__s64) ceph_file_layout_pg_preferred(&ci->i_layout);
if (copy_to_user(arg, &l, sizeof(l)))
@@ -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 = (__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.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 = ceph_file_layout_pg_pool(&ci->i_layout);
nl.preferred_osd =
- (__s64) ceph_file_layout_pg_preferred(&ci->i_layout);
+ (__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 = (__u64) ceph_file_layout_object_size(&ci->i_layout);
- dl.block_size = (__u64) ceph_file_layout_stripe_unit(&ci->i_layout);
+ dl.object_size = ceph_file_layout_object_size(&ci->i_layout);
+ dl.block_size = 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 bfd5b93..0652aa4 100644
--- a/fs/ceph/xattr.c
+++ b/fs/ceph/xattr.c
@@ -112,20 +112,20 @@ static size_t ceph_vxattrcb_file_layout(struct
ceph_inode_info *ci, char *val,
size_t size)
{
int ret;
+ __s32 preferred;
ret = snprintf(val, size,
"chunk_bytes=%lld\nstripe_count=%lld\nobject_size=%lld\n",
- (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));
+ ceph_file_layout_stripe_unit(&ci->i_layout),
+ ceph_file_layout_stripe_count(&ci->i_layout),
+ ceph_file_layout_object_size(&ci->i_layout));
- if (ceph_file_layout_pg_preferred(&ci->i_layout) !=
- CEPH_FILE_LAYOUT_PG_PREFERRED_NONE) {
+ preferred = ceph_file_layout_pg_preferred(&ci->i_layout);
+ if (preferred != CEPH_FILE_LAYOUT_PG_PREFERRED_NONE) {
val += ret;
size -= ret;
- ret += snprintf(val, size, "preferred_osd=%lld\n",
- (unsigned long long)ceph_file_layout_pg_preferred(
- &ci->i_layout));
+ ret += snprintf(val, size, "preferred_osd=%d\n",
+ (int) preferred);
}
return ret;
diff --git a/include/linux/ceph/ceph_fs.h b/include/linux/ceph/ceph_fs.h
index cf86032..698d395 100644
--- a/include/linux/ceph/ceph_fs.h
+++ b/include/linux/ceph/ceph_fs.h
@@ -75,32 +75,30 @@ struct ceph_file_layout {
#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))
+ ((__u64) le32_to_cpu((l)->fl_stripe_unit))
#define ceph_file_layout_stripe_count(l) \
- ((__s32) le32_to_cpu((l)->fl_stripe_count))
+ ((__u64) le32_to_cpu((l)->fl_stripe_count))
#define ceph_file_layout_object_size(l) \
- ((__s32) le32_to_cpu((l)->fl_object_size))
-#define ceph_file_layout_cas_hash(l) \
- ((__s32) le32_to_cpu((l)->fl_cas_hash))
-#define ceph_file_layout_object_stripe_unit(l) \
- ((__s32) le32_to_cpu((l)->fl_object_stripe_unit))
+ ((__u64) le32_to_cpu((l)->fl_object_size))
+#define ceph_file_layout_cas_hash(l) le32_to_cpu((l)->fl_cas_hash)
+#define ceph_file_layout_object_stripe_unit(l)
le32_to_cpu((l)->fl_object_stripe_unit)
#define ceph_file_layout_pg_preferred(l) \
((__s32) le32_to_cpu((l)->fl_pg_preferred))
#define ceph_file_layout_pg_pool(l) \
- ((__s32) le32_to_cpu((l)->fl_pg_pool))
+ ((__u64) le32_to_cpu((l)->fl_pg_pool))
-static inline unsigned ceph_file_layout_stripe_width(struct
ceph_file_layout *l)
+static inline __u64 ceph_file_layout_stripe_width(struct
ceph_file_layout *l)
{
- return (unsigned) (ceph_file_layout_stripe_unit(l) *
- ceph_file_layout_stripe_count(l));
+ return ceph_file_layout_stripe_unit(l) * ceph_file_layout_stripe_count(l);
}
/* "period" == bytes before i start on a new set of objects */
-static inline unsigned ceph_file_layout_period(struct ceph_file_layout *l)
+static inline __u64 ceph_file_layout_period(struct ceph_file_layout *l)
{
- return (unsigned) (ceph_file_layout_object_size(l) *
- ceph_file_layout_stripe_count(l));
+ return ceph_file_layout_object_size(l) *
+ ceph_file_layout_stripe_count(l);
}
int ceph_file_layout_is_valid(const struct ceph_file_layout *layout);
diff --git a/net/ceph/ceph_fs.c b/net/ceph/ceph_fs.c
index c3197b9..97f7c50 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 = (__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);
+ __u64 su = ceph_file_layout_stripe_unit(layout);
+ __u64 sc = ceph_file_layout_stripe_count(layout);
+ __u64 os = 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 26c30e7..bc2d807 100644
--- a/net/ceph/osdmap.c
+++ b/net/ceph/osdmap.c
@@ -946,9 +946,9 @@ void ceph_calc_file_object_mapping(struct
ceph_file_layout *layout,
u64 *object_ext_off,
u64 *object_ext_len)
{
- u64 object_size = (u64) ceph_file_layout_object_size(layout);
- u64 stripe_unit = (u64) ceph_file_layout_stripe_unit(layout);
- u64 stripe_count = (u64) ceph_file_layout_stripe_count(layout);
+ u64 object_size = ceph_file_layout_object_size(layout);
+ u64 stripe_unit = ceph_file_layout_stripe_unit(layout);
+ u64 stripe_count = ceph_file_layout_stripe_count(layout);
u64 stripe_unit_per_object;
u64 stripe_unit_num;
u64 stripe_unit_offset;
@@ -1028,29 +1028,30 @@ int ceph_calc_object_layout(struct
ceph_object_layout *ol,
{
unsigned num, num_mask;
struct ceph_pg pgid;
- s32 preferred = (s32) ceph_file_layout_pg_preferred(fl);
+ s32 preferred = ceph_file_layout_pg_preferred(fl);
int poolid = (int) ceph_file_layout_pg_pool(fl);
struct ceph_pg_pool_info *pool;
unsigned ps;
BUG_ON(!osdmap);
+ BUG_ON(preferred > (s32) SHRT_MAX);
+ BUG_ON(preferred < (s32) SHRT_MIN);
pool = __lookup_pg_pool(&osdmap->pg_pools, poolid);
if (!pool)
return -EIO;
- ps = ceph_str_hash(pool->v.object_hash, oid, strlen(oid));
- if (preferred == CEPH_FILE_LAYOUT_PG_PREFERRED_NONE) {
- num = le32_to_cpu(pool->v.pg_num);
- num_mask = pool->pg_num_mask;
- } else {
+ ps = ceph_str_hash(pool->v.object_hash, oid, strlen(oid));
+ if (preferred != CEPH_FILE_LAYOUT_PG_PREFERRED_NONE) {
+ BUG_ON(preferred < 0);
ps += preferred;
- num = le32_to_cpu(pool->v.lpg_num);
- num_mask = pool->lpg_num_mask;
}
+ num = le32_to_cpu(pool->v.lpg_num);
+ num_mask = pool->lpg_num_mask;
+
pgid.ps = cpu_to_le16(ps);
- pgid.preferred = cpu_to_le16(preferred);
+ pgid.preferred = cpu_to_le16((s16) preferred);
pgid.pool = fl->fl_pg_pool;
if (preferred == CEPH_FILE_LAYOUT_PG_PREFERRED_NONE)
dout("calc_object_layout '%s' pgid %d.%x\n", oid, poolid, ps);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 5/5] ceph: fix up the types of the file layout helpers
2012-03-12 22:57 ` [PATCH 5/5] ceph: fix up the types of the file layout helpers Alex Elder
@ 2012-03-16 6:10 ` Sage Weil
2012-03-30 19:55 ` Alex Elder
0 siblings, 1 reply; 12+ messages in thread
From: Sage Weil @ 2012-03-16 6:10 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
On Mon, 12 Mar 2012, Alex Elder wrote:
> The helper macros for accessing file layout fields of an on-disk
> ceph file layout structure cast their results to type (__s32). This
> is a bit strange, since (with one exception--fl_pg_preferred):
> - there is no need for negative values; and
> - all users of these macros are assigning their result to
> 64-bit variables.
> So just make these macros return a 64-bit unsigned type.
>
> The exception is the preferred placement group, which remains a
> signed 32-bit value. A placement group id encodes the preferred
> primary OSD in a 16-bit value, and there's no sense at this point
> getting too far away from that.
>
> Signed-off-by: Alex Elder <elder@dreamhost.com>
> ---
> fs/ceph/inode.c | 6 +++++-
> fs/ceph/ioctl.c | 22 +++++++++++-----------
> fs/ceph/xattr.c | 16 ++++++++--------
> include/linux/ceph/ceph_fs.h | 26 ++++++++++++--------------
> net/ceph/ceph_fs.c | 6 +++---
> net/ceph/osdmap.c | 25 +++++++++++++------------
> 6 files changed, 52 insertions(+), 49 deletions(-)
>
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index d291928..1828fb9 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -568,6 +568,7 @@ static int fill_inode(struct inode *inode,
> struct ceph_buffer *xattr_blob = NULL;
> int err = 0;
> int queue_trunc = 0;
> + __u64 stripe_unit;
>
> dout("fill_inode %p ino %llx.%llx v %llu had %llu\n",
> inode, ceph_vinop(inode), le64_to_cpu(info->version),
> @@ -643,7 +644,10 @@ static int fill_inode(struct inode *inode,
> }
>
> ci->i_layout = info->layout;
> - inode->i_blkbits = fls(le32_to_cpu(info->layout.fl_stripe_unit)) - 1;
> +
> + stripe_unit = ceph_file_layout_stripe_unit(&info->layout);
> + BUG_ON(stripe_unit > (__u64) INT_MAX);
> + inode->i_blkbits = fls((int) stripe_unit) - 1;
>
> /* xattrs */
> /* note that if i_xattrs.len <= 4, i_xattrs.data will still be NULL.
> */
> diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c
> index 75cff03..2fde342 100644
> --- a/fs/ceph/ioctl.c
> +++ b/fs/ceph/ioctl.c
> @@ -22,10 +22,10 @@ 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 = (__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.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 = ceph_file_layout_pg_pool(&ci->i_layout);
> l.preferred_osd =
> (__s64) ceph_file_layout_pg_preferred(&ci->i_layout);
> if (copy_to_user(arg, &l, sizeof(l)))
> @@ -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 = (__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.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 = ceph_file_layout_pg_pool(&ci->i_layout);
> nl.preferred_osd =
> - (__s64)
> ceph_file_layout_pg_preferred(&ci->i_layout);
> + (__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 = (__u64) ceph_file_layout_object_size(&ci->i_layout);
> - dl.block_size = (__u64) ceph_file_layout_stripe_unit(&ci->i_layout);
> + dl.object_size = ceph_file_layout_object_size(&ci->i_layout);
> + dl.block_size = 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 bfd5b93..0652aa4 100644
> --- a/fs/ceph/xattr.c
> +++ b/fs/ceph/xattr.c
> @@ -112,20 +112,20 @@ static size_t ceph_vxattrcb_file_layout(struct
> ceph_inode_info *ci, char *val,
> size_t size)
> {
> int ret;
> + __s32 preferred;
>
> ret = snprintf(val, size,
> "chunk_bytes=%lld\nstripe_count=%lld\nobject_size=%lld\n",
> - (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));
> + ceph_file_layout_stripe_unit(&ci->i_layout),
> + ceph_file_layout_stripe_count(&ci->i_layout),
> + ceph_file_layout_object_size(&ci->i_layout));
>
> - if (ceph_file_layout_pg_preferred(&ci->i_layout) !=
> - CEPH_FILE_LAYOUT_PG_PREFERRED_NONE) {
> + preferred = ceph_file_layout_pg_preferred(&ci->i_layout);
> + if (preferred != CEPH_FILE_LAYOUT_PG_PREFERRED_NONE) {
> val += ret;
> size -= ret;
> - ret += snprintf(val, size, "preferred_osd=%lld\n",
> - (unsigned long long)ceph_file_layout_pg_preferred(
> - &ci->i_layout));
> + ret += snprintf(val, size, "preferred_osd=%d\n",
> + (int) preferred);
> }
>
> return ret;
> diff --git a/include/linux/ceph/ceph_fs.h b/include/linux/ceph/ceph_fs.h
> index cf86032..698d395 100644
> --- a/include/linux/ceph/ceph_fs.h
> +++ b/include/linux/ceph/ceph_fs.h
> @@ -75,32 +75,30 @@ struct ceph_file_layout {
>
> #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))
> + ((__u64) le32_to_cpu((l)->fl_stripe_unit))
> #define ceph_file_layout_stripe_count(l) \
> - ((__s32) le32_to_cpu((l)->fl_stripe_count))
> + ((__u64) le32_to_cpu((l)->fl_stripe_count))
> #define ceph_file_layout_object_size(l) \
> - ((__s32) le32_to_cpu((l)->fl_object_size))
> -#define ceph_file_layout_cas_hash(l) \
> - ((__s32) le32_to_cpu((l)->fl_cas_hash))
> -#define ceph_file_layout_object_stripe_unit(l) \
> - ((__s32) le32_to_cpu((l)->fl_object_stripe_unit))
> + ((__u64) le32_to_cpu((l)->fl_object_size))
> +#define ceph_file_layout_cas_hash(l) le32_to_cpu((l)->fl_cas_hash)
> +#define ceph_file_layout_object_stripe_unit(l)
> le32_to_cpu((l)->fl_object_stripe_unit)
> #define ceph_file_layout_pg_preferred(l) \
> ((__s32) le32_to_cpu((l)->fl_pg_preferred))
> #define ceph_file_layout_pg_pool(l) \
> - ((__s32) le32_to_cpu((l)->fl_pg_pool))
> + ((__u64) le32_to_cpu((l)->fl_pg_pool))
>
> -static inline unsigned ceph_file_layout_stripe_width(struct ceph_file_layout
> *l)
> +static inline __u64 ceph_file_layout_stripe_width(struct ceph_file_layout *l)
> {
> - return (unsigned) (ceph_file_layout_stripe_unit(l) *
> - ceph_file_layout_stripe_count(l));
> + return ceph_file_layout_stripe_unit(l) *
> ceph_file_layout_stripe_count(l);
> }
>
> /* "period" == bytes before i start on a new set of objects */
> -static inline unsigned ceph_file_layout_period(struct ceph_file_layout *l)
> +static inline __u64 ceph_file_layout_period(struct ceph_file_layout *l)
> {
> - return (unsigned) (ceph_file_layout_object_size(l) *
> - ceph_file_layout_stripe_count(l));
> + return ceph_file_layout_object_size(l) *
> + ceph_file_layout_stripe_count(l);
> }
>
> int ceph_file_layout_is_valid(const struct ceph_file_layout *layout);
> diff --git a/net/ceph/ceph_fs.c b/net/ceph/ceph_fs.c
> index c3197b9..97f7c50 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 = (__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);
> + __u64 su = ceph_file_layout_stripe_unit(layout);
> + __u64 sc = ceph_file_layout_stripe_count(layout);
> + __u64 os = 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 26c30e7..bc2d807 100644
> --- a/net/ceph/osdmap.c
> +++ b/net/ceph/osdmap.c
> @@ -946,9 +946,9 @@ void ceph_calc_file_object_mapping(struct ceph_file_layout
> *layout,
> u64 *object_ext_off,
> u64 *object_ext_len)
> {
> - u64 object_size = (u64) ceph_file_layout_object_size(layout);
> - u64 stripe_unit = (u64) ceph_file_layout_stripe_unit(layout);
> - u64 stripe_count = (u64) ceph_file_layout_stripe_count(layout);
> + u64 object_size = ceph_file_layout_object_size(layout);
> + u64 stripe_unit = ceph_file_layout_stripe_unit(layout);
> + u64 stripe_count = ceph_file_layout_stripe_count(layout);
> u64 stripe_unit_per_object;
> u64 stripe_unit_num;
> u64 stripe_unit_offset;
> @@ -1028,29 +1028,30 @@ int ceph_calc_object_layout(struct ceph_object_layout
> *ol,
> {
> unsigned num, num_mask;
> struct ceph_pg pgid;
> - s32 preferred = (s32) ceph_file_layout_pg_preferred(fl);
> + s32 preferred = ceph_file_layout_pg_preferred(fl);
> int poolid = (int) ceph_file_layout_pg_pool(fl);
> struct ceph_pg_pool_info *pool;
> unsigned ps;
>
> BUG_ON(!osdmap);
> + BUG_ON(preferred > (s32) SHRT_MAX);
> + BUG_ON(preferred < (s32) SHRT_MIN);
>
> pool = __lookup_pg_pool(&osdmap->pg_pools, poolid);
> if (!pool)
> return -EIO;
> - ps = ceph_str_hash(pool->v.object_hash, oid, strlen(oid));
>
> - if (preferred == CEPH_FILE_LAYOUT_PG_PREFERRED_NONE) {
> - num = le32_to_cpu(pool->v.pg_num);
> - num_mask = pool->pg_num_mask;
> - } else {
> + ps = ceph_str_hash(pool->v.object_hash, oid, strlen(oid));
> + if (preferred != CEPH_FILE_LAYOUT_PG_PREFERRED_NONE) {
> + BUG_ON(preferred < 0);
> ps += preferred;
> - num = le32_to_cpu(pool->v.lpg_num);
> - num_mask = pool->lpg_num_mask;
> }
>
> + num = le32_to_cpu(pool->v.lpg_num);
> + num_mask = pool->lpg_num_mask;
Unless i'm misreading this diff, you're switching to always using lpg_num
and lpg_num_mask instead of pg_num and pg_num_mask. That "l" prefix means
"localized," for a different set of localized PGs used when htere is a
preferred OSD. We need to keep these assignments in the if and else
blocks...
> +
> pgid.ps = cpu_to_le16(ps);
> - pgid.preferred = cpu_to_le16(preferred);
> + pgid.preferred = cpu_to_le16((s16) preferred);
> pgid.pool = fl->fl_pg_pool;
> if (preferred == CEPH_FILE_LAYOUT_PG_PREFERRED_NONE)
> dout("calc_object_layout '%s' pgid %d.%x\n", oid, poolid, ps);
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 5/5] ceph: fix up the types of the file layout helpers
2012-03-16 6:10 ` Sage Weil
@ 2012-03-30 19:55 ` Alex Elder
0 siblings, 0 replies; 12+ messages in thread
From: Alex Elder @ 2012-03-30 19:55 UTC (permalink / raw)
To: Sage Weil; +Cc: ceph-devel
On 03/16/2012 01:10 AM, Sage Weil wrote:
> On Mon, 12 Mar 2012, Alex Elder wrote:
>
>> The helper macros for accessing file layout fields of an on-disk
>> ceph file layout structure cast their results to type (__s32). This
>> is a bit strange, since (with one exception--fl_pg_preferred):
>> - there is no need for negative values; and
>> - all users of these macros are assigning their result to
>> 64-bit variables.
>> So just make these macros return a 64-bit unsigned type.
>>
>> The exception is the preferred placement group, which remains a
>> signed 32-bit value. A placement group id encodes the preferred
>> primary OSD in a 16-bit value, and there's no sense at this point
>> getting too far away from that.
>>
>> Signed-off-by: Alex Elder<elder@dreamhost.com>
. . .
>> diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
>> index 26c30e7..bc2d807 100644
>> --- a/net/ceph/osdmap.c
>> +++ b/net/ceph/osdmap.c
. . .
>> @@ -1028,29 +1028,30 @@ int ceph_calc_object_layout(struct ceph_object_layout
. . .
>> - ps = ceph_str_hash(pool->v.object_hash, oid, strlen(oid));
>>
>> - if (preferred == CEPH_FILE_LAYOUT_PG_PREFERRED_NONE) {
>> - num = le32_to_cpu(pool->v.pg_num);
>> - num_mask = pool->pg_num_mask;
>> - } else {
>> + ps = ceph_str_hash(pool->v.object_hash, oid, strlen(oid));
>> + if (preferred != CEPH_FILE_LAYOUT_PG_PREFERRED_NONE) {
>> + BUG_ON(preferred< 0);
>> ps += preferred;
>> - num = le32_to_cpu(pool->v.lpg_num);
>> - num_mask = pool->lpg_num_mask;
>> }
>>
>> + num = le32_to_cpu(pool->v.lpg_num);
>> + num_mask = pool->lpg_num_mask;
>
> Unless i'm misreading this diff, you're switching to always using lpg_num
> and lpg_num_mask instead of pg_num and pg_num_mask. That "l" prefix means
> "localized," for a different set of localized PGs used when htere is a
> preferred OSD. We need to keep these assignments in the if and else
> blocks...
No, you read it right and I got it wrong. I misread the code
and thought they were operating with the same fields. I'll fix
it.
-Alex
>> +
>> pgid.ps = cpu_to_le16(ps);
>> - pgid.preferred = cpu_to_le16(preferred);
>> + pgid.preferred = cpu_to_le16((s16) preferred);
>> pgid.pool = fl->fl_pg_pool;
>> if (preferred == CEPH_FILE_LAYOUT_PG_PREFERRED_NONE)
>> dout("calc_object_layout '%s' pgid %d.%x\n", oid, poolid, ps);
. . .
^ permalink raw reply [flat|nested] 12+ messages in thread