Disk Cache Patch

"Changju Gao" <[email protected]> Wed, 06 Jun 2007 10:30:21 -0600
Newsgroups gmane.linux.evms.devel
Message-ID <[email protected]> 
The local disk cache uses the address of an object as the identification for a cached entry. 
However, the addresses are not unique (in some cases). In our tests, the cache returned 
wrong data and caused serious problems. 

I changed the code to use the device major/minor number to identify cache entries. 

The attached patch fixes the problems we saw. 

Best regards,


- Changju Gao

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

_______________________________________________
Evms-devel mailing list
[email protected]
To subscribe/unsubscribe, please visit:
https://lists.sourceforge.net/lists/listinfo/evms-devel
disk_cache.patch (text/plain, 3.9 KB) 
--- evms-2.5.5/plugins/disk/cache.c	2003-08-27 13:18:45.000000000 -0600
+++ evms-2.5.5/plugins/disk/cache.c	2007-06-06 09:52:48.000000000 -0600
@@ -49,7 +49,8 @@
  * Structure to hold information about a cache
  ***/
 typedef struct cache_entry_s {
-	storage_object_t	* disk;
+	u_int32_t		dev_major;			/* Device major number */
+	u_int32_t		dev_minor;			/* Device minor number */
 	lsn_t			sector;
 	sector_count_t		count;
 	void			* buffer;
@@ -180,7 +181,8 @@
 
 	cache_entry = hash_entry->next;
 	while (cache_entry != hash_entry &&
-	       (cache_entry->disk != disk ||
+	       (cache_entry->dev_major != disk->dev_major ||
+	        cache_entry->dev_minor != disk->dev_minor ||
 	        cache_entry->sector != sector ||
 	        cache_entry->count < count)) {
 		cache_entry = cache_entry->next;
@@ -191,8 +193,8 @@
 		goto out;
 	}
 
-	LOG_DEBUG("Read from cache.  disk %s, sector %"PRIu64", count %"PRIu64".\n",
-		  disk->name, sector, count);
+	LOG_DEBUG("Read from cache.  disk %s (%d:%d), sector %"PRIu64", count %"PRIu64".\n",
+		  disk->name, disk->dev_major, disk->dev_minor, sector, count);
 	memcpy(buffer, cache_entry->buffer, count << EVMS_VSECTOR_SIZE_SHIFT);
 	cache_hit_count++;
 	rc = 0;
@@ -228,7 +230,8 @@
 
 	cache_entry = hash_entry->next;
 	while (cache_entry != hash_entry &&
-	       (cache_entry->disk != disk ||
+	       (cache_entry->dev_major != disk->dev_major ||
+	        cache_entry->dev_minor != disk->dev_minor ||
 	        cache_entry->sector != sector)) {
 		cache_entry = cache_entry->next;
 	}
@@ -236,14 +239,14 @@
 	if (cache_entry != hash_entry) {
 		/* Found a cache entry. */
 		if (cache_entry->count >= count) {
-			LOG_DEBUG("Cache entry already exists for disk %s, sector %"PRIu64".\n",
-				  disk->name, sector);
+			LOG_DEBUG("Cache entry already exists for disk %s (%d:%d), sector %"PRIu64".\n",
+				  disk->name, disk->dev_major, disk->dev_minor, sector);
 			rc = EEXIST;
 			goto out;
 		}
 
-		LOG_DEBUG("Cache entry for disk %s, sector %"PRIu64" is too small.\n",
-			  disk->name, sector);
+		LOG_DEBUG("Cache entry for disk %s (%d:%d) sector %"PRIu64" is too small.\n",
+			  disk->name, disk->dev_major, disk->dev_minor, sector);
 		LOG_DEBUG("   Has %"PRIu64" sectors; need %"PRIu64" sectors.\n",
 			  cache_entry->count, count);
 		free(cache_entry->buffer);
@@ -251,8 +254,8 @@
 		cache_entry->count = 0;
 	} else {
 		/* Need a new cache entry. */
-		LOG_DEBUG("Create a new cache entry for disk %s, sector %"PRIu64", count %"PRIu64".\n",
-			  disk->name, sector, count);
+		LOG_DEBUG("Create a new cache entry for disk %s (%d:%d), sector %"PRIu64", count %"PRIu64".\n",
+			  disk->name, disk->dev_major, disk->dev_minor, sector, count);
 		cache_entry = calloc(1, sizeof(cache_entry_t));
 		if (!cache_entry) {
 			LOG_SERIOUS("Could not allocate memory for a new cache entry.\n");
@@ -260,17 +263,18 @@
 			goto out;
 		}
 
-		cache_entry->disk = disk;
+		cache_entry->dev_major = disk->dev_major;
+		cache_entry->dev_minor = disk->dev_minor;
 		cache_entry->sector = sector;
 	}
 
 	if (cache_entry->buffer == NULL) {
-		LOG_DEBUG("Allocate a new buffer for the cache entry for disk %s, sector %"PRIu64", count %"PRIu64".\n",
-			  disk->name, sector, count);
+		LOG_DEBUG("Allocate a new buffer for the cache entry for disk %s (%d:%d), sector %"PRIu64", count %"PRIu64".\n",
+			  disk->name, disk->dev_major, disk->dev_minor, sector, count);
 		cache_entry->buffer = malloc(count << EVMS_VSECTOR_SIZE_SHIFT);
 		if (!cache_entry->buffer) {
-			LOG_SERIOUS("Could not allocate memory for a buffer for a cache entry for disk %s, sector %"PRIu64", count %"PRIu64".\n",
-				    disk->name, sector, count);
+			LOG_SERIOUS("Could not allocate memory for a buffer for a cache entry for disk %s (%d:%d), sector %"PRIu64", count %"PRIu64".\n",
+				    disk->name, disk->dev_major, disk->dev_minor, sector, count);
 			if (cache_entry->prev) {
 				list_delete(cache_entry);
 			}