Hi Andre,
On Thu, May 14, 2009 at 4:13 PM, Andre Noll <maan@xxxxxxxxxxxxxxx> wrote:
> The number of strip_zones of a raid0 array is bounded by the number of
> drives in the array and is in fact much smaller for typical setups. For
> example, any raid0 array containing identical disks will have only
> a single strip_zone.
>
> Therefore, the hash tables which are used for quickly finding the
> strip_zone that holds a particular sector are of questionable value
> and add quite a bit of unnecessary complexity.
>
> This patch replaces the hash table lookup by equivalent code which
> simply loops over all strip zones to find the zone that holds the
> given sector.
>
> Subsequent cleanup patches will remove the hash table structure.
>
> Signed-off-by: Andre Noll <maan@xxxxxxxxxxxxxxx>
> ---
> drivers/md/raid0.c | 32 +++++++++++++++++++-------------
> 1 files changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
> index c08d755..9fd3c3c 100644
> --- a/drivers/md/raid0.c
> +++ b/drivers/md/raid0.c
> @@ -398,6 +398,22 @@ static int raid0_stop (mddev_t *mddev)
> return 0;
> }
>
> +/* Find the zone which holds a particular offset */
> +static struct strip_zone *find_zone(struct raid0_private_data *conf,
> + sector_t sector)
> +{
> + int i;
> +
> + for (i = 0; i < conf->nr_strip_zones; i++) {
> + struct strip_zone *z = conf->strip_zone + i;
> +
> + if (sector < z->zone_start + z->sectors)
> + return z;
> + }
> + BUG();
> + return NULL;
> +}
> +
> static int raid0_make_request (struct request_queue *q, struct bio *bio)
> {
> mddev_t *mddev = q->queuedata;
> @@ -443,20 +459,10 @@ static int raid0_make_request (struct request_queue *q, struct bio *bio)
> bio_pair_release(bp);
> return 0;
> }
> -
> -
> - {
> - sector_t x = sector >> conf->sector_shift;
> - sector_div(x, (u32)conf->spacing);
> - zone = conf->hash_table[x];
> - }
> -
> - while (sector >= zone->zone_start + zone->sectors)
> - zone++;
> -
> + zone = find_zone(conf, sector);
> + if (!zone)
> + return 1;
Firstly, will find_zone ever return a NULL. You code already asserts
on that situation in find_zone.
But in my opinion,
You should allow NULL to be returned from find_zone( ),
handle NULL in the caller function more gracefully than panicking (BUG).
Also, the above lines mean that if zone==NULL, you return 1, which
will eventually submit your request through generic_make_request.
I might be wrong as I am quite new to this code.
Kindly forgive.
> sect_in_chunk = bio->bi_sector & (chunk_sects - 1);
> -
> -
> {
> sector_t x = (sector - zone->zone_start) >> chunksect_bits;
>
> --
> 1.5.4.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Regards,
Sandeep.
“To learn is to change. Education is a process that changes the learner.”
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
