| Message ID | 20230903032555.np6lu5mouv5tw4ff@moria.home.lan (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [GIT,PULL] bcachefs | expand |
Hi Kent, I thought you'd follow Christians proposal to actually work with people to try to use common APIs (i.e. to use iomap once it's been even more iter-like, for which I'm still waiting for suggestions), and make your new APIs used more widely if they are a good idea (which also requires explaining them better) and aim for 6.7 once that is done. But without that, and without being in linux-next and the VFS maintainer ACK required for I think this is a very bad idea.
On Tue, Sep 05, 2023 at 06:24:47AM -0700, Christoph Hellwig wrote: > Hi Kent, > > I thought you'd follow Christians proposal to actually work with > people to try to use common APIs (i.e. to use iomap once it's been > even more iter-like, for which I'm still waiting for suggestions), > and make your new APIs used more widely if they are a good idea > (which also requires explaining them better) and aim for 6.7 once > that is done. Christoph, I get that iomap is your pet project and you want to make it better and see it more widely used. But the reasons bcachefs doesn't use iomap have been discussed at length, and I've posted and talked about the bcachefs equivalents of that code. You were AWOL on those discussions; you consistently say "bcachefs should use iomap" and then walk away, so those discussions haven't moved forwards. To recap, besides being more iterator like (passing data structures around with iterators, vs. indirect function calls into the filesystem), bcachefs also hangs a bit more state off the pagecache, due to being multi device and also using the same data structure for tracking disk reservations (because why make the buffered write paths look that up separately?). > But without that, and without being in linux-next and the VFS maintainer > ACK required for I think this is a very bad idea. Christain gave his reviewed-by for the dcache patch. Since this patchset doesn't change existing code maintained by others aside from that one patch, not sure how linux-next is relevant here...
On Tue, Sep 05, 2023 at 08:00:07PM -0400, Kent Overstreet wrote: > On Tue, Sep 05, 2023 at 06:24:47AM -0700, Christoph Hellwig wrote: > > Hi Kent, > > > > I thought you'd follow Christians proposal to actually work with > > people to try to use common APIs (i.e. to use iomap once it's been > > even more iter-like, for which I'm still waiting for suggestions), > > and make your new APIs used more widely if they are a good idea > > (which also requires explaining them better) and aim for 6.7 once > > that is done. > > Christoph, I get that iomap is your pet project and you want to make it > better and see it more widely used. > > But the reasons bcachefs doesn't use iomap have been discussed at > length, and I've posted and talked about the bcachefs equivalents of > that code. You were AWOL on those discussions; you consistently say > "bcachefs should use iomap" and then walk away, so those discussions > haven't moved forwards. > > To recap, besides being more iterator like (passing data structures > around with iterators, vs. indirect function calls into the filesystem), > bcachefs also hangs a bit more state off the pagecache, due to being > multi device and also using the same data structure for tracking disk > reservations (because why make the buffered write paths look that up > separately?). I /thought/ the proposal was to use iomap for bcachefs DIO and leave buffered writes for a different day. I agree the iomap buffered write path is inappropriate for bcachefs today. I'd like that to change, but there's a lot of functionality that it would need to support. I don't see the point in keeping bcachefs out of tree for another cycle. Yes, more use of iomap would be great, but I don't think that it being in-tree is going to hinder anything. It's not like it uses bufferheads.
On Wed, Sep 06, 2023 at 01:41:22AM +0100, Matthew Wilcox wrote: > On Tue, Sep 05, 2023 at 08:00:07PM -0400, Kent Overstreet wrote: > > On Tue, Sep 05, 2023 at 06:24:47AM -0700, Christoph Hellwig wrote: > > > Hi Kent, > > > > > > I thought you'd follow Christians proposal to actually work with > > > people to try to use common APIs (i.e. to use iomap once it's been > > > even more iter-like, for which I'm still waiting for suggestions), > > > and make your new APIs used more widely if they are a good idea > > > (which also requires explaining them better) and aim for 6.7 once > > > that is done. > > > > Christoph, I get that iomap is your pet project and you want to make it > > better and see it more widely used. > > > > But the reasons bcachefs doesn't use iomap have been discussed at > > length, and I've posted and talked about the bcachefs equivalents of > > that code. You were AWOL on those discussions; you consistently say > > "bcachefs should use iomap" and then walk away, so those discussions > > haven't moved forwards. > > > > To recap, besides being more iterator like (passing data structures > > around with iterators, vs. indirect function calls into the filesystem), > > bcachefs also hangs a bit more state off the pagecache, due to being > > multi device and also using the same data structure for tracking disk > > reservations (because why make the buffered write paths look that up > > separately?). > > I /thought/ the proposal was to use iomap for bcachefs DIO and leave > buffered writes for a different day. I agree the iomap buffered write > path is inappropriate for bcachefs today. I'd like that to change, > but there's a lot of functionality that it would need to support. No, I'm not going to convert the bcachefs DIO path to iomap; not as it exitss now. Right now I've got a clean separation between the VFS level DIO code, and the lower level bcachefs code that walks the extents btree and issues IOs. I have to consider the iomap approach where the loop-up-mappings-and-issue loop is in iomap code but calling into filesystem code pretty gross. I was talking about this /years/ ago when I did the work to make it possible to efficiently split bios - iomap could have taken the approach bcachefs did, the prereqs were in place when iomap was started, but it didn't happen - iomap ended up being a more conservative approach, a cleaned up version of buffer heads and fs/direct-IO.c. That's fine, iomap is certainly an improvement over what it was replacing, but it would not be an improvement for bcachefs. I think it might be more fruitful to look at consolidating the buffered IO code in bcachefs and iomap. The conceptual models are a bit closer, bcachefs's buffered IO code is just a bit more fully-featured in that it does the dirty block tracking in a unified way. That was a project that turned out pretty nicely.
On Wed, Sep 06, 2023 at 12:10:02PM -0400, Kent Overstreet wrote: > On Wed, Sep 06, 2023 at 01:41:22AM +0100, Matthew Wilcox wrote: > > On Tue, Sep 05, 2023 at 08:00:07PM -0400, Kent Overstreet wrote: > > > On Tue, Sep 05, 2023 at 06:24:47AM -0700, Christoph Hellwig wrote: > > > > Hi Kent, > > > > > > > > I thought you'd follow Christians proposal to actually work with > > > > people to try to use common APIs (i.e. to use iomap once it's been > > > > even more iter-like, for which I'm still waiting for suggestions), > > > > and make your new APIs used more widely if they are a good idea > > > > (which also requires explaining them better) and aim for 6.7 once > > > > that is done. > > > > > > Christoph, I get that iomap is your pet project and you want to make it > > > better and see it more widely used. Christoph quit as maintainer 78 days ago. > > > But the reasons bcachefs doesn't use iomap have been discussed at > > > length, and I've posted and talked about the bcachefs equivalents of > > > that code. You were AWOL on those discussions; you consistently say > > > "bcachefs should use iomap" and then walk away, so those discussions > > > haven't moved forwards. > > > > > > To recap, besides being more iterator like (passing data structures > > > around with iterators, vs. indirect function calls into the filesystem), > > > bcachefs also hangs a bit more state off the pagecache, due to being > > > multi device and also using the same data structure for tracking disk > > > reservations (because why make the buffered write paths look that up > > > separately?). > > > > I /thought/ the proposal was to use iomap for bcachefs DIO and leave I wasn't aware of /any/ active effort to make bcachefs use iomap for any purpose. > > buffered writes for a different day. I agree the iomap buffered write > > path is inappropriate for bcachefs today. I'd like that to change, > > but there's a lot of functionality that it would need to support. > > No, I'm not going to convert the bcachefs DIO path to iomap; not as it > exitss now. > > Right now I've got a clean separation between the VFS level DIO code, > and the lower level bcachefs code that walks the extents btree and > issues IOs. I have to consider the iomap approach where the > loop-up-mappings-and-issue loop is in iomap code but calling into > filesystem code pretty gross. > > I was talking about this /years/ ago when I did the work to make it > possible to efficiently split bios - iomap could have taken the approach > bcachefs did, the prereqs were in place when iomap was started, but it > didn't happen - iomap ended up being a more conservative approach, a > cleaned up version of buffer heads and fs/direct-IO.c. <shrug> iirc the genesis of xfs "iomap" was that it was created to supply space mappings to pnfs, and then got reused for directio and pagecache operations. Later that was hoisted wholesale to the vfs. Then spectre/meltdown happened and our indirect function call toy got taken away, and now we're stuck figuring out how to remove them all. IOWs, individual tactics that each made sense for maintaining the overall stability of xfs, but overall didn't amount to anything ambitious. In the end I'd probably rather do something like this to eliminate all the indirect function calls: int xfs_zero_range( struct xfs_inode *ip, loff_t pos, loff_t len, bool *did_zero) { struct iomap_iter iter = { }; struct inode *inode = VFS_I(ip); int ret = 0; iomap_start_zero_range(&iter, inode, pos, len); while (iter.len > 0) { ret = xfs_buffered_write_iomap_begin(&iter, &iter.write_map, &iter.read_map); if (ret) break; len = iomap_zero_range_iter(&iter, did_zero); if (len < 0) { ret = len; break; } ret = xfs_buffered_write_iomap_end(&iter, len, &iter.write_map); if (ret) break; iomap_iter_advance(&iter, len); } iomap_end_zero_range(&iter); return iter.write > 0 ? iter.write : ret; } (I would also rename iter.iomap and iter.srcmap to make it more obvious which ones get filled out under what circumstances.) > That's fine, iomap is certainly an improvement over what it was > replacing, but it would not be an improvement for bcachefs. Filesystems are free to implement read_ and write_iter as they choose, whether or not that involves iomap. > I think it might be more fruitful to look at consolidating the buffered > IO code in bcachefs and iomap. The conceptual models are a bit closer, > bcachefs's buffered IO code is just a bit more fully-featured in that it > does the dirty block tracking in a unified way. That was a project that > turned out pretty nicely. I think I'd much rather gradually revise iomap for everyone and cut bcachefs over when its ready. I don't think revising iomap is a reasonable precondition for merging bcachefs, nor do I think it's a good idea to risk destabilizing bcachefs just to hack in a new dependency that won't even fit well. --D
So I'm starting to look at this because I have most other pull requests done, and while I realize there's no universal support for it I suspect any further changes are better done in-tree. The out-of-tree thing has been done. However, while I'll continue to look at it in this form, I just realized that it's completely unacceptable for one very obvious reason: On Sat, 2 Sept 2023 at 20:26, Kent Overstreet <kent.overstreet@linux.dev> wrote: > > https://evilpiepirate.org/git/bcachefs.git bcachefs-for-upstream No way am I pulling that without a signed tag and a pgp key with a chain of trust. You've been around for long enough that having such a key shouldn't be a problem for you, so make it happen. There are a few other issues that I have with this, and Christoph did mention a big one: it's not been in linux-next. I don't know why I thought it had been, it's just such an obvious thing for any new "I want this merged upstream" tree. So these kinds of "I'll just ignore _all_ basic rules" kinds of issues do annoy me. I need to know that you understand that if you actually want this upstream, you need to work with upstream. That very much means *NOT* continuing this "I'll just do it my way". You need to show that you can work with others, that you can work within the framework of upstream, and that not every single thread you get into becomes an argument. This, btw, is not negotiable. If you feel uncomfortable with that basic notion, you had better just continue doing development outside the main kernel tree for another decade. The fact that I only now notice that you never submitted this to linux-next is obviously on me. My bad. But at the same time it worries me that it might be a sign of you just thinking that your way is special. Linus
On Wed, 6 Sept 2023 at 12:36, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > There are a few other issues that I have with this, and Christoph did > mention a big one: it's not been in linux-next. Oh, and the fact that it hasn't been in linux-next became immediately clear, and it's a real practical problem. I get a compile error when just doing a basic built-test: fs/bcachefs/btree_key_cache.c: In function ‘btree_key_cache_create’: fs/bcachefs/btree_key_cache.c:356:56: error: implicit conversion from ‘enum btree_node_locked_type’ to ‘enum six_lock_type’ [-Werror=enum-conversion] 356 | mark_btree_node_locked(trans, path, 0, BTREE_NODE_UNLOCKED); | ^~~~~~~~~~~~~~~~~~~ and that compile error is actually a sign of a pretty serious bug. BTREE_NODE_UNLOCKED is of the wrong enum type, and has a value (-1) that simply cannot be represented in a 'enum six_lock_type'. I'm pretty sure that the compiler is actually allowed to assume it is in the range of the source enum. Yes, the inline function then does a cast to 'enum btree_node_locked_type', but because the compiler may assume that the incoming enum has values 0..2, it's not clear that it will necessarily cast the value -1 to the expected value. It probably works in practice on x86 (without -fshort-enum, I think gcc will always use an 'int'), but that code really is wrong. On other platforms, gcc defaults -fshort-enums on, and in that case a 'enum six_lock_type' is actually of underlying type 'unsigned char'. And guess what happens when you have (unsigned char)-1? It does *not* cast back to -1. I don't know whether any of the architectures we support has that "default to -fshort-enums" behaviour, and at least hexagon uses "-fno-short-enums" to avoid this issue, but it really is a serious bug. A Clang build also results in objtool complaints about bcachefs, with fs/bcachefs/buckets.o: warning: objtool: bch2_trans_mark_metadata_bucket() falls through to next function bch2_trans_mark_dev_sb() fs/bcachefs/fsck.o: warning: objtool: write_inode() falls through to next function hash_check_key() and sure enough, the code generation ends up being movq %rbx, %rdi movl %r12d, %esi callq bch2_trans_restart_error .Lfunc_end25: .size write_inode, .Lfunc_end25-write_inode and the issue is that bch2_trans_restart_error() is marked __noreturn, but objtool hasn't been told about it, so objtool is very unhappy about that "code seems to fall off the side of the earth" kind of issue. Both of these are very much signs of "this has never been tested in other environments", and would presumably have been found immediately in linux-next. As it is, I'm not convinced I want to even continue to go through this all, when it fails at the very first hurdle. A clean build is not some "would be nice" feature, and it is big part of why we have automation for new code. Linus
On Wed, 6 Sept 2023 at 13:02, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > And guess what happens when you have (unsigned char)-1? It does *not* > cast back to -1. Side note: again, this may be one of those "it works in practice", because if we have -fshort-enums, I think 'enum btree_node_locked_type' in turn ends up being represented as a 'signed char', because that's the smallest simple type that can fit all those values. I don't think gcc ever uses less than that (ie while a six_lock_type could fit in two bits, it's still going to be considered at least a 8-bit value in practice). So we may have 'enum six_lock_type' essentially being 'unsigned char', and when the code does mark_btree_node_locked(trans, path, 0, BTREE_NODE_UNLOCKED); that BTREE_NODE_UNLOCKED value might actually be 255. And then when it's cast to 'enum btree_node_locked_type' in the inline function, the 255 will be cast to 'signed char', and we'll end up compatible with '(enum btree_node_locked_type)-1' again. So it's one of those things that are seriously wrong to do, but might generate the expected code anyway. Unless the compiler adds any other sanity checks, like UBSAN or something, that actually uses the exact range of the enums. It could happen even without UBSAN, if the compiler ends up going "I can see that the original value came from a 'enum six_lock_type', so I know the original value can't be signed, so any comparison with BTREE_NODE_UNLOCKED can never be true. But again, I suspect that in practice this all just happens to work. That doesn't make it right. Linus
Em Wed, Sep 06, 2023 at 01:20:59PM -0700, Linus Torvalds escreveu: > On Wed, 6 Sept 2023 at 13:02, Linus Torvalds > <torvalds@linux-foundation.org> wrote: > > > > And guess what happens when you have (unsigned char)-1? It does *not* > > cast back to -1. > > Side note: again, this may be one of those "it works in practice", > because if we have -fshort-enums, I think 'enum > btree_node_locked_type' in turn ends up being represented as a 'signed > char', because that's the smallest simple type that can fit all those > values. > I don't think gcc ever uses less than that (ie while a six_lock_type > could fit in two bits, it's still going to be considered at least a > 8-bit value in practice). There are some cases where people stuff the enum into a bitfield, but no, no simple type. ⬢[acme@toolbox perf-tools-next]$ pahole | grep -w enum | grep : enum btrfs_rsv_type type:8; /* 28:16 4 */ enum btrfs_delayed_item_type type:8; /* 100: 0 4 */ enum kernel_pkey_operation op:8; /* 40: 0 4 */ enum integrity_status ima_file_status:4; /* 96: 0 4 */ enum integrity_status ima_mmap_status:4; /* 96: 4 4 */ enum integrity_status ima_bprm_status:4; /* 96: 8 4 */ enum integrity_status ima_read_status:4; /* 96:12 4 */ enum integrity_status ima_creds_status:4; /* 96:16 4 */ enum integrity_status evm_status:4; /* 96:20 4 */ enum fs_context_purpose purpose:8; /* 152: 0 4 */ enum fs_context_phase phase:8; /* 152: 8 4 */ enum fs_value_type type:8; /* 8: 0 4 */ enum sgx_page_type type:16; /* 8: 8 4 */ enum nf_hook_ops_type hook_ops_type:8; /* 24: 8 4 */ enum resctrl_event_id evtid:8; /* 0:10 4 */ enum _cache_type type:5; /* 0: 0 4 */ ⬢[acme@toolbox perf-tools-next]$ pahole _cache_type enum _cache_type { CTYPE_NULL = 0, CTYPE_DATA = 1, CTYPE_INST = 2, CTYPE_UNIFIED = 3, }; ⬢[acme@toolbox perf-tools-next]$ pahole integrity_status enum integrity_status { INTEGRITY_PASS = 0, INTEGRITY_PASS_IMMUTABLE = 1, INTEGRITY_FAIL = 2, INTEGRITY_FAIL_IMMUTABLE = 3, INTEGRITY_NOLABEL = 4, INTEGRITY_NOXATTRS = 5, INTEGRITY_UNKNOWN = 6, }; ⬢[acme@toolbox perf-tools-next] > > So we may have 'enum six_lock_type' essentially being 'unsigned char', > and when the code does > > mark_btree_node_locked(trans, path, 0, BTREE_NODE_UNLOCKED); > > that BTREE_NODE_UNLOCKED value might actually be 255. > > And then when it's cast to 'enum btree_node_locked_type' in the inline > function, the 255 will be cast to 'signed char', and we'll end up > compatible with '(enum btree_node_locked_type)-1' again. > > So it's one of those things that are seriously wrong to do, but might > generate the expected code anyway. > > Unless the compiler adds any other sanity checks, like UBSAN or > something, that actually uses the exact range of the enums. > > It could happen even without UBSAN, if the compiler ends up going "I > can see that the original value came from a 'enum six_lock_type', so I > know the original value can't be signed, so any comparison with > BTREE_NODE_UNLOCKED can never be true. > > But again, I suspect that in practice this all just happens to work. > That doesn't make it right. > > Linus
Hi Kent, On Sat, Sep 02, 2023 at 11:25:55PM -0400, Kent Overstreet wrote: > here's the bcachefs pull request, for 6.6. Hopefully everything > outstanding from the previous PR thread has been resolved; the block > layer prereqs are in now via Jens's tree and the dcache helper has a > reviewed-by from Christain. I pulled this into mainline locally and did an LLVM build, which found an immediate issue. It appears the bcachefs codes uses zero length arrays for flexible arrays instead of the C99 syntax that the kernel is moving to, which will cause issues with -fstrict-flex-arrays=3 (clang 16+, gcc 13+), see commit df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3"). Currently, building x86_64 defconfig + the bcachefs configs with clang warns (or errors with CONFIG_WERROR): In file included from fs/bcachefs/replicas.c:6: fs/bcachefs/replicas.h:46:2: error: array index 0 is past the end of the array (that has type '__u8[0]' (aka 'unsigned char[0]')) [-Werror,-Warray-bounds] 46 | e->devs[0] = dev; | ^ ~ fs/bcachefs/bcachefs_format.h:1392:2: note: array 'devs' declared here 1392 | __u8 devs[0]; | ^ 1 error generated. GCC would warn in the same manner if -Warray-bounds was not disabled for it... :( In file included from fs/bcachefs/buckets.c:22: In function 'bch2_replicas_entry_cached', inlined from 'update_cached_sectors' at fs/bcachefs/buckets.c:409:2, inlined from 'bch2_mark_alloc' at fs/bcachefs/buckets.c:590:9: fs/bcachefs/replicas.h:46:16: error: array subscript 0 is outside array bounds of '__u8[0]' {aka 'unsigned char[]'} [-Werror=array-bounds=] 46 | e->devs[0] = dev; | ~~~~~~~^~~ In file included from fs/bcachefs/bcachefs.h:206, from fs/bcachefs/buckets.c:8: fs/bcachefs/bcachefs_format.h: In function 'bch2_mark_alloc': fs/bcachefs/bcachefs_format.h:1392:33: note: while referencing 'devs' 1392 | __u8 devs[0]; | ^~~~ GCC shows many other instances of this problem for the same reason (I can send a build log if you'd like), such as 'struct snapshot_table' in subvolume_types.h and several other structures in bcachefs_format.h. Cheers, Nathan
On Wed, Sep 06, 2023 at 06:55:38PM -0300, Arnaldo Carvalho de Melo wrote: > Em Wed, Sep 06, 2023 at 01:20:59PM -0700, Linus Torvalds escreveu: > > On Wed, 6 Sept 2023 at 13:02, Linus Torvalds > > <torvalds@linux-foundation.org> wrote: > > > > > > And guess what happens when you have (unsigned char)-1? It does *not* > > > cast back to -1. > > > > Side note: again, this may be one of those "it works in practice", > > because if we have -fshort-enums, I think 'enum > > btree_node_locked_type' in turn ends up being represented as a 'signed > > char', because that's the smallest simple type that can fit all those > > values. > > > I don't think gcc ever uses less than that (ie while a six_lock_type > > could fit in two bits, it's still going to be considered at least a > > 8-bit value in practice). > > There are some cases where people stuff the enum into a bitfield, but > no, no simple type. > > ⬢[acme@toolbox perf-tools-next]$ pahole | grep -w enum | grep : > enum btrfs_rsv_type type:8; /* 28:16 4 */ > enum btrfs_delayed_item_type type:8; /* 100: 0 4 */ The simple grep might give a skewed view, in the above case there's also /* Bitfield combined with previous fields */ in the full output, with adjacent bool struct members it's all packed into one int. I think I've always seen an int for enums, unless it was explicitly narrowed in the structure (:8) or by __packed attribute in the enum definition. > enum kernel_pkey_operation op:8; /* 40: 0 4 */ > enum integrity_status ima_file_status:4; /* 96: 0 4 */ > enum integrity_status ima_mmap_status:4; /* 96: 4 4 */ > enum integrity_status ima_bprm_status:4; /* 96: 8 4 */ > enum integrity_status ima_read_status:4; /* 96:12 4 */ > enum integrity_status ima_creds_status:4; /* 96:16 4 */ > enum integrity_status evm_status:4; /* 96:20 4 */ > enum fs_context_purpose purpose:8; /* 152: 0 4 */ > enum fs_context_phase phase:8; /* 152: 8 4 */ > enum fs_value_type type:8; /* 8: 0 4 */ > enum sgx_page_type type:16; /* 8: 8 4 */ > enum nf_hook_ops_type hook_ops_type:8; /* 24: 8 4 */ > enum resctrl_event_id evtid:8; /* 0:10 4 */ > enum _cache_type type:5; /* 0: 0 4 */
On Wed, 6 Sept 2023 at 14:55, Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > > I don't think gcc ever uses less than that (ie while a six_lock_type > > could fit in two bits, it's still going to be considered at least a > > 8-bit value in practice). > > There are some cases where people stuff the enum into a bitfield, but > no, no simple type. Note that I am talking about the types gcc uses natively. To show what I'm talking about, build this (silly) code that has some of the same enum types that bcachefs has: #include <stdio.h> enum enum1 { val1 = 0, val2 = 1, val3 = 2, }; enum enum2 { num1 = -1, num2 = 0, num3 = 1, num4 = 2, }; int main(int argc, char **argv) { printf("%d %d (%zu %zu)\n", (enum enum1) num1, (enum enum1) num1 == num1, sizeof(enum enum1), sizeof(enum enum2)); return 0; } and run it. On x86 with no special options, you should get something like this: -1 1 (4 4) ie both types have a four-byte size, and casting 'num1' to 'enum enum1' will in fact give you back -1, and will then compare equal to num1 in the end. Because both types are in practice just 'int'. But now do the same with -fshort-enums, and you instead get 255 0 (1 1) because both types are just a single byte, and casting 'num1' to 'enum enum1' will in fact result in 255 (because it's an _unsigned_ type), and then comparing with the original num1 value will no longer compare equal. (But casting it then further back to 'enum enum2' will in fact result in -1 again, because you're effectively casting it back to 'signed char', and then 255 and -1 are in fact the same in that type). End result: you can get some really unexpected behavior if you cast enums to other types. Which is, of course, exactly why the compiler is warning about the comparison and about passing in the wrong type of enum. Sadly, the compiler doesn't warn about the cast, so you can hide all of this by just adding casts. That doesn't make it *right*, of course. Linus
On Wed, 6 Sept 2023 at 16:20, David Sterba <dsterba@suse.cz> wrote: > > I think I've always seen an int for enums, unless it was > explicitly narrowed in the structure (:8) or by __packed attribute in > the enum definition. 'int' is definitely the default (and traditional) behavior. But exactly because enums can act very differently depending on compiler options (and some of those may have different defaults on different architectures), we should never ever have a bare 'enum' as part of a structure in any UAPI. In fact, having an enum as a bitfield is much better for that case. Doing a quick grep shows that sadly people haven't realized that. Now: using -fshort-enum can break a _lot_ of libraries exactly for this kind of reason, so the kernel isn't unusual, and I don't know of anybody who actually uses -fshort-enum. I'm mentioning -fshort-enum not because it's likely to be used, but mainly because it's an easy way to show some issues. You can get very similar issues by just having unusual enum values. Doing enum mynum { val = 0x80000000 }; does something special too. I leave it to the reader to figure out, but as a hint it's basically exactly the same issue as I was trying to show with my crazy -fshort-enum example. Linus
Em Wed, Sep 06, 2023 at 04:34:32PM -0700, Linus Torvalds escreveu: > On Wed, 6 Sept 2023 at 16:20, David Sterba <dsterba@suse.cz> wrote: > > I think I've always seen an int for enums, unless it was > > explicitly narrowed in the structure (:8) or by __packed attribute in > > the enum definition. > 'int' is definitely the default (and traditional) behavior. > But exactly because enums can act very differently depending on > compiler options (and some of those may have different defaults on > different architectures), we should never ever have a bare 'enum' as > part of a structure in any UAPI. > In fact, having an enum as a bitfield is much better for that case. > Doing a quick grep shows that sadly people haven't realized that. > Now: using -fshort-enum can break a _lot_ of libraries exactly for > this kind of reason, so the kernel isn't unusual, and I don't know of > anybody who actually uses -fshort-enum. I'm mentioning -fshort-enum > not because it's likely to be used, but mainly because it's an easy > way to show some issues. > You can get very similar issues by just having unusual enum values. Doing > > enum mynum { val = 0x80000000 }; > does something special too. > I leave it to the reader to figure out, but as a hint it's basically > exactly the same issue as I was trying to show with my crazy > -fshort-enum example. Two extra hints: ⬢[acme@toolbox perf-tools-next]$ grep KIND_ENUM64 include/uapi/linux/btf.h BTF_KIND_ENUM64 = 19, /* Enumeration up to 64-bit values */ /* BTF_KIND_ENUM64 is followed by multiple "struct btf_enum64". ⬢[acme@toolbox perf-tools-next]$ ⬢[acme@toolbox perf-tools-next]$ pahole --help |& grep enum --skip_encoding_btf_enum64 Do not encode ENUM64s in BTF. ⬢[acme@toolbox perf-tools-next]$ :-) - Arnaldo
Em Wed, Sep 06, 2023 at 08:46:14PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Wed, Sep 06, 2023 at 04:34:32PM -0700, Linus Torvalds escreveu: > > On Wed, 6 Sept 2023 at 16:20, David Sterba <dsterba@suse.cz> wrote: > > > I think I've always seen an int for enums, unless it was > > > explicitly narrowed in the structure (:8) or by __packed attribute in > > > the enum definition. > > 'int' is definitely the default (and traditional) behavior. > > But exactly because enums can act very differently depending on > > compiler options (and some of those may have different defaults on > > different architectures), we should never ever have a bare 'enum' as > > part of a structure in any UAPI. > > In fact, having an enum as a bitfield is much better for that case. > > Doing a quick grep shows that sadly people haven't realized that. > > Now: using -fshort-enum can break a _lot_ of libraries exactly for > > this kind of reason, so the kernel isn't unusual, and I don't know of > > anybody who actually uses -fshort-enum. I'm mentioning -fshort-enum > > not because it's likely to be used, but mainly because it's an easy > > way to show some issues. > > You can get very similar issues by just having unusual enum values. Doing > > enum mynum { val = 0x80000000 }; > > does something special too. > > I leave it to the reader to figure out, but as a hint it's basically > > exactly the same issue as I was trying to show with my crazy > > -fshort-enum example. > > Two extra hints: > ⬢[acme@toolbox perf-tools-next]$ grep KIND_ENUM64 include/uapi/linux/btf.h > BTF_KIND_ENUM64 = 19, /* Enumeration up to 64-bit values */ > /* BTF_KIND_ENUM64 is followed by multiple "struct btf_enum64". > ⬢[acme@toolbox perf-tools-next]$ > ⬢[acme@toolbox perf-tools-next]$ pahole --help |& grep enum > --skip_encoding_btf_enum64 Do not encode ENUM64s in BTF. > ⬢[acme@toolbox perf-tools-next]$ > :-) Some examples: [root@five perf-tools-next]# bpftool btf dump file /sys/kernel/btf/vmlinux | grep -w ENUM64 [2954] ENUM64 '(anon)' encoding=SIGNED size=8 vlen=28 [6519] ENUM64 'blake2b_iv' encoding=UNSIGNED size=8 vlen=8 [12990] ENUM64 '(anon)' encoding=UNSIGNED size=8 vlen=3 [16283] ENUM64 'netdev_priv_flags' encoding=UNSIGNED size=8 vlen=33 [21717] ENUM64 '(anon)' encoding=UNSIGNED size=8 vlen=11 [28247] ENUM64 'ib_uverbs_device_cap_flags' encoding=UNSIGNED size=8 vlen=28 [34836] ENUM64 'perf_callchain_context' encoding=UNSIGNED size=8 vlen=7 [48851] ENUM64 '(anon)' encoding=UNSIGNED size=8 vlen=9 [54703] ENUM64 'hmm_pfn_flags' encoding=UNSIGNED size=8 vlen=7 [root@five perf-tools-next]# pahole netdev_priv_flags enum netdev_priv_flags { IFF_802_1Q_VLAN = 1, IFF_EBRIDGE = 2, IFF_BONDING = 4, IFF_ISATAP = 8, IFF_WAN_HDLC = 16, IFF_XMIT_DST_RELEASE = 32, IFF_DONT_BRIDGE = 64, IFF_DISABLE_NETPOLL = 128, IFF_MACVLAN_PORT = 256, IFF_BRIDGE_PORT = 512, IFF_OVS_DATAPATH = 1024, IFF_TX_SKB_SHARING = 2048, IFF_UNICAST_FLT = 4096, IFF_TEAM_PORT = 8192, IFF_SUPP_NOFCS = 16384, IFF_LIVE_ADDR_CHANGE = 32768, IFF_MACVLAN = 65536, IFF_XMIT_DST_RELEASE_PERM = 131072, IFF_L3MDEV_MASTER = 262144, IFF_NO_QUEUE = 524288, IFF_OPENVSWITCH = 1048576, IFF_L3MDEV_SLAVE = 2097152, IFF_TEAM = 4194304, IFF_RXFH_CONFIGURED = 8388608, IFF_PHONY_HEADROOM = 16777216, IFF_MACSEC = 33554432, IFF_NO_RX_HANDLER = 67108864, IFF_FAILOVER = 134217728, IFF_FAILOVER_SLAVE = 268435456, IFF_L3MDEV_RX_HANDLER = 536870912, IFF_NO_ADDRCONF = 1073741824, IFF_TX_SKB_NO_LINEAR = 2147483648, IFF_CHANGE_PROTO_DOWN = 4294967296, } [root@five perf-tools-next]# pahole --hex ib_uverbs_device_cap_flags enum ib_uverbs_device_cap_flags { IB_UVERBS_DEVICE_RESIZE_MAX_WR = 0x1, IB_UVERBS_DEVICE_BAD_PKEY_CNTR = 0x2, IB_UVERBS_DEVICE_BAD_QKEY_CNTR = 0x4, IB_UVERBS_DEVICE_RAW_MULTI = 0x8, IB_UVERBS_DEVICE_AUTO_PATH_MIG = 0x10, IB_UVERBS_DEVICE_CHANGE_PHY_PORT = 0x20, IB_UVERBS_DEVICE_UD_AV_PORT_ENFORCE = 0x40, IB_UVERBS_DEVICE_CURR_QP_STATE_MOD = 0x80, IB_UVERBS_DEVICE_SHUTDOWN_PORT = 0x100, IB_UVERBS_DEVICE_PORT_ACTIVE_EVENT = 0x400, IB_UVERBS_DEVICE_SYS_IMAGE_GUID = 0x800, IB_UVERBS_DEVICE_RC_RNR_NAK_GEN = 0x1000, IB_UVERBS_DEVICE_SRQ_RESIZE = 0x2000, IB_UVERBS_DEVICE_N_NOTIFY_CQ = 0x4000, IB_UVERBS_DEVICE_MEM_WINDOW = 0x20000, IB_UVERBS_DEVICE_UD_IP_CSUM = 0x40000, IB_UVERBS_DEVICE_XRC = 0x100000, IB_UVERBS_DEVICE_MEM_MGT_EXTENSIONS = 0x200000, IB_UVERBS_DEVICE_MEM_WINDOW_TYPE_2A = 0x800000, IB_UVERBS_DEVICE_MEM_WINDOW_TYPE_2B = 0x1000000, IB_UVERBS_DEVICE_RC_IP_CSUM = 0x2000000, IB_UVERBS_DEVICE_RAW_IP_CSUM = 0x4000000, IB_UVERBS_DEVICE_MANAGED_FLOW_STEERING = 0x20000000, IB_UVERBS_DEVICE_RAW_SCATTER_FCS = 0x400000000, IB_UVERBS_DEVICE_PCI_WRITE_END_PADDING = 0x1000000000, IB_UVERBS_DEVICE_FLUSH_GLOBAL = 0x4000000000, IB_UVERBS_DEVICE_FLUSH_PERSISTENT = 0x8000000000, IB_UVERBS_DEVICE_ATOMIC_WRITE = 0x10000000000, } [root@five perf-tools-next]#
On Wed, Sep 06, 2023 at 03:28:47PM -0700, Nathan Chancellor wrote: > Hi Kent, > > On Sat, Sep 02, 2023 at 11:25:55PM -0400, Kent Overstreet wrote: > > here's the bcachefs pull request, for 6.6. Hopefully everything > > outstanding from the previous PR thread has been resolved; the block > > layer prereqs are in now via Jens's tree and the dcache helper has a > > reviewed-by from Christain. > > I pulled this into mainline locally and did an LLVM build, which found > an immediate issue. It appears the bcachefs codes uses zero length It looks like this series hasn't been in -next at all? That seems like a pretty important step. Also, when I look at the PR, it seems to be a branch history going back _years_. For this kind of a feature, I'd expect a short series of "here's the code" in incremental additions (e.g. look at the x86 shstk series), not the development history from it being out of tree -- this could easily lead to ugly bisection problems, etc. -Kees
On 9/6/23 8:03 PM, Kees Cook wrote: > On Wed, Sep 06, 2023 at 03:28:47PM -0700, Nathan Chancellor wrote: >> Hi Kent, >> >> On Sat, Sep 02, 2023 at 11:25:55PM -0400, Kent Overstreet wrote: >>> here's the bcachefs pull request, for 6.6. Hopefully everything >>> outstanding from the previous PR thread has been resolved; the block >>> layer prereqs are in now via Jens's tree and the dcache helper has a >>> reviewed-by from Christain. >> >> I pulled this into mainline locally and did an LLVM build, which found >> an immediate issue. It appears the bcachefs codes uses zero length > > It looks like this series hasn't been in -next at all? That seems like a > pretty important step. > > Also, when I look at the PR, it seems to be a branch history going > back _years_. For this kind of a feature, I'd expect a short series of > "here's the code" in incremental additions (e.g. look at the x86 shstk > series), not the development history from it being out of tree -- this > could easily lead to ugly bisection problems, etc. When we merged btrfs, Linus helped redo all of the btrfs out of tree commits on top of kernel git. I can't remember at this point if it was his idea or mine, but git history is obviously improved by remembering my sparse file joy: commit 3a686375629da5d2e2ad019265b66ef113c87455 Author: Chris Mason <chris.mason@oracle.com> Date: Thu May 24 13:35:57 2007 -0400 Btrfs: sparse files! I'd have a preference for keeping the old history, warts and all, but wanted to give a data point to help jog people's memory around problems it might have caused. -chris
On Wed, Sep 06, 2023 at 12:36:18PM -0700, Linus Torvalds wrote: > So I'm starting to look at this because I have most other pull > requests done, and while I realize there's no universal support for it > I suspect any further changes are better done in-tree. The out-of-tree > thing has been done. > > However, while I'll continue to look at it in this form, I just > realized that it's completely unacceptable for one very obvious > reason: > > On Sat, 2 Sept 2023 at 20:26, Kent Overstreet <kent.overstreet@linux.dev> wrote: > > > > https://evilpiepirate.org/git/bcachefs.git bcachefs-for-upstream > > No way am I pulling that without a signed tag and a pgp key with a > chain of trust. You've been around for long enough that having such a > key shouldn't be a problem for you, so make it happen. > > There are a few other issues that I have with this, and Christoph did > mention a big one: it's not been in linux-next. I don't know why I > thought it had been, it's just such an obvious thing for any new "I > want this merged upstream" tree. > > So these kinds of "I'll just ignore _all_ basic rules" kinds of issues > do annoy me. > > I need to know that you understand that if you actually want this > upstream, you need to work with upstream. > > That very much means *NOT* continuing this "I'll just do it my way". > You need to show that you can work with others, that you can work > within the framework of upstream, and that not every single thread you > get into becomes an argument. > > This, btw, is not negotiable. If you feel uncomfortable with that > basic notion, you had better just continue doing development outside > the main kernel tree for another decade. > > The fact that I only now notice that you never submitted this to > linux-next is obviously on me. My bad. > > But at the same time it worries me that it might be a sign of you just > thinking that your way is special. Linus, I specifically asked you about linux-next in the offlist pre-pull request thread back in May. It would have been nice if I could've gotten an answer back then; instead, I'm only getting a definitive answer on that now. That question has even come up in meetings and no one could give a definitive answer; the suggestion was to email you and CC people and ask, which is precisely what I did. Sigh. Now I'm wondering what other surprises I'm going to get the next time around...
On Wed, Sep 06, 2023 at 05:03:06PM -0700, Kees Cook wrote: > On Wed, Sep 06, 2023 at 03:28:47PM -0700, Nathan Chancellor wrote: > > Hi Kent, > > > > On Sat, Sep 02, 2023 at 11:25:55PM -0400, Kent Overstreet wrote: > > > here's the bcachefs pull request, for 6.6. Hopefully everything > > > outstanding from the previous PR thread has been resolved; the block > > > layer prereqs are in now via Jens's tree and the dcache helper has a > > > reviewed-by from Christain. > > > > I pulled this into mainline locally and did an LLVM build, which found > > an immediate issue. It appears the bcachefs codes uses zero length > > It looks like this series hasn't been in -next at all? That seems like a > pretty important step. > > Also, when I look at the PR, it seems to be a branch history going > back _years_. For this kind of a feature, I'd expect a short series of > "here's the code" in incremental additions (e.g. look at the x86 shstk > series), not the development history from it being out of tree -- this > could easily lead to ugly bisection problems, etc. Chris already commented on this, but - we really want the full history available, that's important context for anyone working on the code going forward. Brian was just mentioning digging through the history for context in the meeting last Tuesday, and I've been going to a lot of effort to make sure every commit builds and runs.
On Thu, 7 Sept 2023 at 13:37, Kent Overstreet <kent.overstreet@linux.dev> wrote: > > Linus, I specifically asked you about linux-next in the offlist pre-pull > request thread back in May. It would have been nice if I could've gotten > an answer back then; instead, I'm only getting a definitive answer on > that now. I may not have answered because it was so *obvious*. Was there really any question about it? Anyway, the fact that the code doesn't even build for me is kind of a show-stopper regardless. It's the kind of things linux-next is supposed to notice early, so that we aren't in the situation where the code has not even been build-tested properly. Linus
On Wed, Sep 06, 2023 at 12:36:18PM -0700, Linus Torvalds wrote: > So I'm starting to look at this because I have most other pull > requests done, and while I realize there's no universal support for it > I suspect any further changes are better done in-tree. The out-of-tree > thing has been done. > > However, while I'll continue to look at it in this form, I just > realized that it's completely unacceptable for one very obvious > reason: > > On Sat, 2 Sept 2023 at 20:26, Kent Overstreet <kent.overstreet@linux.dev> wrote: > > > > https://evilpiepirate.org/git/bcachefs.git bcachefs-for-upstream > > No way am I pulling that without a signed tag and a pgp key with a > chain of trust. You've been around for long enough that having such a > key shouldn't be a problem for you, so make it happen. > > There are a few other issues that I have with this, and Christoph did > mention a big one: it's not been in linux-next. I don't know why I > thought it had been, it's just such an obvious thing for any new "I > want this merged upstream" tree. > > So these kinds of "I'll just ignore _all_ basic rules" kinds of issues > do annoy me. > > I need to know that you understand that if you actually want this > upstream, you need to work with upstream. > > That very much means *NOT* continuing this "I'll just do it my way". > You need to show that you can work with others, that you can work > within the framework of upstream, and that not every single thread you > get into becomes an argument. > > This, btw, is not negotiable. If you feel uncomfortable with that > basic notion, you had better just continue doing development outside > the main kernel tree for another decade. > > The fact that I only now notice that you never submitted this to > linux-next is obviously on me. My bad. > > But at the same time it worries me that it might be a sign of you just > thinking that your way is special. > > Linus Honestly, though, this process is getting entirely kafkaesque. I've been spending the past month or two working laying the groundwork for putting together a team to work on this, because god knows we need fresh blood in filesystem land - but that's on hold. Getting blindsided by another three month delay hurts, but that's not even the main thing. The biggest thing has just been the non stop hostility and accusations - everything from "fracturing the community" too "ignoring all the rules" and my favorite, "is this the hill Kent wants to die on?" - when I'm just trying to get work done. I don't generally think of myself as being particularly difficult to work with, I get along fine with most of the filesystem developers I interact with - regularly sharing ideas back and forth with the XFS people - but these review discussions have been entirely dominated by the most divisive people in our community, and I'm being told it's entirely on me to work with the guy whos one constant in the past 15 years has been to try and block everything I submit? I'm just trying to get work done here. I'm not trying to ignore the rules. I'm trying to work with people who are willing to have reasonable discussions. ------------------- When I was a teenager, I wanted nothing more than to be a Linux kernel programmer. I thought it utterly amazing that this huge group of people from around the world were working together over the internet, and that anyone could take part if they had the chops. That was my escape from a shitty family situation and the assholes in my life. But my life is different now; I have new and better people in my life, and I have to be thinking about them, and if merging bcachefs means I have to spend a lot more time in interactions like this then it's going to make me a shitty person to be around; and I don't want to do that to myself and I definitely don't want to do that to the people I care about. I'm going to go offline for awhile and think about what I want to do next.
Hi Kent, hi Linus, hi everyone, Kent Overstreet - 08.09.23, 01:40:01 CEST: > The biggest thing has just been the non stop hostility and accusations - > everything from "fracturing the community" too "ignoring all the rules" > and my favorite, "is this the hill Kent wants to die on?" - when I'm > just trying to get work done. I observed this for a while now, without commenting and found the following pattern on both "sides" of the story: Accusing the other one of wrong-doing. As long as those involved in the merging process continue that pattern that story of not merging bcachefs most likely will continue. And even if it gets merged, there would be ongoing conflict about it. Cause I have no control over how someone else acts. Quite the contrary: The more I expect and require someone else to change the more resistance I am most likely to meet. I only can change how I act. This pattern stops exactly when everyone involved looks at their own part in this repeated and frustrating "bcachefs is not merged to the mainline Linux kernel" dance. And from what I observed the failure to merge it is not caused by a single developer. Neither from you, Kent, neither from anyone else. It is the combination of the single actions of several developers and the social interaction between them that caused the failure to merge it so far. Accusing the other one is giving all the power to change the situation to someone else. I am sure merging it will work when everyone involved first looks at themselves and asks themselves the questions "Have I contributed to make merging bcachefs difficult and if so how and most importantly how can I act more constructive about it?". And I mean that for the developers who have been skeptical about the merge as well as the supportive developers including Kent. There have been actions on both "sides" that contributed to delay a merge. I am not going to make a list but leave it to everyone involved to consider themselves what those were. For the recent requests of having it GPG signed as well as having it go through next: I think those requests are reasonable. As far as I read bcache back then went through next as well. Would it have been nice to have been told that earlier? Yes. But both of those requests are certainly not a show-stopper to have bcachefs merged at a later time. Of course I know I have been asking others to go within and consider their own behavior in this mail while being perfectly aware that I cannot change how anyone else acts. However, maybe it is an inspiration to some to decide for themselves to consider a change. In the best hopes to see bcachefs being merged to the "official" Linux kernel soon,
On 9/8/23 00:40, Kent Overstreet wrote: > On Wed, Sep 06, 2023 at 12:36:18PM -0700, Linus Torvalds wrote: >> So I'm starting to look at this because I have most other pull >> requests done, and while I realize there's no universal support for it >> I suspect any further changes are better done in-tree. The out-of-tree >> thing has been done. >> >> However, while I'll continue to look at it in this form, I just >> realized that it's completely unacceptable for one very obvious >> reason: >> >> On Sat, 2 Sept 2023 at 20:26, Kent Overstreet <kent.overstreet@linux.dev> wrote: >>> >>> https://evilpiepirate.org/git/bcachefs.git bcachefs-for-upstream >> >> No way am I pulling that without a signed tag and a pgp key with a >> chain of trust. You've been around for long enough that having such a >> key shouldn't be a problem for you, so make it happen. >> >> There are a few other issues that I have with this, and Christoph did >> mention a big one: it's not been in linux-next. I don't know why I >> thought it had been, it's just such an obvious thing for any new "I >> want this merged upstream" tree. >> >> So these kinds of "I'll just ignore _all_ basic rules" kinds of issues >> do annoy me. >> >> I need to know that you understand that if you actually want this >> upstream, you need to work with upstream. >> >> That very much means *NOT* continuing this "I'll just do it my way". >> You need to show that you can work with others, that you can work >> within the framework of upstream, and that not every single thread you >> get into becomes an argument. >> >> This, btw, is not negotiable. If you feel uncomfortable with that >> basic notion, you had better just continue doing development outside >> the main kernel tree for another decade. >> >> The fact that I only now notice that you never submitted this to >> linux-next is obviously on me. My bad. >> >> But at the same time it worries me that it might be a sign of you just >> thinking that your way is special. >> >> Linus > > Honestly, though, this process is getting entirely kafkaesque. > > I've been spending the past month or two working laying the groundwork > for putting together a team to work on this, because god knows we need > fresh blood in filesystem land - but that's on hold. Getting blindsided > by another three month delay hurts, but that's not even the main thing. > > The biggest thing has just been the non stop hostility and accusations - > everything from "fracturing the community" too "ignoring all the rules" > and my favorite, "is this the hill Kent wants to die on?" - when I'm > just trying to get work done. > > I don't generally think of myself as being particularly difficult to > work with, I get along fine with most of the filesystem developers I > interact with - regularly sharing ideas back and forth with the XFS > people - but these review discussions have been entirely dominated by > the most divisive people in our community, and I'm being told it's > entirely on me to work with the guy whos one constant in the past 15 > years has been to try and block everything I submit? > > I'm just trying to get work done here. I'm not trying to ignore the > rules. I'm trying to work with people who are willing to have reasonable > discussions. > > ------------------- > > When I was a teenager, I wanted nothing more than to be a Linux kernel > programmer. I thought it utterly amazing that this huge group of people > from around the world were working together over the internet, and that > anyone could take part if they had the chops. > > That was my escape from a shitty family situation and the assholes in my > life. > > But my life is different now; I have new and better people in my life, > and I have to be thinking about them, and if merging bcachefs means I > have to spend a lot more time in interactions like this then it's going > to make me a shitty person to be around; and I don't want to do that to > myself and I definitely don't want to do that to the people I care > about. > > I'm going to go offline for awhile and think about what I want to do > next. I've been holding off replying here for a while because I really hoped that this situation would just work itself out. (I apologise for adding more noise in advance) I agree that it really sucks that sometimes you don't get replies to things sometimes or the review from the people you need it from all the time, or didn't tell you something you needed to know. But, I think it's really important though to realize that you are talking to other people on the ML and not review machines (unless that person is Dave Airlie on Zink ;P) and very often, other work can come up that would block them being able to spend time reviewing or guiding you on this process. Everyone on here is another person who has their own huuuge slog of work that is super important for security, stability, shipping a product/feature, keeping their job, etc. Eg. I proposed several revisions on the casefolding support for bcachefs, but right now I am busy doing some other AMDGPU and Gamescope/Proton + color work so I haven't had a chance to follow up more on that since the last discussion. You might think that because X takes a while to respond/review or a didn't mention that you actually needed to do Y or missed your meeting; it's because they don't care, but it's probably way more likely that they are just busy and going through their own personal hell. One of the harsh things about open source is rationalizing that nobody owes you a review or any of their time. If people are willing to review your features and changes in any capacity, then they also have an interest in your project. If you can understand that, then you are going to have a much better time proposing things upstream. I also really want to see bcachefs in mainline, and I know you can do it. :-) Cheers - Joshie
On Tue, Sep 05, 2023 at 08:00:07PM -0400, Kent Overstreet wrote: > But the reasons bcachefs doesn't use iomap have been discussed at > length, Care to send a pointer? > that code. You were AWOL on those discussions; you consistently say > "bcachefs should use iomap" and then walk away, so those discussions > haven't moved forwards. Well, let's hsave the proper discussion then. I defintively don't think "I just walked away", because that's not what I do. I did give up on quite a few of discussions with your because they turned from technical into personal very quick, but I don't even remember that for this case. > To recap, besides being more iterator like (passing data structures > around with iterators, Which is something you can do with iomap, and we're making use of that in quite a few places. Thanks to the work from willy that I helped a bit with this has been done years ago. It might or might not make sense to replace iomap_begin/end with a single iter callback that can be inlined, but that's not changing anything fundamental. > bcachefs also hangs a bit more state off the pagecache, due to being > multi device and also using the same data structure for tracking disk > reservations (because why make the buffered write paths look that up > separately?). I'm not even parsing the sentence. But as said many times I'm all ear if we stick to technical questions. We've added all kinds of improvements to iomap especially for gfs2 and btrfs, and while it sometimes took some time we're all better off with that as we're converging on a model of doing I/O instead of everytone doing something slightly different. > > > But without that, and without being in linux-next and the VFS maintainer > > ACK required for I think this is a very bad idea. > > Christain gave his reviewed-by for the dcache patch. Since this patchset > doesn't change existing code maintained by others aside from that one > patch, not sure how linux-next is relevant here... Because file systems really should have a VFS maintainer ACK, just like block drivers have a block maintainer ACK, or network drivers have a net maintainer ACK. Doing this differently for the most complex driver interface in the kernel doesn't make any sense whatsover.
On Thu, Sep 07, 2023 at 04:39:46PM -0400, Kent Overstreet wrote: > On Wed, Sep 06, 2023 at 05:03:06PM -0700, Kees Cook wrote: > > On Wed, Sep 06, 2023 at 03:28:47PM -0700, Nathan Chancellor wrote: > > > Hi Kent, > > > > > > On Sat, Sep 02, 2023 at 11:25:55PM -0400, Kent Overstreet wrote: > > > > here's the bcachefs pull request, for 6.6. Hopefully everything > > > > outstanding from the previous PR thread has been resolved; the block > > > > layer prereqs are in now via Jens's tree and the dcache helper has a > > > > reviewed-by from Christain. > > > > > > I pulled this into mainline locally and did an LLVM build, which found > > > an immediate issue. It appears the bcachefs codes uses zero length > > > > It looks like this series hasn't been in -next at all? That seems like a > > pretty important step. > > > > Also, when I look at the PR, it seems to be a branch history going > > back _years_. For this kind of a feature, I'd expect a short series of > > "here's the code" in incremental additions (e.g. look at the x86 shstk > > series), not the development history from it being out of tree -- this > > could easily lead to ugly bisection problems, etc. > > Chris already commented on this, but - we really want the full history > available, that's important context for anyone working on the code going > forward. Brian was just mentioning digging through the history for > context in the meeting last Tuesday, and I've been going to a lot of > effort to make sure every commit builds and runs. > Yeah.. IMO the main advantages of a sort of squashed down/sanitized git history is to either aid in code review or just clean up a history that is aesthetically a mess. For the former, the consensus seems to be that no one person is going to sit down and review the entire codebase, but rather folks have been digging into peripheral areas they have experience in (i.e., locking, pagecache, etc.) to call out any major concerns. I believe Kent has also offered to give pointers or just sit down with anybody who needs assistance to navigate the codebase for review purposes. For the latter, ISTM that bcachefs has pretty much followed kernel patch conventions, with it being originally derived from another upstream kernel subsystem and whatnot. The flipside is that losing the history makes it incrementally more annoying for developers working on bcachefs going forward. So I can see an argument for doing things either way in general just depending on context, but it looks like there's precedent for either approach. Looking back at btrfs in v2.6.29, that looks like a ~900 or so commit history that was pulled in. bcachefs has a larger commit log (~2500+) at this point, but if we can do whatever magic Chris referred to to try and avoid any logistical issues for the broader kernel community, I think that would be ideal. BTW this is just my .02 of course, but I'm also fairly certain at least one or two developers have looked at the git log and expressed the exact opposite opinion expressed here: that seeing an upstream-like history is appreciated because it reflects a sane/compatible development process. That again isn't to say one way or the other is the right approach for a merge, just that it seems subjective to some degree and so inevitably there will be different opinions.. Brian
To all kernel developers.
Kent Overstreet - 03.09.23, 05:25:55 CEST:
> Hi Linus,
[…]
Sometimes it is all too easy to forget saying thank you!
Thank you to all of you for your work on the Linux kernel.
I greatly appreciate it.
Except for some older and one (almost insanely nice) newer device that run
AmigaOS or variants of that operating system, all my computing devices
including router and phone run a Linux kernel. And then considering the
huge amount of Linux servers that actually power most of what we call the
internet and its services… awesome!
That would not have been possible without your work!
So: Thank you!
Best,
On Wed, Sep 06, 2023 at 05:03:06PM -0700, Kees Cook wrote: > On Wed, Sep 06, 2023 at 03:28:47PM -0700, Nathan Chancellor wrote: > > Hi Kent, > > > > On Sat, Sep 02, 2023 at 11:25:55PM -0400, Kent Overstreet wrote: > > > here's the bcachefs pull request, for 6.6. Hopefully everything > > > outstanding from the previous PR thread has been resolved; the block > > > layer prereqs are in now via Jens's tree and the dcache helper has a > > > reviewed-by from Christain. > > > > I pulled this into mainline locally and did an LLVM build, which found > > an immediate issue. It appears the bcachefs codes uses zero length > > It looks like this series hasn't been in -next at all? That seems like a > pretty important step. > > Also, when I look at the PR, it seems to be a branch history going > back _years_. For this kind of a feature, I'd expect a short series of > "here's the code" in incremental additions (e.g. look at the x86 shstk > series), not the development history from it being out of tree -- this > could easily lead to ugly bisection problems, etc. I disagree entirely. One of the most valuable things we have for XFS is a complete change history going back to the first commit in 1993. We don't have all that in the kernel git tree - that only goes back to 2005. We do, however, have a separate git archive that runs from the initial commit in 1993 to 2008, and so we can track bugs and changes back all the way to when they were first introduced. I'm looking at something in that historic XFS archive every second day; understanding the XFS code and why it is like it is requires looking back in time on a regular basis. And because XFS engineers have been really good with commit messages for a really long time, there is a lot of information in that historic archive that is simply not documented anywhere else. So if we have the full history of bcachefs developement sitting in a git tree, we'd be *utterly stupid* to discard it when merging it into the mainline tree. Nothing else documents the reasons for the code being like it is right now better than the change history of the code. For people trying to understand the code or trying to perform root cause analysis of a bug, a complete revision history is a rich gold mine full of valuable nuggets.... -Dave.
On Wed, Sep 06, 2023 at 01:20:59PM -0700, Linus Torvalds wrote: > On Wed, 6 Sept 2023 at 13:02, Linus Torvalds > <torvalds@linux-foundation.org> wrote: > > > > And guess what happens when you have (unsigned char)-1? It does *not* > > cast back to -1. > > Side note: again, this may be one of those "it works in practice", > because if we have -fshort-enums, I think 'enum > btree_node_locked_type' in turn ends up being represented as a 'signed > char', because that's the smallest simple type that can fit all those > values. > > I don't think gcc ever uses less than that (ie while a six_lock_type > could fit in two bits, it's still going to be considered at least a > 8-bit value in practice). > > So we may have 'enum six_lock_type' essentially being 'unsigned char', > and when the code does > > mark_btree_node_locked(trans, path, 0, BTREE_NODE_UNLOCKED); > > that BTREE_NODE_UNLOCKED value might actually be 255. > > And then when it's cast to 'enum btree_node_locked_type' in the inline > function, the 255 will be cast to 'signed char', and we'll end up > compatible with '(enum btree_node_locked_type)-1' again. > > So it's one of those things that are seriously wrong to do, but might > generate the expected code anyway. > > Unless the compiler adds any other sanity checks, like UBSAN or > something, that actually uses the exact range of the enums. > > It could happen even without UBSAN, if the compiler ends up going "I > can see that the original value came from a 'enum six_lock_type', so I > know the original value can't be signed, so any comparison with > BTREE_NODE_UNLOCKED can never be true. > > But again, I suspect that in practice this all just happens to work. > That doesn't make it right. No, this was just broken - it should have been mark_btree_node_unlocked(), we never should've been passing that enum val there.
Hi Linus, here's the bcachefs pull request, for 6.6. Hopefully everything outstanding from the previous PR thread has been resolved; the block layer prereqs are in now via Jens's tree and the dcache helper has a reviewed-by from Christain. Since the last 6.5 PR, it's now marked as EXPERIMENTAL; there's also been a bunch of on disk forwards compatibility work. Previous PR thread... https://lore.kernel.org/all/20230626214656.hcp4puionmtoloat@moria.home.lan/ As before, the list of patches is just from the bcachefs-prereqs branch, the diffstat is for the entire pull. Test results are up here: https://evilpiepirate.org/~testdashboard/ci?branch=bcachefs-for-upstream Cheers, Kent ---------------------------------------------------------------- The following changes since commit b97d64c722598ffed42ece814a2cb791336c6679: Merge tag '6.6-rc-smb3-client-fixes-part1' of git://git.samba.org/sfrench/cifs-2.6 (2023-08-30 21:01:40 -0700) are available in the Git repository at: https://evilpiepirate.org/git/bcachefs.git bcachefs-for-upstream for you to fetch changes up to df0bfb05a6b3817b1fb5ea4d80514681e86fe702: bcachefs: Fix snapshot_skiplist_good() (2023-09-02 15:09:04 -0400) ---------------------------------------------------------------- Brian Foster (1): locking: export contention tracepoints for bcachefs six locks Christopher James Halse Rogers (1): stacktrace: Export stack_trace_save_tsk Kent Overstreet (13): sched: Add task_struct->faults_disabled_mapping fs: factor out d_mark_tmpfile() lib/string_helpers: string_get_size() now returns characters wrote lib: Export errname locking/osq: Export osq_(lock|unlock) bcache: move closures to lib/ MAINTAINERS: Add entry for closures closures: closure_wait_event() closures: closure_nr_remaining() closures: Add a missing include MAINTAINERS: Add entry for generic-radix-tree lib/generic-radix-tree.c: Don't overflow in peek() lib/generic-radix-tree.c: Add peek_prev() MAINTAINERS | 23 + drivers/md/bcache/Kconfig | 10 +- drivers/md/bcache/Makefile | 4 +- drivers/md/bcache/bcache.h | 2 +- drivers/md/bcache/super.c | 1 - drivers/md/bcache/util.h | 3 +- fs/Kconfig | 1 + fs/Makefile | 1 + fs/bcachefs/Kconfig | 85 + fs/bcachefs/Makefile | 85 + fs/bcachefs/acl.c | 412 +++ fs/bcachefs/acl.h | 58 + fs/bcachefs/alloc_background.c | 2157 +++++++++++++++ fs/bcachefs/alloc_background.h | 257 ++ fs/bcachefs/alloc_foreground.c | 1571 +++++++++++ fs/bcachefs/alloc_foreground.h | 224 ++ fs/bcachefs/alloc_types.h | 126 + fs/bcachefs/backpointers.c | 873 +++++++ fs/bcachefs/backpointers.h | 131 + fs/bcachefs/bbpos.h | 48 + fs/bcachefs/bcachefs.h | 1146 ++++++++ fs/bcachefs/bcachefs_format.h | 2368 +++++++++++++++++ fs/bcachefs/bcachefs_ioctl.h | 368 +++ fs/bcachefs/bkey.c | 1107 ++++++++ fs/bcachefs/bkey.h | 782 ++++++ fs/bcachefs/bkey_buf.h | 61 + fs/bcachefs/bkey_cmp.h | 129 + fs/bcachefs/bkey_methods.c | 456 ++++ fs/bcachefs/bkey_methods.h | 188 ++ fs/bcachefs/bkey_sort.c | 201 ++ fs/bcachefs/bkey_sort.h | 44 + fs/bcachefs/bset.c | 1587 +++++++++++ fs/bcachefs/bset.h | 541 ++++ fs/bcachefs/btree_cache.c | 1213 +++++++++ fs/bcachefs/btree_cache.h | 130 + fs/bcachefs/btree_gc.c | 2124 +++++++++++++++ fs/bcachefs/btree_gc.h | 114 + fs/bcachefs/btree_io.c | 2245 ++++++++++++++++ fs/bcachefs/btree_io.h | 228 ++ fs/bcachefs/btree_iter.c | 3192 +++++++++++++++++++++++ fs/bcachefs/btree_iter.h | 940 +++++++ fs/bcachefs/btree_journal_iter.c | 531 ++++ fs/bcachefs/btree_journal_iter.h | 57 + fs/bcachefs/btree_key_cache.c | 1074 ++++++++ fs/bcachefs/btree_key_cache.h | 48 + fs/bcachefs/btree_locking.c | 791 ++++++ fs/bcachefs/btree_locking.h | 423 +++ fs/bcachefs/btree_trans_commit.c | 1156 ++++++++ fs/bcachefs/btree_types.h | 739 ++++++ fs/bcachefs/btree_update.c | 898 +++++++ fs/bcachefs/btree_update.h | 353 +++ fs/bcachefs/btree_update_interior.c | 2488 ++++++++++++++++++ fs/bcachefs/btree_update_interior.h | 337 +++ fs/bcachefs/btree_write_buffer.c | 375 +++ fs/bcachefs/btree_write_buffer.h | 14 + fs/bcachefs/btree_write_buffer_types.h | 44 + fs/bcachefs/buckets.c | 2107 +++++++++++++++ fs/bcachefs/buckets.h | 413 +++ fs/bcachefs/buckets_types.h | 92 + fs/bcachefs/buckets_waiting_for_journal.c | 166 ++ fs/bcachefs/buckets_waiting_for_journal.h | 15 + fs/bcachefs/buckets_waiting_for_journal_types.h | 23 + fs/bcachefs/chardev.c | 769 ++++++ fs/bcachefs/chardev.h | 31 + fs/bcachefs/checksum.c | 753 ++++++ fs/bcachefs/checksum.h | 211 ++ fs/bcachefs/clock.c | 193 ++ fs/bcachefs/clock.h | 38 + fs/bcachefs/clock_types.h | 37 + fs/bcachefs/compress.c | 714 +++++ fs/bcachefs/compress.h | 55 + fs/bcachefs/counters.c | 107 + fs/bcachefs/counters.h | 17 + fs/bcachefs/darray.h | 87 + fs/bcachefs/data_update.c | 562 ++++ fs/bcachefs/data_update.h | 43 + fs/bcachefs/debug.c | 957 +++++++ fs/bcachefs/debug.h | 32 + fs/bcachefs/dirent.c | 590 +++++ fs/bcachefs/dirent.h | 70 + fs/bcachefs/disk_groups.c | 556 ++++ fs/bcachefs/disk_groups.h | 106 + fs/bcachefs/ec.c | 1972 ++++++++++++++ fs/bcachefs/ec.h | 260 ++ fs/bcachefs/ec_types.h | 41 + fs/bcachefs/errcode.c | 63 + fs/bcachefs/errcode.h | 252 ++ fs/bcachefs/error.c | 294 +++ fs/bcachefs/error.h | 206 ++ fs/bcachefs/extent_update.c | 173 ++ fs/bcachefs/extent_update.h | 12 + fs/bcachefs/extents.c | 1403 ++++++++++ fs/bcachefs/extents.h | 757 ++++++ fs/bcachefs/extents_types.h | 40 + fs/bcachefs/eytzinger.h | 281 ++ fs/bcachefs/fifo.h | 127 + fs/bcachefs/fs-common.c | 501 ++++ fs/bcachefs/fs-common.h | 43 + fs/bcachefs/fs-io-buffered.c | 1099 ++++++++ fs/bcachefs/fs-io-buffered.h | 27 + fs/bcachefs/fs-io-direct.c | 679 +++++ fs/bcachefs/fs-io-direct.h | 16 + fs/bcachefs/fs-io-pagecache.c | 788 ++++++ fs/bcachefs/fs-io-pagecache.h | 176 ++ fs/bcachefs/fs-io.c | 1250 +++++++++ fs/bcachefs/fs-io.h | 184 ++ fs/bcachefs/fs-ioctl.c | 559 ++++ fs/bcachefs/fs-ioctl.h | 81 + fs/bcachefs/fs.c | 1961 ++++++++++++++ fs/bcachefs/fs.h | 209 ++ fs/bcachefs/fsck.c | 2483 ++++++++++++++++++ fs/bcachefs/fsck.h | 14 + fs/bcachefs/inode.c | 1111 ++++++++ fs/bcachefs/inode.h | 204 ++ fs/bcachefs/io.c | 3051 ++++++++++++++++++++++ fs/bcachefs/io.h | 202 ++ fs/bcachefs/io_types.h | 165 ++ fs/bcachefs/journal.c | 1438 ++++++++++ fs/bcachefs/journal.h | 526 ++++ fs/bcachefs/journal_io.c | 1888 ++++++++++++++ fs/bcachefs/journal_io.h | 65 + fs/bcachefs/journal_reclaim.c | 874 +++++++ fs/bcachefs/journal_reclaim.h | 86 + fs/bcachefs/journal_sb.c | 219 ++ fs/bcachefs/journal_sb.h | 24 + fs/bcachefs/journal_seq_blacklist.c | 322 +++ fs/bcachefs/journal_seq_blacklist.h | 22 + fs/bcachefs/journal_types.h | 345 +++ fs/bcachefs/keylist.c | 52 + fs/bcachefs/keylist.h | 74 + fs/bcachefs/keylist_types.h | 16 + fs/bcachefs/lru.c | 162 ++ fs/bcachefs/lru.h | 69 + fs/bcachefs/mean_and_variance.c | 159 ++ fs/bcachefs/mean_and_variance.h | 198 ++ fs/bcachefs/mean_and_variance_test.c | 240 ++ fs/bcachefs/migrate.c | 182 ++ fs/bcachefs/migrate.h | 7 + fs/bcachefs/move.c | 1162 +++++++++ fs/bcachefs/move.h | 95 + fs/bcachefs/move_types.h | 36 + fs/bcachefs/movinggc.c | 423 +++ fs/bcachefs/movinggc.h | 12 + fs/bcachefs/nocow_locking.c | 123 + fs/bcachefs/nocow_locking.h | 49 + fs/bcachefs/nocow_locking_types.h | 20 + fs/bcachefs/opts.c | 599 +++++ fs/bcachefs/opts.h | 563 ++++ fs/bcachefs/printbuf.c | 415 +++ fs/bcachefs/printbuf.h | 284 ++ fs/bcachefs/quota.c | 981 +++++++ fs/bcachefs/quota.h | 74 + fs/bcachefs/quota_types.h | 43 + fs/bcachefs/rebalance.c | 368 +++ fs/bcachefs/rebalance.h | 28 + fs/bcachefs/rebalance_types.h | 26 + fs/bcachefs/recovery.c | 1057 ++++++++ fs/bcachefs/recovery.h | 33 + fs/bcachefs/recovery_types.h | 48 + fs/bcachefs/reflink.c | 399 +++ fs/bcachefs/reflink.h | 81 + fs/bcachefs/replicas.c | 1059 ++++++++ fs/bcachefs/replicas.h | 91 + fs/bcachefs/replicas_types.h | 27 + fs/bcachefs/sb-clean.c | 395 +++ fs/bcachefs/sb-clean.h | 16 + fs/bcachefs/sb-members.c | 173 ++ fs/bcachefs/sb-members.h | 176 ++ fs/bcachefs/seqmutex.h | 48 + fs/bcachefs/siphash.c | 173 ++ fs/bcachefs/siphash.h | 87 + fs/bcachefs/six.c | 914 +++++++ fs/bcachefs/six.h | 388 +++ fs/bcachefs/snapshot.c | 1687 ++++++++++++ fs/bcachefs/snapshot.h | 272 ++ fs/bcachefs/str_hash.h | 370 +++ fs/bcachefs/subvolume.c | 451 ++++ fs/bcachefs/subvolume.h | 35 + fs/bcachefs/subvolume_types.h | 31 + fs/bcachefs/super-io.c | 1265 +++++++++ fs/bcachefs/super-io.h | 133 + fs/bcachefs/super.c | 2015 ++++++++++++++ fs/bcachefs/super.h | 52 + fs/bcachefs/super_types.h | 52 + fs/bcachefs/sysfs.c | 1059 ++++++++ fs/bcachefs/sysfs.h | 48 + fs/bcachefs/tests.c | 970 +++++++ fs/bcachefs/tests.h | 15 + fs/bcachefs/trace.c | 16 + fs/bcachefs/trace.h | 1265 +++++++++ fs/bcachefs/two_state_shared_lock.c | 8 + fs/bcachefs/two_state_shared_lock.h | 59 + fs/bcachefs/util.c | 1144 ++++++++ fs/bcachefs/util.h | 852 ++++++ fs/bcachefs/varint.c | 123 + fs/bcachefs/varint.h | 11 + fs/bcachefs/vstructs.h | 63 + fs/bcachefs/xattr.c | 649 +++++ fs/bcachefs/xattr.h | 50 + fs/dcache.c | 12 +- {drivers/md/bcache => include/linux}/closure.h | 46 +- include/linux/dcache.h | 1 + include/linux/exportfs.h | 6 + include/linux/generic-radix-tree.h | 68 +- include/linux/sched.h | 1 + include/linux/string_helpers.h | 4 +- init/init_task.c | 1 + kernel/locking/mutex.c | 3 + kernel/locking/osq_lock.c | 2 + kernel/stacktrace.c | 2 + lib/Kconfig | 3 + lib/Kconfig.debug | 9 + lib/Makefile | 2 + {drivers/md/bcache => lib}/closure.c | 36 +- lib/errname.c | 1 + lib/generic-radix-tree.c | 76 +- lib/string_helpers.c | 10 +- 217 files changed, 94348 insertions(+), 56 deletions(-) create mode 100644 fs/bcachefs/Kconfig create mode 100644 fs/bcachefs/Makefile create mode 100644 fs/bcachefs/acl.c create mode 100644 fs/bcachefs/acl.h create mode 100644 fs/bcachefs/alloc_background.c create mode 100644 fs/bcachefs/alloc_background.h create mode 100644 fs/bcachefs/alloc_foreground.c create mode 100644 fs/bcachefs/alloc_foreground.h create mode 100644 fs/bcachefs/alloc_types.h create mode 100644 fs/bcachefs/backpointers.c create mode 100644 fs/bcachefs/backpointers.h create mode 100644 fs/bcachefs/bbpos.h create mode 100644 fs/bcachefs/bcachefs.h create mode 100644 fs/bcachefs/bcachefs_format.h create mode 100644 fs/bcachefs/bcachefs_ioctl.h create mode 100644 fs/bcachefs/bkey.c create mode 100644 fs/bcachefs/bkey.h create mode 100644 fs/bcachefs/bkey_buf.h create mode 100644 fs/bcachefs/bkey_cmp.h create mode 100644 fs/bcachefs/bkey_methods.c create mode 100644 fs/bcachefs/bkey_methods.h create mode 100644 fs/bcachefs/bkey_sort.c create mode 100644 fs/bcachefs/bkey_sort.h create mode 100644 fs/bcachefs/bset.c create mode 100644 fs/bcachefs/bset.h create mode 100644 fs/bcachefs/btree_cache.c create mode 100644 fs/bcachefs/btree_cache.h create mode 100644 fs/bcachefs/btree_gc.c create mode 100644 fs/bcachefs/btree_gc.h create mode 100644 fs/bcachefs/btree_io.c create mode 100644 fs/bcachefs/btree_io.h create mode 100644 fs/bcachefs/btree_iter.c create mode 100644 fs/bcachefs/btree_iter.h create mode 100644 fs/bcachefs/btree_journal_iter.c create mode 100644 fs/bcachefs/btree_journal_iter.h create mode 100644 fs/bcachefs/btree_key_cache.c create mode 100644 fs/bcachefs/btree_key_cache.h create mode 100644 fs/bcachefs/btree_locking.c create mode 100644 fs/bcachefs/btree_locking.h create mode 100644 fs/bcachefs/btree_trans_commit.c create mode 100644 fs/bcachefs/btree_types.h create mode 100644 fs/bcachefs/btree_update.c create mode 100644 fs/bcachefs/btree_update.h create mode 100644 fs/bcachefs/btree_update_interior.c create mode 100644 fs/bcachefs/btree_update_interior.h create mode 100644 fs/bcachefs/btree_write_buffer.c create mode 100644 fs/bcachefs/btree_write_buffer.h create mode 100644 fs/bcachefs/btree_write_buffer_types.h create mode 100644 fs/bcachefs/buckets.c create mode 100644 fs/bcachefs/buckets.h create mode 100644 fs/bcachefs/buckets_types.h create mode 100644 fs/bcachefs/buckets_waiting_for_journal.c create mode 100644 fs/bcachefs/buckets_waiting_for_journal.h create mode 100644 fs/bcachefs/buckets_waiting_for_journal_types.h create mode 100644 fs/bcachefs/chardev.c create mode 100644 fs/bcachefs/chardev.h create mode 100644 fs/bcachefs/checksum.c create mode 100644 fs/bcachefs/checksum.h create mode 100644 fs/bcachefs/clock.c create mode 100644 fs/bcachefs/clock.h create mode 100644 fs/bcachefs/clock_types.h create mode 100644 fs/bcachefs/compress.c create mode 100644 fs/bcachefs/compress.h create mode 100644 fs/bcachefs/counters.c create mode 100644 fs/bcachefs/counters.h create mode 100644 fs/bcachefs/darray.h create mode 100644 fs/bcachefs/data_update.c create mode 100644 fs/bcachefs/data_update.h create mode 100644 fs/bcachefs/debug.c create mode 100644 fs/bcachefs/debug.h create mode 100644 fs/bcachefs/dirent.c create mode 100644 fs/bcachefs/dirent.h create mode 100644 fs/bcachefs/disk_groups.c create mode 100644 fs/bcachefs/disk_groups.h create mode 100644 fs/bcachefs/ec.c create mode 100644 fs/bcachefs/ec.h create mode 100644 fs/bcachefs/ec_types.h create mode 100644 fs/bcachefs/errcode.c create mode 100644 fs/bcachefs/errcode.h create mode 100644 fs/bcachefs/error.c create mode 100644 fs/bcachefs/error.h create mode 100644 fs/bcachefs/extent_update.c create mode 100644 fs/bcachefs/extent_update.h create mode 100644 fs/bcachefs/extents.c create mode 100644 fs/bcachefs/extents.h create mode 100644 fs/bcachefs/extents_types.h create mode 100644 fs/bcachefs/eytzinger.h create mode 100644 fs/bcachefs/fifo.h create mode 100644 fs/bcachefs/fs-common.c create mode 100644 fs/bcachefs/fs-common.h create mode 100644 fs/bcachefs/fs-io-buffered.c create mode 100644 fs/bcachefs/fs-io-buffered.h create mode 100644 fs/bcachefs/fs-io-direct.c create mode 100644 fs/bcachefs/fs-io-direct.h create mode 100644 fs/bcachefs/fs-io-pagecache.c create mode 100644 fs/bcachefs/fs-io-pagecache.h create mode 100644 fs/bcachefs/fs-io.c create mode 100644 fs/bcachefs/fs-io.h create mode 100644 fs/bcachefs/fs-ioctl.c create mode 100644 fs/bcachefs/fs-ioctl.h create mode 100644 fs/bcachefs/fs.c create mode 100644 fs/bcachefs/fs.h create mode 100644 fs/bcachefs/fsck.c create mode 100644 fs/bcachefs/fsck.h create mode 100644 fs/bcachefs/inode.c create mode 100644 fs/bcachefs/inode.h create mode 100644 fs/bcachefs/io.c create mode 100644 fs/bcachefs/io.h create mode 100644 fs/bcachefs/io_types.h create mode 100644 fs/bcachefs/journal.c create mode 100644 fs/bcachefs/journal.h create mode 100644 fs/bcachefs/journal_io.c create mode 100644 fs/bcachefs/journal_io.h create mode 100644 fs/bcachefs/journal_reclaim.c create mode 100644 fs/bcachefs/journal_reclaim.h create mode 100644 fs/bcachefs/journal_sb.c create mode 100644 fs/bcachefs/journal_sb.h create mode 100644 fs/bcachefs/journal_seq_blacklist.c create mode 100644 fs/bcachefs/journal_seq_blacklist.h create mode 100644 fs/bcachefs/journal_types.h create mode 100644 fs/bcachefs/keylist.c create mode 100644 fs/bcachefs/keylist.h create mode 100644 fs/bcachefs/keylist_types.h create mode 100644 fs/bcachefs/lru.c create mode 100644 fs/bcachefs/lru.h create mode 100644 fs/bcachefs/mean_and_variance.c create mode 100644 fs/bcachefs/mean_and_variance.h create mode 100644 fs/bcachefs/mean_and_variance_test.c create mode 100644 fs/bcachefs/migrate.c create mode 100644 fs/bcachefs/migrate.h create mode 100644 fs/bcachefs/move.c create mode 100644 fs/bcachefs/move.h create mode 100644 fs/bcachefs/move_types.h create mode 100644 fs/bcachefs/movinggc.c create mode 100644 fs/bcachefs/movinggc.h create mode 100644 fs/bcachefs/nocow_locking.c create mode 100644 fs/bcachefs/nocow_locking.h create mode 100644 fs/bcachefs/nocow_locking_types.h create mode 100644 fs/bcachefs/opts.c create mode 100644 fs/bcachefs/opts.h create mode 100644 fs/bcachefs/printbuf.c create mode 100644 fs/bcachefs/printbuf.h create mode 100644 fs/bcachefs/quota.c create mode 100644 fs/bcachefs/quota.h create mode 100644 fs/bcachefs/quota_types.h create mode 100644 fs/bcachefs/rebalance.c create mode 100644 fs/bcachefs/rebalance.h create mode 100644 fs/bcachefs/rebalance_types.h create mode 100644 fs/bcachefs/recovery.c create mode 100644 fs/bcachefs/recovery.h create mode 100644 fs/bcachefs/recovery_types.h create mode 100644 fs/bcachefs/reflink.c create mode 100644 fs/bcachefs/reflink.h create mode 100644 fs/bcachefs/replicas.c create mode 100644 fs/bcachefs/replicas.h create mode 100644 fs/bcachefs/replicas_types.h create mode 100644 fs/bcachefs/sb-clean.c create mode 100644 fs/bcachefs/sb-clean.h create mode 100644 fs/bcachefs/sb-members.c create mode 100644 fs/bcachefs/sb-members.h create mode 100644 fs/bcachefs/seqmutex.h create mode 100644 fs/bcachefs/siphash.c create mode 100644 fs/bcachefs/siphash.h create mode 100644 fs/bcachefs/six.c create mode 100644 fs/bcachefs/six.h create mode 100644 fs/bcachefs/snapshot.c create mode 100644 fs/bcachefs/snapshot.h create mode 100644 fs/bcachefs/str_hash.h create mode 100644 fs/bcachefs/subvolume.c create mode 100644 fs/bcachefs/subvolume.h create mode 100644 fs/bcachefs/subvolume_types.h create mode 100644 fs/bcachefs/super-io.c create mode 100644 fs/bcachefs/super-io.h create mode 100644 fs/bcachefs/super.c create mode 100644 fs/bcachefs/super.h create mode 100644 fs/bcachefs/super_types.h create mode 100644 fs/bcachefs/sysfs.c create mode 100644 fs/bcachefs/sysfs.h create mode 100644 fs/bcachefs/tests.c create mode 100644 fs/bcachefs/tests.h create mode 100644 fs/bcachefs/trace.c create mode 100644 fs/bcachefs/trace.h create mode 100644 fs/bcachefs/two_state_shared_lock.c create mode 100644 fs/bcachefs/two_state_shared_lock.h create mode 100644 fs/bcachefs/util.c create mode 100644 fs/bcachefs/util.h create mode 100644 fs/bcachefs/varint.c create mode 100644 fs/bcachefs/varint.h create mode 100644 fs/bcachefs/vstructs.h create mode 100644 fs/bcachefs/xattr.c create mode 100644 fs/bcachefs/xattr.h rename {drivers/md/bcache => include/linux}/closure.h (93%) rename {drivers/md/bcache => lib}/closure.c (88%)