On Mon, Jul 22, 2013 at 08:57:45PM +0800, Zheng Liu wrote: > On Mon, Jul 22, 2013 at 08:02:55PM +1000, Dave Chinner wrote: > > On Mon, Jul 22, 2013 at 10:17:42AM +0800, Zheng Liu wrote: > > > I don't think fiemap is a good interface. The application uses > > > fiemap(2) to retrieve extent mapping. > > > > fiemap is used to query information about extent maps. What it > > returns is entirely dependent on the input parameters that are > > passed to it. Indeed, from Documentation/filesystems/fiemap.txt: > > > > "If fm_extent_count is zero, then the fm_extents[] array is ignored > > (no extents will be returned), and the fm_mapped_extents count will > > hold the number of extents needed in fm_extents[] to hold the file's > > current mapping." > > > > Think about that for a minute. What does the filesystem do with such > > an fiemap query when the extent map is not cached? That's right, > > *fiemap reads the extent map from disk into the cache* and then > > returns the number of extents in the range. > > > > All I have suggested is adding a flag to make this an *explicit > > operation* rather than a side effect of a "count extents" query. I > > fail to see any justification for a whole new interface when we > > already have a perfectly functional one that already provides the > > functionality that is required... > > Yes, I understand your point of view. We can use fiemap to do that. > All I concern is about semantics. When someone mention about fiemap, > first I remember is that I can use it to retrieve the extent mappings. But that's exactly what Ted wants to do - retreive extent maps from disk. From Documentation/filesystems/fiemap.txt: "The fiemap ioctl is an efficient method for userspace to get file extent mappings. ..... Certain flags to modify the way in which mappings are looked up can be set in fm_flags. ..... This scheme is intended to allow the fiemap interface to grow in the future but without losing compatibility with old software. ..... " The functionality being discussed here is userspace being able to retreive extents from disk into memory. The initial description of FIEMAP covers this. We can change the way fiemap behaves by input flags - that's clearly stated - and it is intended to be extended in this manner for future functionality. That's what I suggested to make it explicit, but it's not actually necessary to actually read the extent map into memory with FIEMAP. Keep in mind that this "new extent map functionality" is *exactly* why we designed the FIEMAP interface to be extensible. > But for fadvise, it looks like more naturally. fadvise() is for giving data access behaviour hints. It is not for controlling how filesystems access their internal metadata. > When I look at it, I > always think that I can use it to provide a hint to the kernel, and then > the kernel will do the rest of things for me. So that is why I prefer > to use a fadvise flag rather than use fiemap. But Ted's case is not a "hint" - it's a direct command to fetch the extent map from disk. You can do that already with FIEMAP, so no new code or interfaces are needed. fadvise() is not the proper interface for manipulating filesystem metadata behaviour, and fiemap can already do what you need. There is no need for any new interfaces here. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx -- 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
