Re: [PATCH 0/5 v2] add extent status tree caching
From: Theodore Ts'o <tytso@mit.edu>
Date: 2013-08-04 01:59:10
On Tue, Jul 30, 2013 at 01:08:07PM +1000, Dave Chinner wrote:
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.
I've been looking at the definition of fiemap, and I'm not convinced.
To quote from the fiemap.txt:
The fiemap ioctl is an efficient method for userspace to get file
extent mappings.
That's not what is going on here. We are pre-caching them into kernel
memory, not in user-space. In addition, we're also setting a flag to
keep these extents preferentially in memory compared to other entries
in the extent cache.
I agree that posix_fadvise() isn't really a good match, either:
"posix_fadvise - predeclare an access pattern for file data"
How about this? FIEMAP is an ioctl, anyway. How about if we just
declare this as a new fs-independent ioctl, much like FS_IOC_FIEMAP?
#define FS_IOC_PRECACHE_EXTENTS _IO('f', 18)
This is, of course, assuming that other file systems are interested in
implementing this functionality. If not, we can just keep it as
EXT4_IOC_PRECACHE_EXTENTS, and just call it a day. (We can always add
a definition of FS_IOC_PRECACHE_EXTENTS set to ext4 ioctl's code
point, at some later point, if people change their minds.)
- Ted