Skip to contentSkip to conversation

Introduction

PATCH New NeilBrown 1 participant 3 messages Nov 16, 2004 MD RAID

lore
Share & tools
lore

NeilBrown

author
#0 lore raw

PATCH 1/1 Introduction

Following are two patches for md/linear.c in 2.6.10-rc1-mm5

The first fixes a bug that was recently introduced, that affects linear arrays where the components aren't all the same size (I didn't test that case :-( ). I would be good if this was in 2.6.10.

The second is a code cleanup and minor reduction in memory usage. There is no particular hurry for this patch to go in.

NeilBrown

author
#1 lore raw

PATCH 1/1 Fix problem with unsigned variable going "negative" in linear.c

We replace 'size' by 'start'.
'start' means exactly the same as 'curr_offset - size', and the equivalence of the new code can be tested based on this. The difference is that 'start' will never be negative and so can fit in a 'sector_t' while 'size' could be negative.

Also make curr_offset sector_t, as it should have been.

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/linear.c |   21 +++++++++++----------
 1 files changed, 11 insertions(+), 10 deletions(-)
drivers/md/linear.c renamed from drivers/md/linear.c~current~ +11 -10
diff ./drivers/md/linear.c~current~ ./drivers/md/linear.c
@@ -117,8 +117,8 @@ static int linear_run (mddev_t *mddev)
 	struct linear_hash *table;
 	mdk_rdev_t *rdev;
 	int i, nb_zone, cnt;
