On Thu, Aug 04, 2011 at 12:22:58AM -0700, Allison Henderson wrote: > > Oh, I think we do avoid calling the unmap for this last condition > though. The first and last page offsets are calculated earlier for > calling truncate_inode_pages_range to release all the pages in the > hole. The idea is that everything from first_page_offset to > last_page_offset covers all the page aligned pages in the hole. So > then if offset and length are aligned, we basically end up with > first_page_offset = offset and last_page_offset = offset + length, > and the page_len will turn out to be zero. Right math? Maybe we > can add some comments or something to help clarify. Yeah, sorry, I wasn't clear enough about the condition. Consider the situation where we punch the region: 4092 -- 8197 In the previous section of code, we would zero out the byte ranges 4092--4095 and 8192--8197. What's left is a completely page-aligned range, which would have already been taken care of already. But since we're calculating based on offsets, I believe there will be an unnecessary call to ext4_unmap_page_range(). BTW, the name ext4_unmap_page_range() is a bit confusing; maybe we should rename it to ext4_unmap_partial_page_buffers()? I know you were copying from the ext4_block_zero_page_range() function and its calling sequence (but in my opinion that function wasn't named well and the comments in that code aren't good either). I also wonder why we can't fold the functionality found in ext4_unmap_page_range() into ext4_block_zero_page_range(). Did you look into that option? Regards, - Ted -- 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
