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

From: Gao Xiang
Date: Sat Aug 31 2019 - 21:37:40 EST


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