2021-02-10 02:10:57

by Paul Gortmaker

Subject: [PATCH 5/8] lib: bitmap: pair nbits value with region struct

A region is a standalone entity to some degree, but it needs to
be paired with a bitmap width in order to set context and determine
if the region even fits into the width of the bitmap.

This will reduce parameter passing and enable using nbits as part
of future dynamic region parameter parsing.

Cc: Yury Norov <[email protected]>
Cc: Rasmus Villemoes <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Suggested-by: Yury Norov <[email protected]>
Suggested-by: Andy Shevchenko <[email protected]>
Signed-off-by: Paul Gortmaker <[email protected]>
---
lib/bitmap.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/lib/bitmap.c b/lib/bitmap.c
index 9596ba53c36b..6b568f98af3d 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -499,6 +499,16 @@ struct region {
unsigned int end;
};

+/*
+ * The region "0-3" is a complete specification, i.e. "the 1st four cores"
+ * for a CPU map, but it needs to be paired to a width in order to have a
+ * meaningful and valid context. (i.e. 4 core region on 4+ core machine...)
+ */
+struct bitmap_region {
+ struct region *r;
+ unsigned int nbits;
+};
+
static void bitmap_set_region(const struct region *r, unsigned long *bitmap)
{
unsigned int start;
@@ -507,12 +517,14 @@ static void bitmap_set_region(const struct region *r, unsigned long *bitmap)
bitmap_set(bitmap, start, min(r->end - start + 1, r->off));
}

-static int bitmap_check_region(const struct region *r, int nbits)
+static int bitmap_check_region(const struct bitmap_region *br)
{
+ struct region *r = br->r;
+
if (r->start > r->end || r->group_len == 0 || r->off > r->group_len)
return -EINVAL;

- if (r->end >= nbits)
+ if (r->end >= br->nbits)
return -ERANGE;

return 0;
@@ -635,8 +647,12 @@ static const char *bitmap_parse_region(const char *str, struct region *r)
int bitmap_parselist(const char *buf, unsigned long *maskp, int nmaskbits)
{
struct region r;
+ struct bitmap_region br;
long ret;

+ br.r = &r;
+ br.nbits = nmaskbits;
+
bitmap_zero(maskp, nmaskbits);

while (buf) {
@@ -648,7 +664,7 @@ int bitmap_parselist(const char *buf, unsigned long *maskp, int nmaskbits)
if (IS_ERR(buf))
return PTR_ERR(buf);

- ret = bitmap_check_region(&r, nmaskbits);
+ ret = bitmap_check_region(&br);
if (ret)
return ret;

--
2.17.1

2021-02-10 16:41:36

by Andy Shevchenko

Subject: Re: [PATCH 5/8] lib: bitmap: pair nbits value with region struct

On Tue, Feb 09, 2021 at 05:59:04PM -0500, Paul Gortmaker wrote:
> A region is a standalone entity to some degree, but it needs to
> be paired with a bitmap width in order to set context and determine
> if the region even fits into the width of the bitmap.
>
> This will reduce parameter passing and enable using nbits as part
> of future dynamic region parameter parsing.

...

> +struct bitmap_region {
> + struct region *r;

Why do we need it as a pointer?

> + unsigned int nbits;
> +};

...

> struct region r;
> + struct bitmap_region br;

> + br.r = &r;
> + br.nbits = nmaskbits;

I thought about simply

struct bitmap_region br;

br.nbits = nmaskbits;

--
With Best Regards,
Andy Shevchenko