ceph: ceph file layout helper fixes

PATCH In review Alex Elder 2 participants 12 messages Mar 12, 2012 – Mar 30, 2012

Subsystems (4)

Block layer Ceph client Ceph common code (libceph) RADOS block device (RBD)

lore

Share & tools

lore

Alex Elder

author

2012-03-12 22:50 UTC #0 lore raw

cover ceph: ceph file layout helper fixes

This series makes a set of changes to the file layout helper macros used in Ceph. First, their definitions are moved into a more appropriate source file. Then they are changed to take a pointer rather than a struct argument. Next, changes are made so they're always used for accessing a Ceph inode's file layout information, and a symbolic value is used to represent "no preferred placement group." And finally, some changes ensure close attention is paid to the types of these values and what they get assigned to, and for most of them they're changed to use 64-bit unsigned values.

-Alex

Alex Elder

author

2012-03-12 22:57 UTC #1 lore raw

PATCH 1/5 ceph: move file layout helpers to ceph_fs.h

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 <[email protected]>

---
 include/linux/ceph/ceph_fs.h |   29 +++++++++++++++++++++++++++++
 include/linux/ceph/osdmap.h  |   29 -----------------------------
 2 files changed, 29 insertions(+), 29 deletions(-)

include/linux/ceph/ceph_fs.h +29 -0

index b8c6069..62e3bb3 100644
@@ -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);

include/linux/ceph/osdmap.h +0 ~~-29~~

index ba4c205..fb588b7 100644
@@ -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);

Signature · 1 line

1.7.5.4

Alex Elder

author

2012-03-12 22:57 UTC #2 lore raw

PATCH 2/5 ceph: make ceph file layout helpers take a pointer

In reply to Alex Elder

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 <[email protected]>

---
 fs/ceph/ioctl.c              |   16 ++++++++--------
 fs/ceph/xattr.c              |   10 +++++-----
 include/linux/ceph/ceph_fs.h |   28 +++++++++++++++-------------
 3 files changed, 28 insertions(+), 26 deletions(-)

fs/ceph/ioctl.c +8 -8

index 790914a59..23cb78b 100644
@@ -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;

fs/ceph/xattr.c +5 -5

index 715cfc6..75960b1 100644
@@ -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;

include/linux/ceph/ceph_fs.h +11 -8

index 62e3bb3..2645704 100644
@@ -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 {

Signature · 1 line

1.7.5.4

Alex Elder

author

2012-03-12 22:57 UTC #3 lore raw

PATCH 3/5 ceph: make use of ceph file layout helpers

In reply to Alex Elder

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(-)

drivers/block/rbd.c +1 -1

index 6bbd5af..e9521ed 100644
@@ -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);

fs/ceph/ioctl.c +12 ~~-12~~

index 23cb78b..75cff03 100644
@@ -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;

fs/ceph/xattr.c +3 -2

index 75960b1..bfd5b93 100644
@@ -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",

include/linux/ceph/ceph_fs.h +2 -1

index 2645704..cf86032 100644
@@ -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) \

net/ceph/ceph_fs.c +3 -3

index 41466cc..c3197b9 100644
@@ -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)))

net/ceph/osdmap.c +13 ~~-12~~

index 29ad46e..f5f6e41 100644
@@ -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;

Signature · 1 line

1.7.5.4

Sage Weil

2012-03-16 05:59 UTC #4 lore raw

Re: [PATCH 3/5] ceph: make use of ceph file layout helpers

Quoted 109 lines · Alex Elder

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(-)

drivers/block/rbd.c +1 -1

index 6bbd5af..e9521ed 100644
@@ -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);

fs/ceph/ioctl.c +9 -9

index 23cb78b..75cff03 100644
@@ -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);

fs/ceph/ioctl.c +0 -1

 		nl.preferred_osd =
-

(s32)le32_to_cpu(ci->i_layout.fl_pg_preferred);

fs/ceph/ioctl.c +1 -0

+				(__s64)

ceph_file_layout_pg_preferred(&ci->i_layout);

fs/ceph/ioctl.c +2 -2

 	} 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);

fs/ceph/ioctl.c +0 -0

 	/* block_offset = object_offset % block_size */
 	tmp = dl.object_offset;

fs/ceph/xattr.c +0 -1

index 75960b1..bfd5b93 100644
@@ -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),

