On Tue, Jun 25, 2019 at 12:10:20PM +0200, Christoph Hellwig wrote: > On Tue, Jun 25, 2019 at 09:43:04AM +1000, Dave Chinner wrote: > > I'm a little concerned this is going to limit what we can do > > with the XFS IO path because now we can't change this code without > > considering the direct impact on other filesystems. The QA burden of > > changing the XFS writeback code goes through the roof with this > > change (i.e. we can break multiple filesystems, not just XFS). > > Going through the roof is a little exaggerated. You've already mentioned two new users you want to add. I don't even have zone capable hardware here to test one of the users you are indicating will use this code, and I suspect that very few people do. That's a non-trivial increase in testing requirements for filesystem developers and distro QA departments who will want to change and/or validate this code path. > Yes, it will be more > testing overhead, but that is life in a world where we try to share > code rather than duplicating it, which is pretty much a general > kernel policy that has served us well. Yes, but we also need to acknowledge why we have re-implemented everything in fs/iomap.c - we haven't lifted code from XFS to do that - we've replaced existing generic code that didn't do what we needed and couldn't easily be modified to do what we needed because of all it's external dependencies. Indeed, integrating gfs2 into the existing generic iomap code has required quite a bit of munging and adding new code paths and so on. That's mostly been straight forward because it's just been adding flags and conditional code to the existing paths. The way we regularly rewrite sections of the XFS writeback code is a very different sort of modification, and one that will be much harder to do if we have to make those changes to generic code. i.e. shared code is good if it's simple and doesn't have a lot of external dependencies that restrict the type and scope of modifications that can be made easily. Shared code that is complex and comes from code that was tightly integrated with a specific subsystem architecture is going to carry all those architectural foilbles into the new "generic" code. Once it gets sufficient users it's going to end up with the same "we can't change this code" problems that we had with the existing IO path, and we'll go back to implementing our own writeback path.... > > The writepage code is one of the areas that, historically speaking, > > has one of the highest rates of modification in XFS - we've > > substantially reworked this code from top to bottom 4 or 5 times in > > a bit over ten years, and each time it's been removing abstraction > > layers and getting the writeback code closer to the internal XFS > > extent mapping infrastructure. > > I don't think we had all that much churn. We've had more churn over time to the writeback code than just about any other subsystem in XFS. It also represents a complete 180-degree flip on how we've been streamlining the writeback path in XFS over the past few years. We've been moving closer and closer to the generic writeback infrastructure as well as closer to the XFS inode extent tree. I've been planning on taking it even closer to the extent tree to give us lockless, modification range coherent extent map caching in this path (e.g. write() can add new delalloc extents without invalidating cached writeback maps). This patchset re-introduces the iomap abstraction over the bmbt - an abstraction we removed some time ago - and that makes these sorts of improvements much harder and more complex to implement.... IOWs, I'm not convinced that lifting this code out of XFS is the best long term plan for XFS or the iomap code here.... > Yes, we've improved it a > lot, but much of that was in response to core changes, and pretty much > all of it benefits other users as well. And the more users we have > for this infrastructure that more clout it has with core VM folks > when we have to push back odd design decisions. If we have to make stuff "generic" to be able to influence how other subsystems go about providing infrastructure to filesytsems, then our development community and processes are even more broken than I think they are. Developer communication and design influence are not problems we should be trying to fix with code. Cheers, Dave. -Dave. -- Dave Chinner david@xxxxxxxxxxxxx
