Optimize sector division

Optimize sector division --- if the number of stripes is a power of two,
we don't have to do expensive division and can do shift and mask instead.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
drivers/md/dm-stripe.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

Index: linux-2.6.35-rc6-fast/drivers/md/dm-stripe.c
================================================== =================
--- linux-2.6.35-rc6-fast.orig/drivers/md/dm-stripe.c 2010-07-28 00:01:04.000000000 +0200
+++ linux-2.6.35-rc6-fast/drivers/md/dm-stripe.c 2010-07-28 00:06:19.000000000 +0200
@@ -25,6 +25,7 @@ struct stripe {

struct stripe_c {
uint32_t stripes;
+ int stripes_shift;

/* The size of this target / num. stripes */
sector_t stripe_width;
@@ -164,6 +165,10 @@ static int stripe_ctr(struct dm_target *
sc->ti = ti;

sc->stripes = stripes;
+ if (!(stripes & (stripes - 1)))
+ sc->stripes_shift = ffs(stripes) - 1;
+ else
+ sc->stripes_shift = -1;
sc->stripe_width = width;
ti->split_io = chunk_size;
ti->num_flush_requests = stripes;
@@ -212,7 +217,11 @@ static void stripe_map_sector(struct str
{
sector_t offset = sector - sc->ti->begin;
sector_t chunk = offset >> sc->chunk_shift;
- *stripe = sector_div(chunk, sc->stripes);
+ if (likely(sc->stripes_shift >= 0))
+ *stripe = chunk & ((1 << sc->stripes_shift) - 1),
+ chunk >>= sc->stripes_shift;
+ else
+ *stripe = sector_div(chunk, sc->stripes);
*result = (chunk << sc->chunk_shift) | (offset & sc->chunk_mask);
}


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

On Tue, Jul 27 2010 at 6:12pm -0400,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> Optimize sector division
>
> Optimize sector division --- if the number of stripes is a power of two,
> we don't have to do expensive division and can do shift and mask instead.
>
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

(Tested and)

Reviewed-by: Mike Snitzer <snitzer@redhat.com>

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Looking closer I'm seeing something odd:

On Tue, Jul 27 2010 at 6:12pm -0400,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> @@ -212,7 +217,11 @@ static void stripe_map_sector(struct str
> {
> sector_t offset = sector - sc->ti->begin;
> sector_t chunk = offset >> sc->chunk_shift;
> - *stripe = sector_div(chunk, sc->stripes);
> + if (likely(sc->stripes_shift >= 0))
> + *stripe = chunk & ((1 << sc->stripes_shift) - 1),
> + chunk >>= sc->stripes_shift;

What's going on here with the comma? Shouldn't it be a semi-colon? And
if so we need curly-braces around the if (likely...).

> + else
> + *stripe = sector_div(chunk, sc->stripes);
> *result = (chunk << sc->chunk_shift) | (offset & sc->chunk_mask);
> }

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

On Wed, 28 Jul 2010, Mike Snitzer wrote:

> Looking closer I'm seeing something odd:
>
> On Tue, Jul 27 2010 at 6:12pm -0400,
> Mikulas Patocka <mpatocka@redhat.com> wrote:
>
> > @@ -212,7 +217,11 @@ static void stripe_map_sector(struct str
> > {
> > sector_t offset = sector - sc->ti->begin;
> > sector_t chunk = offset >> sc->chunk_shift;
> > - *stripe = sector_div(chunk, sc->stripes);
> > + if (likely(sc->stripes_shift >= 0))
> > + *stripe = chunk & ((1 << sc->stripes_shift) - 1),
> > + chunk >>= sc->stripes_shift;
>
> What's going on here with the comma? Shouldn't it be a semi-colon? And
> if so we need curly-braces around the if (likely...).

I'ts just a syntactic trick to avoid the braces. If you think it is weird,
you can change it to:
if (likely(sc->stripes_shift >= 0)) {
*stripe = chunk & ((1 << sc->stripes_shift) - 1);
chunk >>= sc->stripes_shift;
} else

The comma operator in C (if not used to denote function arguments) means
"evaluate the expression before the comma and discard the result, then
evaluate the expression after the comma and return the result".

Comma has the lowest priority of all operators. Even lower than "=".

So "a = (1, 2, 3);" means "a = 3;"
"a = 1, 2, 3" is parsed as "(a = 1), 2, 3" and it assigns 1 to a, discards
2 and returns 3 (if used as part of another expression; if used
standalone, it discards that 3).

"if (c) a = 1, b = 2;" assigns 1 to a and 2 to b if c is non-zero. It is
equivalent to "if (c) { a = 1; b = 2; }"

Another common use for comma is to stuff arbitrary function calls into the
expression --- i.e. for example
#define read_register(x) (printk("reading register %x
", x), inb(BASE + (x)))
which could then be used as
int r3 = read_register(3);

Mikulas

> > + else
> > + *stripe = sector_div(chunk, sc->stripes);
> > *result = (chunk << sc->chunk_shift) | (offset & sc->chunk_mask);
> > }
>

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel