On Wed, Dec 21, 2016 at 04:23:17PM +1100, Dave Chinner wrote: > It's a philoophical and architectural issue. We currently have a > distinct separation between generic functionality and per-filesystem > specific definitions. The on-disk definitions are owned by the > filesystem not the generic code, and the generic code /never/ sees > what the filesystem stores on disk. THe VFS is supposed to be > completely independent of what the filesystems store on disk - it's > an abstract concept - so that it can morph into whatever is required > to support the functionality the different filesystem provides. I had the same concerns as Dave and Darrick --- which is that it just "feels" wrong to define file system encoding semantics in generic code. So it's really much more of a taste issue than anything else. I'll note that we've already started taking some steps down this slippery path with the definition of whiteout --- where we define WHITEOUT_MODE and WHITEOUT_DEV to be 0, and both the generic code and the file system code need to agree that the on-disk encoding of a whiteout is an inode with mode S_IFCHR | WHITEOUT_MODE, and with the device number set to WHITEOUT_DEV. (Why we define WHITEOUT_MODE to be zero when everyone has to *know* and use the same on-disk encoding of S_IFCHR | WhiTEOUT_MODE doesn't make any sense to me; it would be much simpler to #define WHITEOUT_MODE to be S_IFCHR since if it ever changed, all file on-disk encodings of overlay file systems would go *boom*.) The fact that the three bit encoding of the directory file types is fully defined, and can *not* ever be extended without breaking things means that the chances that it would be a problem is much less. So I don't object quite as much as Dave and Darrick --- but I'll also point out the benefits are quite small. All we are saving is 16 bytes per file system compiled into the kernel. So it's a lot of discussion / changes for very little gain. > The way we normally handle this is a method callout of some kind > into the filesystem to do the filesystem specific function, and > if there are multiple filesystems that do the same thing, they use a > common function. So that part of the patchset - providing common > helpers to inode mode/filesytem d_type conversion - is fine. The common helpers are inline functions that do an array lookup. If we provide some other more generic way of letting the file systems provide conversion functions, at that point we're not really saving anything, and just adding complexity for no real benefit. Which perhaps is my biggest argument against it; making this be generic adds complexity where it's not really buying us anything (except for 16 bytes of data segment). But whether we think it's a bad idea due to a slippery slope argument or due to a complexity argument, it's clear that the gains are marginal, and while I personally am willing compromise a little on the slippery slope argument if the gains are large enough --- for example, the fs/crypto code is certainly adding some commonality in ways that affect on-disk encodings, in this particular case, the wins just don't seem high enough. Cheers, - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
