diff mbox

[16/33] libceph: introduce decode{,_new}_pg_temp() and switch to them

Message ID 1395944299-21970-17-git-send-email-ilya.dryomov@inktank.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ilya Dryomov March 27, 2014, 6:18 p.m. UTC 
Consolidate pg_temp (full map, map<pg_t, vector<u32>>) and new_pg_temp
(inc map, same) decoding logic into a common helper and switch to it.

Signed-off-by: Ilya Dryomov <ilya.dryomov@inktank.com>
---
 net/ceph/osdmap.c |  139 ++++++++++++++++++++++++++---------------------------
 1 file changed, 67 insertions(+), 72 deletions(-)

Comments

Alex Elder March 27, 2014, 8:05 p.m. UTC | #1 
On 03/27/2014 01:18 PM, Ilya Dryomov wrote:
> Consolidate pg_temp (full map, map<pg_t, vector<u32>>) and new_pg_temp
> (inc map, same) decoding logic into a common helper and switch to it.

Again, it's nice to see this kind of refactoring being done.

Looks good.

Reviewed-by: Alex Elder <elder@linaro.org>


> Signed-off-by: Ilya Dryomov <ilya.dryomov@inktank.com>
> ---
>  net/ceph/osdmap.c |  139 ++++++++++++++++++++++++++---------------------------
>  1 file changed, 67 insertions(+), 72 deletions(-)
> 
> diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
> index 4565c72fec5c..0134df3639d2 100644
> --- a/net/ceph/osdmap.c
> +++ b/net/ceph/osdmap.c
> @@ -732,6 +732,67 @@ static int decode_new_pools(void **p, void *end, struct ceph_osdmap *map)
>  	return __decode_pools(p, end, map, true);
>  }
>  
> +static int __decode_pg_temp(void **p, void *end, struct ceph_osdmap *map,
> +			    bool incremental)
> +{
> +	u32 n;
> +
> +	ceph_decode_32_safe(p, end, n, e_inval);
> +	while (n--) {
> +		struct ceph_pg pgid;
> +		u32 len, i;
> +		int ret;
> +
> +		ret = ceph_decode_pgid(p, end, &pgid);
> +		if (ret)
> +			return ret;
> +
> +		ceph_decode_32_safe(p, end, len, e_inval);
> +
> +		ret = __remove_pg_mapping(&map->pg_temp, pgid);
> +		BUG_ON(!incremental && ret != -ENOENT);
> +
> +		if (!incremental || len > 0) {
> +			struct ceph_pg_mapping *pg;
> +
> +			ceph_decode_need(p, end, len*sizeof(u32), e_inval);
> +
> +			if (len > (UINT_MAX - sizeof(*pg)) / sizeof(u32))
> +				return -EINVAL;
> +
> +			pg = kzalloc(sizeof(*pg) + len*sizeof(u32), GFP_NOFS);
> +			if (!pg)
> +				return -ENOMEM;
> +
> +			pg->pgid = pgid;
> +			pg->len = len;
> +			for (i = 0; i < len; i++)
> +				pg->osds[i] = ceph_decode_32(p);
> +
> +			ret = __insert_pg_mapping(pg, &map->pg_temp);
> +			if (ret) {
> +				kfree(pg);
> +				return ret;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +
> +e_inval:
> +	return -EINVAL;
> +}
> +
> +static int decode_pg_temp(void **p, void *end, struct ceph_osdmap *map)
> +{
> +	return __decode_pg_temp(p, end, map, false);
> +}
> +
> +static int decode_new_pg_temp(void **p, void *end, struct ceph_osdmap *map)
> +{
> +	return __decode_pg_temp(p, end, map, true);
> +}
> +
>  /*
>   * decode a full map.
>   */
> @@ -804,36 +865,9 @@ static int osdmap_decode(void **p, void *end, struct ceph_osdmap *map)
>  		ceph_decode_addr(&map->osd_addr[i]);
>  
>  	/* pg_temp */
> -	ceph_decode_32_safe(p, end, len, e_inval);
> -	for (i = 0; i < len; i++) {
> -		int n, j;
> -		struct ceph_pg pgid;
> -		struct ceph_pg_mapping *pg;
> -
> -		err = ceph_decode_pgid(p, end, &pgid);
> -		if (err)
> -			goto bad;
> -		ceph_decode_need(p, end, sizeof(u32), e_inval);
> -		n = ceph_decode_32(p);
> -		if (n > (UINT_MAX - sizeof(*pg)) / sizeof(u32))
> -			goto e_inval;
> -		ceph_decode_need(p, end, n * sizeof(u32), e_inval);
> -		pg = kmalloc(sizeof(*pg) + n*sizeof(u32), GFP_NOFS);
> -		if (!pg) {
> -			err = -ENOMEM;
> -			goto bad;
> -		}
> -		pg->pgid = pgid;
> -		pg->len = n;
> -		for (j = 0; j < n; j++)
> -			pg->osds[j] = ceph_decode_32(p);
> -
> -		err = __insert_pg_mapping(pg, &map->pg_temp);
> -		if (err)
> -			goto bad;
> -		dout(" added pg_temp %lld.%x len %d\n", pgid.pool, pgid.seed,
> -		     len);
> -	}
> +	err = decode_pg_temp(p, end, map);
> +	if (err)
> +		goto bad;
>  
>  	/* crush */
>  	ceph_decode_32_safe(p, end, len, e_inval);
> @@ -1032,48 +1066,9 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end,
>  	}
>  
>  	/* new_pg_temp */
> -	ceph_decode_32_safe(p, end, len, e_inval);
> -	while (len--) {
> -		struct ceph_pg_mapping *pg;
> -		int j;
> -		struct ceph_pg pgid;
> -		u32 pglen;
> -
> -		err = ceph_decode_pgid(p, end, &pgid);
> -		if (err)
> -			goto bad;
> -		ceph_decode_need(p, end, sizeof(u32), e_inval);
> -		pglen = ceph_decode_32(p);
> -		if (pglen) {
> -			ceph_decode_need(p, end, pglen*sizeof(u32), e_inval);
> -
> -			/* removing existing (if any) */
> -			(void) __remove_pg_mapping(&map->pg_temp, pgid);
> -
> -			/* insert */
> -			if (pglen > (UINT_MAX - sizeof(*pg)) / sizeof(u32))
> -				goto e_inval;
> -			pg = kmalloc(sizeof(*pg) + sizeof(u32)*pglen, GFP_NOFS);
> -			if (!pg) {
> -				err = -ENOMEM;
> -				goto bad;
> -			}
> -			pg->pgid = pgid;
> -			pg->len = pglen;
> -			for (j = 0; j < pglen; j++)
> -				pg->osds[j] = ceph_decode_32(p);
> -			err = __insert_pg_mapping(pg, &map->pg_temp);
> -			if (err) {
> -				kfree(pg);
> -				goto bad;
> -			}
> -			dout(" added pg_temp %lld.%x len %d\n", pgid.pool,
> -			     pgid.seed, pglen);
> -		} else {
> -			/* remove */
> -			__remove_pg_mapping(&map->pg_temp, pgid);
> -		}
> -	}
> +	err = decode_new_pg_temp(p, end, map);
> +	if (err)
> +		goto bad;
>  
>  	/* ignore the rest */
>  	*p = end;
> 

--
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
diff mbox

Patch

diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
index 4565c72fec5c..0134df3639d2 100644
--- a/net/ceph/osdmap.c
+++ b/net/ceph/osdmap.c
@@ -732,6 +732,67 @@  static int decode_new_pools(void **p, void *end, struct ceph_osdmap *map)
 	return __decode_pools(p, end, map, true);
 }
 
+static int __decode_pg_temp(void **p, void *end, struct ceph_osdmap *map,
+			    bool incremental)
+{
+	u32 n;
+
+	ceph_decode_32_safe(p, end, n, e_inval);
+	while (n--) {
+		struct ceph_pg pgid;
+		u32 len, i;
+		int ret;
+
+		ret = ceph_decode_pgid(p, end, &pgid);
+		if (ret)
+			return ret;
+
+		ceph_decode_32_safe(p, end, len, e_inval);
+
+		ret = __remove_pg_mapping(&map->pg_temp, pgid);
+		BUG_ON(!incremental && ret != -ENOENT);
+
+		if (!incremental || len > 0) {
+			struct ceph_pg_mapping *pg;
+
+			ceph_decode_need(p, end, len*sizeof(u32), e_inval);
+
+			if (len > (UINT_MAX - sizeof(*pg)) / sizeof(u32))
+				return -EINVAL;
+
+			pg = kzalloc(sizeof(*pg) + len*sizeof(u32), GFP_NOFS);
+			if (!pg)
+				return -ENOMEM;
+
+			pg->pgid = pgid;
+			pg->len = len;
+			for (i = 0; i < len; i++)
+				pg->osds[i] = ceph_decode_32(p);
+
+			ret = __insert_pg_mapping(pg, &map->pg_temp);
+			if (ret) {
+				kfree(pg);
+				return ret;
+			}
+		}
+	}
+
+	return 0;
+
+e_inval:
+	return -EINVAL;
+}
+
+static int decode_pg_temp(void **p, void *end, struct ceph_osdmap *map)
+{
+	return __decode_pg_temp(p, end, map, false);
+}
+
+static int decode_new_pg_temp(void **p, void *end, struct ceph_osdmap *map)
+{
+	return __decode_pg_temp(p, end, map, true);
+}
+
 /*
  * decode a full map.
  */
@@ -804,36 +865,9 @@  static int osdmap_decode(void **p, void *end, struct ceph_osdmap *map)
 		ceph_decode_addr(&map->osd_addr[i]);
 
 	/* pg_temp */
-	ceph_decode_32_safe(p, end, len, e_inval);
-	for (i = 0; i < len; i++) {
-		int n, j;
-		struct ceph_pg pgid;
-		struct ceph_pg_mapping *pg;
-
-		err = ceph_decode_pgid(p, end, &pgid);
-		if (err)
-			goto bad;
-		ceph_decode_need(p, end, sizeof(u32), e_inval);
-		n = ceph_decode_32(p);
-		if (n > (UINT_MAX - sizeof(*pg)) / sizeof(u32))
-			goto e_inval;
-		ceph_decode_need(p, end, n * sizeof(u32), e_inval);
-		pg = kmalloc(sizeof(*pg) + n*sizeof(u32), GFP_NOFS);
-		if (!pg) {
-			err = -ENOMEM;
-			goto bad;
-		}
-		pg->pgid = pgid;
-		pg->len = n;
-		for (j = 0; j < n; j++)
-			pg->osds[j] = ceph_decode_32(p);
-
-		err = __insert_pg_mapping(pg, &map->pg_temp);
-		if (err)
-			goto bad;
-		dout(" added pg_temp %lld.%x len %d\n", pgid.pool, pgid.seed,
-		     len);
-	}
+	err = decode_pg_temp(p, end, map);
+	if (err)
+		goto bad;
 
 	/* crush */
 	ceph_decode_32_safe(p, end, len, e_inval);
@@ -1032,48 +1066,9 @@  struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end,
 	}
 
 	/* new_pg_temp */