fs/ceph/xattr.c static size_t ceph_vxattrcb_file_layout(struct · line 123

Quoted 10 lines

fs/ceph/xattr.c +3 -1

+		(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

Quoted 65 lines

fs/ceph/xattr.c +0 -0

 		val += ret;
 		size -= ret;
 		ret += snprintf(val, size, "preferred_osd=%lld\n",

include/linux/ceph/ceph_fs.h +2 -1

index 2645704..cf86032 100644
@@ -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) \

net/ceph/ceph_fs.c +3 -3

index 41466cc..c3197b9 100644
@@ -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)))

net/ceph/osdmap.c +3 -3

index 29ad46e..f5f6e41 100644
@@ -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;

net/ceph/osdmap.c +2 -2

 	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;

net/ceph/osdmap.c int ceph_calc_object_layout(struct ceph_object_layout · line 1016

Quoted 7 lines

net/ceph/osdmap.c +2 -1

@@ -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 >= ...

Quoted 6 lines

net/ceph/osdmap.c +3 -0

+		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;

net/ceph/osdmap.c int ceph_calc_object_layout(struct ceph_object_layout *ol, · line 1027

Quoted 10 lines

net/ceph/osdmap.c +1 -4

-	} 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.

Quoted 19 lines

net/ceph/osdmap.c +2 -2

+		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);

net/ceph/osdmap.c +0 -0

 	ol->ol_pgid = pgid;
 	ol->ol_stripe_unit = fl->fl_object_stripe_unit;

Signature · 6 lines

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

Alex Elder

author

2012-03-30 19:55 UTC #5 lore raw

Re: [PATCH 3/5] ceph: make use of ceph file layout helpers

Quoted 5 lines · Sage Weil

Quoted 4 lines · Alex Elder
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.

Quoted 8 lines

Quoted 8 lines
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]>

A few comments below. I'm about to re-post this series.

. . .

Quoted 17 lines

fs/ceph/xattr.c +1 -1

index 75960b1..bfd5b93 100644
@@ -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));

Quoted 5 lines

fs/ceph/xattr.c static size_t ceph_vxattrcb_file_layout(struct ceph_inode_info *ci, char *val, · line 123
Quoted 3 lines
fs/ceph/xattr.c +2 -1
-	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).

Quoted 7 lines

include/linux/ceph/ceph_fs.h
Quoted 7 lines
  		val += ret;
  		size -= ret;
  		ret += snprintf(val, size, "preferred_osd=%lld\n",
include/linux/ceph/ceph_fs.h +0 -0
index 2645704..cf86032 100644

. . .

Quoted 10 lines

include/linux/ceph/ceph_fs.h int ceph_calc_object_layout(struct ceph_object_layout · line 1016
Quoted 8 lines
include/linux/ceph/ceph_fs.h +2 -1
@@ -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.

Quoted 8 lines

include/linux/ceph/ceph_fs.h int ceph_calc_object_layout(struct ceph_object_layout · line 1019
Quoted 8 lines
include/linux/ceph/ceph_fs.h +3 -1
+		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 {

. . .

Alex Elder

author

2012-03-12 22:57 UTC #6 lore raw

PATCH 4/5 ceph: use 64-bit math in ceph_calc_file_object_mapping()

In reply to Alex Elder

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 <[email protected]>

---
 net/ceph/osdmap.c |  112 
+++++++++++++++++++++++++++++++++--------------------
 1 files changed, 70 insertions(+), 42 deletions(-)

net/ceph/osdmap.c +70 ~~-42~~

index f5f6e41..26c30e7 100644
@@ -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);

Signature · 1 line

1.7.5.4

Sage Weil

2012-03-16 06:17 UTC #7 lore raw

Re: [PATCH 4/5] ceph: use 64-bit math in ceph_calc_file_object_mapping()

Quoted 8 lines · Alex Elder

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

Quoted 152 lines

Signed-off-by Alex Elder <[email protected]>

---
 net/ceph/osdmap.c |  112
+++++++++++++++++++++++++++++++++--------------------
 1 files changed, 70 insertions(+), 42 deletions(-)

net/ceph/osdmap.c +70 ~~-42~~

index f5f6e41..26c30e7 100644
@@ -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);

Signature · 6 lines

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

Alex Elder

author

2012-03-16 13:25 UTC #8 lore raw

Re: [PATCH 4/5] ceph: use 64-bit math in ceph_calc_file_object_mapping()

Quoted 21 lines · Sage Weil

Quoted 8 lines · Alex Elder
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).

Quoted 6 lines

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

Quoted 155 lines

sage

Quoted 152 lines

Signed-off-by Alex Elder<[email protected]>

---
  net/ceph/osdmap.c |  112
+++++++++++++++++++++++++++++++++--------------------
  1 files changed, 70 insertions(+), 42 deletions(-)

net/ceph/osdmap.c +70 ~~-42~~

index f5f6e41..26c30e7 100644
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Alex Elder

author

2012-03-12 22:57 UTC #9 lore raw

PATCH 5/5 ceph: fix up the types of the file layout helpers

In reply to Alex Elder

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 <[email protected]>

---
 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(-)

fs/ceph/inode.c +5 -1

index d291928..1828fb9 100644
@@ -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. */

fs/ceph/ioctl.c +11 ~~-11~~

index 75cff03..2fde342 100644
@@ -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;

fs/ceph/xattr.c +8 -8

index bfd5b93..0652aa4 100644
@@ -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;

include/linux/ceph/ceph_fs.h +12 ~~-14~~

index cf86032..698d395 100644
@@ -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);

