ext3 getblocks() support for read

Hi,

Here is the patch to support ext3 getblocks() for non allocation cases. (for reads & re-writes). This is useful with DIO reads, DIO re-writes and to go with Christoph's getblocks() for readpages() work.

Mingming is working on adding multiblock allocation support using reservation (which can be incrementally added later).

Comments ?

fs/ext3/inode.c +7 -2

@@ -681,9 +683,10 @@ ext3_get_block_handle(handle_t *handle, 
 	Indirect *partial;
 	unsigned long goal;
 	int left;
-	int boundary = 0;
-	const int depth = ext3_block_to_path(inode, iblock, offsets, &boundary);
+	int blks_boundary = 0;
+	const int depth = ext3_block_to_path(inode, iblock, offsets, &blks_boundary);
 	struct ext3_inode_info *ei = EXT3_I(inode);
+	int count = 1;
 
 	J_ASSERT(handle != NULL || create == 0);
 

@@ -694,7 +697,18 @@ ext3_get_block_handle(handle_t *handle, 
 
 	/* Simplest case - block found, no allocation needed */
 	if (!partial) {
+		unsigned long first_block = le32_to_cpu(chain[depth-1].key);
+
 		clear_buffer_new(bh_result);
+
+		/*

Quoted 7 lines · Badari Pulavarty

Hi,

Here is the patch to support ext3 getblocks() for non allocation cases. (for reads & re-writes). This is useful with DIO reads, DIO re-writes and to go with Christoph's getblocks() for readpages() work.

Quoted 5 lines

Mingming is working on adding multiblock allocation support using reservation (which can be incrementally added later).

Comments ?

My ext3 multiple block allocation patch posted a while ago includes the multiple blocks map as well. Looks mostly the same way you did here, but I like the way that how the # of mapped(or allocated) blocks are returned.

My plan is to break the whole ext3 multiple block allocation(also does map) patch into small patches, and re-send soon.

Quoted 23 lines

fs/ext3/inode.c +7 -2

@@ -681,9 +683,10 @@ ext3_get_block_handle(handle_t *handle, 
 	Indirect *partial;
 	unsigned long goal;
 	int left;
-	int boundary = 0;
-	const int depth = ext3_block_to_path(inode, iblock, offsets, &boundary);
+	int blks_boundary = 0;
+	const int depth = ext3_block_to_path(inode, iblock, offsets, &blks_boundary);
 	struct ext3_inode_info *ei = EXT3_I(inode);
+	int count = 1;
 
 	J_ASSERT(handle != NULL || create == 0);
 

@@ -694,7 +697,18 @@ ext3_get_block_handle(handle_t *handle, 
 
 	/* Simplest case - block found, no allocation needed */
 	if (!partial) {
+		unsigned long first_block = le32_to_cpu(chain[depth-1].key);
+
 		clear_buffer_new(bh_result);
+
+		/*

fs/ext3/inode.c ext3_get_block_handle(handle_t *handle, · line 714

Quoted 10 lines

fs/ext3/inode.c +7 -0

+		 * Find all the contiguous blocks and return at once.
+		 */
+		while (count < max_blocks && count <= blks_boundary &&
+			(le32_to_cpu(*(chain[depth-1].p+count)) == 
+				(first_block + count))) {
+			count++;
+		}
 		goto got_it;
 	}
 

Here we need to be careful about the branch we just read, since we are looking up multiple blocks (they are on the same branch) at the same time, it is possible that during the look up, another threads is trucating the same branch we are trying to map. Before since we are doing only one look up, a simple verify_chain() should be safe.

The simple way is, for the non-allocation case, take the truncate_sem before the ext3_get_branch, like for the allocation-case, even for the simple case -- but that probably will slow down the non-allocation, probably a bad option. But we could re-check(calling verify_chain())inside the while loop.