From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ipmail06.adl2.internode.on.net ([150.101.137.129]:9658 "EHLO ipmail06.adl2.internode.on.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751638AbcLUFXW (ORCPT ); Wed, 21 Dec 2016 00:23:22 -0500 Date: Wed, 21 Dec 2016 16:23:17 +1100 From: Dave Chinner To: Amir Goldstein Cc: "Darrick J. Wong" , Jan Kara , Theodore Ts'o , Chris Mason , Boaz Harrosh , Jaegeuk Kim , Ryusuke Konishi , Mark Fasheh , Evgeniy Dushistov , Miklos Szeredi , Al Viro , linux-fsdevel Subject: Re: [RFC][PATCH 11/11] xfs: use common file type conversion Message-ID: <20161221052317.GA4758@dastard> References: <1482178268-22883-1-git-send-email-amir73il@gmail.com> <1482178268-22883-12-git-send-email-amir73il@gmail.com> <20161220002832.GB7297@birch.djwong.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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 > 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. 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. Cheers, Dave. -- Dave Chinner david@fromorbit.com