net/ceph/ceph_fs.c +3 -3

index c3197b9..97f7c50 100644
@@ -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)))

net/ceph/osdmap.c +13 ~~-12~~

index 26c30e7..bc2d807 100644
@@ -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);

Signature · 1 line

1.7.5.4

Sage Weil

2012-03-16 06:10 UTC #10 lore raw

Re: [PATCH 5/5] ceph: fix up the types of the file layout helpers

Quoted 257 lines · Alex Elder

Signed-off-by Alex Elder <[email protected]>

---
 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(-)

fs/ceph/inode.c +5 -1

index d291928..1828fb9 100644
@@ -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.

fs/ceph/ioctl.c +6 -8

index 75cff03..2fde342 100644
@@ -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);

fs/ceph/ioctl.c +2 -1

+		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);

fs/ceph/ioctl.c +3 -2

+			(__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);

fs/ceph/ioctl.c +0 -0

 	/* block_offset = object_offset % block_size */
 	tmp = dl.object_offset;

fs/ceph/xattr.c +8 -8

index bfd5b93..0652aa4 100644
@@ -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;

include/linux/ceph/ceph_fs.h +12 ~~-14~~

index cf86032..698d395 100644
@@ -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);

net/ceph/ceph_fs.c +3 -3

index c3197b9..97f7c50 100644
@@ -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)))

net/ceph/osdmap.c +3 -3

index 26c30e7..bc2d807 100644
@@ -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;

net/ceph/osdmap.c +3 -6

 	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 {

net/ceph/osdmap.c int ceph_calc_object_layout(struct ceph_object_layout · line 1052

Quoted 10 lines

net/ceph/osdmap.c +5 -2

+	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...

Quoted 16 lines

net/ceph/osdmap.c +2 -1

+
 	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);

Signature · 6 lines

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

Alex Elder

author

2012-03-30 19:55 UTC #11 lore raw

Re: [PATCH 5/5] ceph: fix up the types of the file layout helpers

Quoted 10 lines · Sage Weil

Quoted 8 lines · Alex Elder
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.

Quoted 6 lines

Quoted 6 lines
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<[email protected]>

. . .

Quoted 4 lines

net/ceph/osdmap.c
Quoted 4 lines
net/ceph/osdmap.c +0 -0
index 26c30e7..bc2d807 100644

. . .

Quoted 1 line

net/ceph/osdmap.c int ceph_calc_object_layout(struct ceph_object_layout
Quoted 1 line
net/ceph/osdmap.c +0 -0
@@ -1028,29 +1028,30 @@ int ceph_calc_object_layout(struct ceph_object_layout

. . .

Quoted 14 lines

net/ceph/osdmap.c +3 -7

-	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;
  	}

Quoted 8 lines

net/ceph/osdmap.c int ceph_calc_object_layout(struct ceph_object_layout *ol, · line 1051
Quoted 2 lines
net/ceph/osdmap.c +2 -0
+	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

Quoted 7 lines

net/ceph/osdmap.c int ceph_calc_object_layout(struct ceph_object_layout *ol, · line 1057

Quoted 7 lines

net/ceph/osdmap.c +2 -1

+
  	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);

. . .