On Thu, Jul 06, 2023 at 01:38:19PM -0400, Kent Overstreet wrote: > On Thu, Jul 06, 2023 at 12:40:55PM -0400, Josef Bacik wrote: ... > > I am really, really wanting you to succeed here Kent. If the general consensus > > is you need to have some idiot review fs/bcachefs I will happily carve out some > > time and dig in. > > That would be much appreciated - I'll owe you some beers next time I see > you. But before jumping in, let's see if we can get people who have > already worked with the code to say something. > I've been poking at bcachefs for several months or so now. I'm happy to chime in on my practical experience thus far, though I'm still not totally clear what folks are looking for on this front, in terms of actual review. I agree with Josef's sentiment that a thorough code review of the entire fs is not really practical. I've not done that and don't plan to in the short term. As it is, I have been able to dig into various areas of the code, learn some of the basic principles, diagnose/fix issues and get some of those fixes merged without too much trouble. IMO, the code is fairly well organized at a high level, reasonably well documented and debuggable/supportable. That isn't to say some of those things couldn't be improved (and I expect they will be), but these are more time and resource constraints than anything and so I don't see any major red flags in that regard. Some of my bigger personal gripes would be a lot of macro code generation stuff makes it a bit harder (but not impossible) for a novice to come up to speed, and similarly a bit more introductory/feature level documentation would be useful to help navigate areas of code without having to rely on Kent as much. The documentation that is available is still pretty good for gaining a high level understanding of the fs data structures, though I agree that more content on things like on-disk format would be really nice. Functionality wise I think it's inevitable that there will be some growing pains as user and developer base grows. For that reason I think having some kind of experimental status for a period of time is probably the right approach. Most of the issues I've dug into personally have been corner case type things, but experience shows that these are the sorts of things that eventually arise with more users. We've also briefly discussed things like whether bcachefs could take more advantage of some of the test coverage that btrfs already has in fstests, since the feature sets should largely overlap. That is TBD, but is something else that might be a good step towards further proving out reliability. Related to that, something I'm not sure I've seen described anywhere is the functional/production status of the filesystem itself (not necessarily the development status of the various features). For example, is the filesystem used in production at any level? If so, what kinds of deployments, workloads and use cases do you know about? How long have they been in use, etc.? I realize we may not have knowledge or permission to share details, but any general info about usage in the wild would be interesting. The development process is fairly ad hoc, so I suspect that is something that would have to evolve if this lands upstream. Kent, did you have thoughts/plans around that? I don't mind contributing reviews where I can, but that means patches would be posted somewhere for feedback, etc. I suppose that has potential to slow things down, but also gives people a chance to see what's happening, review or ask questions, etc., which is another good way to learn or simply keep up with things. All in all I pretty much agree with Josef wrt to the merge request. ISTM the main issues right now are the external dependencies and development/community situation (i.e. bus factor). As above, I plan to continue contributions at least in terms of fixes and whatnot so long as $employer continues to allow me to dedicate at least some time to it and the community is functional ;), but it's not clear to me if that is sufficient to address the concerns here. WRT the dependencies, I agree it makes sense to be deliberate and for anything that is contentious, either just drop it or lift it into bcachefs for now to avoid the need to debate on these various fronts in the first place (and simplify the pull request as much as possible). With those issues addressed, perhaps it would be helpful if other interested fs maintainers/devs could chime in with any thoughts on what they'd want to see in order to ack (but not necessarily "review") a new filesystem pull request..? I don't have the context of the off list thread, but from this thread ISTM that perhaps Josef and Darrick are close to being "soft" acks provided the external dependencies are worked out. Christoph sent a nak based on maintainer status. Kent, you can add me as a reviewer if 1. you think that will help and 2. if you plan to commit to some sort of more formalized development process that will facilitate review..? I don't know if that means an ack from Christoph, but perhaps it addresses the nak. I don't really expect anybody to review the entire codebase, but obviously it's available for anybody who might want to dig into certain areas in more detail.. Brian
