Re: [PATCH] drivers/staging/exfat - by default, prohibit mount of fat/vfat

From: Gao Xiang <hsiangkao@aol.com>
To: Dave Chinner <david@fromorbit.com>
Cc: "Valdis Klētnieks" <valdis.kletnieks@vt.edu>,
	"Christoph Hellwig" <hch@infradead.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Sasha Levin" <alexander.levin@microsoft.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] drivers/staging/exfat - by default, prohibit mount of fat/vfat
Date: Sun, 1 Sep 2019 09:37:19 +0800	[thread overview]
Message-ID: <20190901013715.GA8243@hsiangkao-HP-ZHAN-66-Pro-G1> (raw)
In-Reply-To: <20190901010721.GG7777@dread.disaster.area>

Hi Dave,

On Sun, Sep 01, 2019 at 11:07:21AM +1000, Dave Chinner wrote:
> On Sat, Aug 31, 2019 at 06:25:21AM -0400, Valdis Kl??tnieks wrote:
> > On Fri, 30 Aug 2019 23:46:16 -0700, Christoph Hellwig said:
> > > Since when did Linux kernel submissions become "show me a better patch"
> > > to reject something obviously bad?
> > 
> > Well, do you even have a *suggestion* for a better idea?  Other than "just rip
> > it out"?  Keeping in mind that:
> > 
> > > As I said the right approach is to probably (pending comments from the
> > > actual fat maintainer) to merge exfat support into the existing fs/fat/
> > > codebase.  You obviously seem to disagree (and at the same time not).
> > 
> > At this point, there isn't any true consensus on whether that's the best
> > approach at the current.
> 
> Which, quite frankly, means it has been merged prematurely.
> 
> Valdis - the model for getting a new filesystem merged is the one
> taken by Orangefs. That was not merged through the staging tree,
> it was reviewd via patches to linux-fsdevel that were iterated far
> faster than the stable merge cycle allows, allowed all the cleanups
> to be done independently of the feature work needed, the structural
> changes we easy to discuss, quote, etc.

fs/orangefs/dir.c
 61 static int do_readdir(struct orangefs_inode_s *oi,
 62     struct orangefs_dir *od, struct dentry *dentry,
 63     struct orangefs_kernel_op_s *op)

131 static int parse_readdir(struct orangefs_dir *od,
132     struct orangefs_kernel_op_s *op)

189 static int fill_from_part(struct orangefs_dir_part *part,
190     struct dir_context *ctx)

fs/orangefs/file.c
 19 static int flush_racache(struct inode *inode)

 48 ssize_t wait_for_direct_io(enum ORANGEFS_io_type type, struct inode *inode,
 49     loff_t *offset, struct iov_iter *iter, size_t total_size,
 50     loff_t readahead_size, struct orangefs_write_range *wr, int *index_return)

fs/orangefs/super.c
304 
305 int fsid_key_table_initialize(void)
306 {
307         return 0;
308 }
309 
310 void fsid_key_table_finalize(void)
311 {
312 }

----> no prefix and empty functions

fs/orangefs/xattr.c
 31 static int is_reserved_key(const char *key, size_t size)
 40 static inline int convert_to_internal_xattr_flags(int setxattr_flags)
 54 static unsigned int xattr_key(const char *key)

> 
> These are the sorts of problems we are having with EROFS right now,
> even though it's been in staging for some time, and it's clear we
> are already having them with exfat - fundamental architecture issues
> have not yet been decided, and so there's likely major structural
> change yet to be done.
> 
> That's stuff that is much more easily done and reveiwed by patches
> on a mailing list. You don't need the code in the staging tree to
> get this sort of thing done and, really, having it already merged
> gets in the way of doing major structural change as it cannot be
> rapidly iterated independently of the kernel dev cycle...
> 
> So when Christoph say:
> 
> > "Just rip it out"
> 
> what he is really saying is that Greg has jumped the jump and is -
> yet again - fucking over filesystem developers because he's
> taken the review process for a new filesystem out of hands _yet
> again_.
> 
> He did this with POHMELFS, then Lustre, then EROFS - they all got
> merged into stable over the objections of senior filesystem
> developers.  The first two were an utter disaster, the latter is
> rapidly turning into one.

I don't know what "rapidly turning" means:
 This round I am working on all suggestions from fs developers;
 and I have been addressing what Christoph said for days, he hope that
 all functions should be prefixed with "erofs_" and I am doing.

That is all.

Thanks,
Gao Xiang