On Wed, Dec 21, 2016 at 7:23 AM, Dave Chinner <david@xxxxxxxxxxxxx> wrote: > On Tue, Dec 20, 2016 at 07:20:22AM +0200, Amir Goldstein wrote: >> On Tue, Dec 20, 2016 at 2:28 AM, Darrick J. Wong >> <darrick.wong@xxxxxxxxxx> wrote: >> > On Mon, Dec 19, 2016 at 10:11:08PM +0200, Amir Goldstein wrote: >> > Note: We did this all this work a short time ago so that ext4 and XFS >> > could present the same FSGETXATTR ioctl to user programs (major benefit) >> > but it was kind of a pain to get right. I don't see an upside to ceding >> > control of this part of the disk format to the VFS. >> > >> >> The answer "this is not wanted for XFS" is perfectly valid :) >> The common implementation can be used by the small fs, which never >> want anymore than the basic ext2 implementation. >> >> However, since the DT_ values and XFS_DIR3_FT_ values of 0..7 >> are already carved in stone, as long as comments in both common code >> and libxfs code makes that clear, I see no harm in using the common >> implementation in xfs, besides the need to yank more code from fs.h to >> libxfs, but it's your decision. > > 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. > Technically, this abstraction is not within the scope of VFS, but more something of a "libfs", or code that belongs under fs/common, much like the new fs/crypto and in theory those common bits should also belong to a userspace libfs library, but that's taking this a bit too far now. > 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 part that isn't fine, IMO, is defining the filesystem d_type > values in generic code. They should be defined by the filesystem and > passed to the generic conversion functions as a constant. It may > require a different structure to do this cleanly (i.e. something > other than a sparse array keyed on S_IFMT), but I think that having > the VFS define on-disk formats like this is a slippery slope that > only leads to long term pain and ossification. > I agree. I will add this fs-specfic-type-conversion to the common helpers. common code will convert i_mode => DT_ => common FT_ and if fs provides a specific conversion table that will also be used to convert common FT_ values to FS_FT_ values. I'll see if that turns up to look useful. if not I will just leave the common conversions from sparse DT values in common code and leave the rest in fs specific code. Thanks for the feedback. Ted, Chris, Do you share a similar "philosophy" on the matter? -- 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