-	ceph_decode_32_safe(p, end, len, e_inval);
-	while (len--) {
-		struct ceph_pg_mapping *pg;
-		int j;
-		struct ceph_pg pgid;
-		u32 pglen;
-
-		err = ceph_decode_pgid(p, end, &pgid);
-		if (err)
-			goto bad;
-		ceph_decode_need(p, end, sizeof(u32), e_inval);
-		pglen = ceph_decode_32(p);
-		if (pglen) {
-			ceph_decode_need(p, end, pglen*sizeof(u32), e_inval);
-
-			/* removing existing (if any) */
-			(void) __remove_pg_mapping(&map->pg_temp, pgid);
-
-			/* insert */
-			if (pglen > (UINT_MAX - sizeof(*pg)) / sizeof(u32))
-				goto e_inval;
-			pg = kmalloc(sizeof(*pg) + sizeof(u32)*pglen, GFP_NOFS);
-			if (!pg) {
-				err = -ENOMEM;
-				goto bad;
-			}
-			pg->pgid = pgid;
-			pg->len = pglen;
-			for (j = 0; j < pglen; j++)
-				pg->osds[j] = ceph_decode_32(p);
-			err = __insert_pg_mapping(pg, &map->pg_temp);
-			if (err) {
-				kfree(pg);
-				goto bad;
-			}
-			dout(" added pg_temp %lld.%x len %d\n", pgid.pool,
-			     pgid.seed, pglen);
-		} else {
-			/* remove */
-			__remove_pg_mapping(&map->pg_temp, pgid);
-		}
-	}
+	err = decode_new_pg_temp(p, end, map);
+	if (err)
+		goto bad;
 
 	/* ignore the rest */
 	*p = end;