-	sector_t size;
-	unsigned int curr_offset;
+	sector_t start;
+	sector_t curr_offset;
 	struct list_head *tmp;
 
 	conf = kmalloc (sizeof (*conf) + mddev->raid_disks*sizeof(dev_info_t),

@@ -193,23 +193,24 @@ static int linear_run (mddev_t *mddev)
 	 * Here we generate the linear hash table
 	 */
 	table = conf->hash_table;
-	size = 0;
+	start = 0;
 	curr_offset = 0;
 	for (i = 0; i < cnt; i++) {
 		dev_info_t *disk = conf->disks + i;
 
+		if (start > curr_offset)
+			table[-1].dev1 = disk;
+
 		disk->offset = curr_offset;
 		curr_offset += disk->size;
 
-		if (size < 0) {
-			table[-1].dev1 = disk;
-		}
-		size += disk->size;
-
-		while (size>0) {
+		/* 'curr_offset' is the end of this disk
+		 * 'start' is the start of table
+		 */
+		while (start < curr_offset) {
 			table->dev0 = disk;
 			table->dev1 = NULL;
-			size -= conf->smallest->size;
+			start += conf->smallest->size;
 			table++;
 		}
 	}
author
#2 lore raw

PATCH 1/1 Improve 'hash' code in linear.c

In reply to NeilBrown

The hashtable that linear uses to find the right device stores two pointers for every entry.
The second is always one of:
The first plus 1
NULL
When NULL, it is never access, so any value can be stored. Thus it could always be "first plus 1", and so we don't need to store it as it is trivial to calculate.

This patch halves the size of this table, which results in some simpler code as well.

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/linear.c         |   38 ++++++++++++++------------------------
 ./include/linux/raid/linear.h |    7 +------
 2 files changed, 15 insertions(+), 30 deletions(-)
drivers/md/linear.c renamed from drivers/md/linear.c~current~ +14 -24
diff ./drivers/md/linear.c~current~ ./drivers/md/linear.c
@@ -31,7 +31,7 @@
  */
 static inline dev_info_t *which_dev(mddev_t *mddev, sector_t sector)
 {
-	struct linear_hash *hash;
+	dev_info_t *hash;
 	linear_conf_t *conf = mddev_to_conf(mddev);
 	sector_t block = sector >> 1;
 

@@ -39,12 +39,11 @@ static inline dev_info_t *which_dev(mdde
 	 * sector_div(a,b) returns the remainer and sets a to a/b
 	 */
 	(void)sector_div(block, conf->smallest->size);
-	hash = conf->hash_table + block;
+	hash = conf->hash_table[block];
 
-	if ((sector>>1) >= (hash->dev0->size + hash->dev0->offset))
-		return hash->dev1;
-	else
-		return hash->dev0;
+	while ((sector>>1) >= (hash->size + hash->offset))
+		hash++;
+	return hash;
 }
 
 /**

@@ -114,7 +113,7 @@ static int linear_issue_flush(request_qu
 static int linear_run (mddev_t *mddev)
 {
 	linear_conf_t *conf;
-	struct linear_hash *table;
+	dev_info_t **table;
 	mdk_rdev_t *rdev;
 	int i, nb_zone, cnt;
 	sector_t start;

@@ -184,7 +183,7 @@ static int linear_run (mddev_t *mddev)
 		nb_zone = conf->nr_zones = sz + (round ? 1 : 0);
 	}
 			
-	conf->hash_table = kmalloc (sizeof (struct linear_hash) * nb_zone,
+	conf->hash_table = kmalloc (sizeof (dev_info_t*) * nb_zone,
 					GFP_KERNEL);
 	if (!conf->hash_table)
 		goto out;

@@ -198,9 +197,6 @@ static int linear_run (mddev_t *mddev)
 	for (i = 0; i < cnt; i++) {
 		dev_info_t *disk = conf->disks + i;
 
-		if (start > curr_offset)
-			table[-1].dev1 = disk;
-
 		disk->offset = curr_offset;
 		curr_offset += disk->size;
 

@@ -208,10 +204,8 @@ static int linear_run (mddev_t *mddev)
 		 * 'start' is the start of table
 		 */
 		while (start < curr_offset) {
-			table->dev0 = disk;
-			table->dev1 = NULL;
+			*table++ = disk;
 			start += conf->smallest->size;
-			table++;
 		}
 	}
 	if (table-conf->hash_table != nb_zone)

@@ -255,13 +249,6 @@ static int linear_make_request (request_
 
 	tmp_dev = which_dev(mddev, bio->bi_sector);
 	block = bio->bi_sector >> 1;
-  
-	if (unlikely(!tmp_dev)) {
-		printk("linear_make_request: hash->dev1==NULL for block %llu\n",
-			(unsigned long long)block);
-		bio_io_error(bio, bio->bi_size);
-		return 0;
-	}
     
 	if (unlikely(block >= (tmp_dev->size + tmp_dev->offset)
 		     || block < tmp_dev->offset)) {

@@ -306,17 +293,20 @@ static void linear_status (struct seq_fi
 #ifdef MD_DEBUG
 	int j;
 	linear_conf_t *conf = mddev_to_conf(mddev);
+	sector_t s = 0;
   
 	seq_printf(seq, "      ");
 	for (j = 0; j < conf->nr_zones; j++)
 	{
 		char b[BDEVNAME_SIZE];
+		s += conf->smallest_size;
 		seq_printf(seq, "[%s",
-			   bdevname(conf->hash_table[j].dev0->rdev->bdev,b));
+			   bdevname(conf->hash_table[j][0].rdev->bdev,b));
 
-		if (conf->hash_table[j].dev1)
+		while (s > conf->hash_table[j][0].offset + 
+		           conf->hash_table[j][0].size)
 			seq_printf(seq, "/%s] ",
-				   bdevname(conf->hash_table[j].dev1->rdev->bdev,b));
+				   bdevname(conf->hash_table[j][1].rdev->bdev,b));
 		else
 			seq_printf(seq, "] ");
 	}
include/linux/raid/linear.h renamed from include/linux/raid/linear.h~current~ +1 -6
diff ./include/linux/raid/linear.h~current~ ./include/linux/raid/linear.h
@@ -11,14 +11,9 @@ struct dev_info {
 
 typedef struct dev_info dev_info_t;
 
-struct linear_hash
-{
-	dev_info_t *dev0, *dev1;
-};
-
 struct linear_private_data
 {
-	struct linear_hash	*hash_table;
+	dev_info_t		**hash_table;
 	dev_info_t		*smallest;
 	int			nr_zones;
 	dev_info_t		disks[0];