On Apr 18, 2008 17:09 -0400, Josef Bacik wrote: > Here is my patch for fiemap support on ext3. The main reason for doing this is > because it will make it easier for application developers who are wanting to > take advantage of fiemap on extent based fs's to be able to use the same > interface for ext3 as well without having to fallback onto something like > fibmap. Fibmap also means you are calling ext3_get_block for _every_ block in > the file, which is ineffecient when ext3_get_blocks can map multiple contiguous > blocks all at once, reducing the number of times you have to call > ext3_get_blocks. Tested this with sandeens fiemap test program and verified it > with filefrag. Thanks much, > > Signed-off-by: Josef Bacik <jbacik@xxxxxxxxxx> Josef, thanks for doing this work. Having more than a single filesystem implement FIEMAP (especially a block-mapped one) is very useful. Did you look at all at making a "generic_fiemap()" function? It seems very little of ext3_fiemap() is ext3 specific, only the call to ext3_force_commit() (which could just be a sync on the inode), ext3_block_map() (generic for all block-based filesystems), and truncate_mutex (would i_sem be enough?). > +int ext3_fiemap(struct inode *inode, unsigned long arg) > +{ > + /* > + * if fm_start is in the middle of the current block, get the next > + * block so we don't end up returning a start thats before the given > + * fm_start > + */ > + start_blk = (fiemap_s->fm_start + (1 << inode->i_blkbits) - 1) >> > + inode->i_blkbits; Hmm, I'd think that if someone is requesting the mapping for bytes [50-5000] they wouldn't be very happy with the mapping returned being [4096-8191], because it is missing part of the requested range. Instead, the fm_start should be rounded down to the start of the first block and up to the end of the last block to return [0-8191] (fm_start = 0, fm_length = 8192). Cheers, Andreas -- Andreas Dilger Sr. Staff Engineer, Lustre Group Sun Microsystems of Canada, Inc. -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html